Decouple from build: swap out build client: creating build (knative#2065

)

* Use dynamic client to create Build from Configuration

* do start buildInformerFactory

* test/e2e: build maps: use []interface{} instead of []string

so DeepCopy() works

* cleanups

- set build type meta
- eliminate unused BuildClientSet field in reconciler.Base

* try to fix e2e

* eliminate BuildClientSet from reconciler.Options{}

* moar coverage

* even moar

cmd/controller/main.go

@@ -31,6 +31,7 @@ import (
 
 	vpa "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned"
 	vpainformers "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/informers/externalversions"
+	"k8s.io/client-go/dynamic"
 	kubeinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
@@ -104,6 +105,11 @@ func main() {
 		logger.Fatalf("Error building build clientset: %v", err)
 	}
 
+	dynamicClient, err := dynamic.NewForConfig(cfg)
+	if err != nil {
+		logger.Fatalf("Error building build clientset: %v", err)
+	}
+
 	cachingClient, err := cachingclientset.NewForConfig(cfg)
 	if err != nil {
 		logger.Fatalf("Error building caching clientset: %v", err)
@@ -128,7 +134,7 @@ func main() {
 		SharedClientSet:  sharedClient,
 		ServingClientSet: servingClient,
 		CachingClientSet: cachingClient,
-		BuildClientSet:   buildClient,
+		DynamicClientSet: dynamicClient,
 		ConfigMapWatcher: configMapWatcher,
 		Logger:           logger,
 	}

pkg/apis/serving/v1alpha1/configuration_types.go

@@ -17,9 +17,8 @@ limitations under the License.
 package v1alpha1
 
 import (
-	build "github.com/knative/build/pkg/apis/build/v1alpha1"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/knative/pkg/apis"
@@ -80,7 +79,7 @@ type ConfigurationSpec struct {
 	// Build optionally holds the specification for the build to
 	// perform to produce the Revision's container image.
 	// +optional
-	Build *build.BuildSpec `json:"build,omitempty"`
+	Build *unstructured.Unstructured `json:"build,omitempty"`
 
 	// RevisionTemplate holds the latest specification for the Revision to
 	// be stamped out. If a Build specification is provided, then the

pkg/reconciler/reconciler.go

@@ -17,9 +17,9 @@ limitations under the License.
 package reconciler
 
 import (
+	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes/scheme"
 
-	buildclientset "github.com/knative/build/pkg/client/clientset/versioned"
 	cachingclientset "github.com/knative/caching/pkg/client/clientset/versioned"
 	sharedclientset "github.com/knative/pkg/client/clientset/versioned"
 	"github.com/knative/pkg/configmap"
@@ -40,7 +40,7 @@ type Options struct {
 	KubeClientSet    kubernetes.Interface
 	SharedClientSet  sharedclientset.Interface
 	ServingClientSet clientset.Interface
-	BuildClientSet   buildclientset.Interface
+	DynamicClientSet dynamic.Interface
 	CachingClientSet cachingclientset.Interface
 
 	ConfigMapWatcher configmap.Watcher
@@ -58,8 +58,8 @@ type Base struct {
 	// ServingClientSet allows us to configure Serving objects
 	ServingClientSet clientset.Interface
 
-	// BuildClientSet allows us to configure Build objects
-	BuildClientSet buildclientset.Interface
+	// DynamicClientSet allows us to configure pluggable Build objects
+	DynamicClientSet dynamic.Interface
 
 	// CachingClientSet allows us to instantiate Image objects
 	CachingClientSet cachingclientset.Interface
@@ -97,7 +97,7 @@ func NewBase(opt Options, controllerAgentName string) *Base {
 		KubeClientSet:    opt.KubeClientSet,
 		SharedClientSet:  opt.SharedClientSet,
 		ServingClientSet: opt.ServingClientSet,
-		BuildClientSet:   opt.BuildClientSet,
+		DynamicClientSet: opt.DynamicClientSet,
 		CachingClientSet: opt.CachingClientSet,
 		ConfigMapWatcher: opt.ConfigMapWatcher,
 		Recorder:         recorder,

pkg/reconciler/testing/table.go

@@ -83,7 +83,7 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
 
 	for i, want := range r.WantCreates {
 		if i >= len(actions.Creates) {
-			t.Errorf("Missing create: %v", want)
+			t.Errorf("Missing create: %#v", want)
 			continue
 		}
 		got := actions.Creates[i]
@@ -97,13 +97,13 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
 	}
 	if got, want := len(actions.Creates), len(r.WantCreates); got > want {
 		for _, extra := range actions.Creates[want:] {
-			t.Errorf("Extra create: %v", extra)
+			t.Errorf("Extra create: %#v", extra)
 		}
 	}
 
 	for i, want := range r.WantUpdates {
 		if i >= len(actions.Updates) {
-			t.Errorf("Missing update: %v", want.GetObject())
+			t.Errorf("Missing update: %#v", want.GetObject())
 			continue
 		}
 		got := actions.Updates[i]
@@ -113,13 +113,13 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
 	}
 	if got, want := len(actions.Updates), len(r.WantUpdates); got > want {
 		for _, extra := range actions.Updates[want:] {
-			t.Errorf("Extra update: %v", extra)
+			t.Errorf("Extra update: %#v", extra)
 		}
 	}
 
 	for i, want := range r.WantDeletes {
 		if i >= len(actions.Deletes) {
-			t.Errorf("Missing delete: %v", want)
+			t.Errorf("Missing delete: %#v", want)
 			continue
 		}
 		got := actions.Deletes[i]
@@ -132,13 +132,13 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
 	}
 	if got, want := len(actions.Deletes), len(r.WantDeletes); got > want {
 		for _, extra := range actions.Deletes[want:] {
-			t.Errorf("Extra delete: %v", extra)
+			t.Errorf("Extra delete: %#v", extra)
 		}
 	}
 
 	for i, want := range r.WantPatches {
 		if i >= len(actions.Patches) {
-			t.Errorf("Missing patch: %v", want)
+			t.Errorf("Missing patch: %#v", want)
 			continue
 		}
 
@@ -155,7 +155,7 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
 	}
 	if got, want := len(actions.Patches), len(r.WantPatches); got > want {
 		for _, extra := range actions.Patches[want:] {
-			t.Errorf("Extra patch: %v", extra)
+			t.Errorf("Extra patch: %#v", extra)
 		}
 	}
 }

pkg/reconciler/v1alpha1/configuration/configuration.go

@@ -27,6 +27,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/client-go/tools/cache"
 
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
@@ -196,12 +197,13 @@ func (c *Reconciler) createRevision(ctx context.Context, config *v1alpha1.Config
 	if config.Spec.Build != nil {
 		// TODO(mattmoor): Determine whether we reuse the previous build.
 		build := resources.MakeBuild(config)
-		created, err := c.BuildClientSet.BuildV1alpha1().Builds(build.Namespace).Create(build)
+		gvr, _ := meta.UnsafeGuessKindToResource(build.GroupVersionKind())
+		created, err := c.DynamicClientSet.Resource(gvr).Namespace(build.GetNamespace()).Create(build)
 		if err != nil {
 			return nil, err
 		}
-		logger.Infof("Created Build:\n%+v", created.Name)
-		c.Recorder.Eventf(config, corev1.EventTypeNormal, "Created", "Created Build %q", created.Name)
+		logger.Infof("Created Build:\n%+v", created.GetName())
+		c.Recorder.Eventf(config, corev1.EventTypeNormal, "Created", "Created Build %q", created.GetName())
 	}
 
 	rev := resources.MakeRevision(config)

pkg/reconciler/v1alpha1/configuration/configuration_test.go

@@ -19,14 +19,14 @@ package configuration
 import (
 	"testing"
 
-	buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
 	duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
 	"github.com/knative/pkg/controller"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/knative/serving/pkg/reconciler"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgotesting "k8s.io/client-go/testing"
 
@@ -40,13 +40,13 @@ var (
 			Image: "busybox",
 		},
 	}
-	buildSpec = buildv1alpha1.BuildSpec{
-		Steps: []corev1.Container{{
-			Image: "build-step1",
-		}, {
-			Image: "build-step2",
+	buildSpec = *resources.UnstructuredWithContent(map[string]interface{}{
+		"steps": []interface{}{map[string]interface{}{
+			"image": "build-step1",
+		}, map[string]interface{}{
+			"image": "build-step2",
 		}},
-	}
+	})
 )
 
 // This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method.
@@ -346,7 +346,7 @@ func TestReconcile(t *testing.T) {
 	}))
 }
 
-func cfgWithBuildAndStatus(name, namespace string, generation int64, build *buildv1alpha1.BuildSpec, status v1alpha1.ConfigurationStatus) *v1alpha1.Configuration {
+func cfgWithBuildAndStatus(name, namespace string, generation int64, build *unstructured.Unstructured, status v1alpha1.ConfigurationStatus) *v1alpha1.Configuration {
 	return &v1alpha1.Configuration{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -367,7 +367,7 @@ func cfgWithStatus(name, namespace string, generation int64, status v1alpha1.Con
 	return cfgWithBuildAndStatus(name, namespace, generation, nil, status)
 }
 
-func cfgWithBuild(name, namespace string, generation int64, build *buildv1alpha1.BuildSpec) *v1alpha1.Configuration {
+func cfgWithBuild(name, namespace string, generation int64, build *unstructured.Unstructured) *v1alpha1.Configuration {
 	return cfgWithBuildAndStatus(name, namespace, generation, build, v1alpha1.ConfigurationStatus{})
 }
 

pkg/reconciler/v1alpha1/configuration/queueing_test.go

@@ -20,7 +20,6 @@ import (
 	"testing"
 	"time"
 
-	fakebuildclientset "github.com/knative/build/pkg/client/clientset/versioned/fake"
 	fakesharedclientset "github.com/knative/pkg/client/clientset/versioned/fake"
 	ctrl "github.com/knative/pkg/controller"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
@@ -113,7 +112,6 @@ func newTestController(t *testing.T, servingObjects ...runtime.Object) (
 			KubeClientSet:    kubeClient,
 			SharedClientSet:  sharedClient,
 			ServingClientSet: servingClient,
-			BuildClientSet:   fakebuildclientset.NewSimpleClientset(),
 			Logger:           TestLogger(t),
 		},
 		servingInformer.Serving().V1alpha1().Configurations(),

pkg/reconciler/v1alpha1/configuration/resources/build.go

@@ -17,24 +17,53 @@ limitations under the License.
 package resources
 
 import (
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
 	"github.com/knative/pkg/kmeta"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources/names"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
-func MakeBuild(config *v1alpha1.Configuration) *buildv1alpha1.Build {
+func MakeBuild(config *v1alpha1.Configuration) *unstructured.Unstructured {
 	if config.Spec.Build == nil {
 		return nil
 	}
-	return &buildv1alpha1.Build{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace:       config.Namespace,
-			Name:            names.Build(config),
-			OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(config)},
-		},
-		Spec: *config.Spec.Build,
+	u := WithBuildSpec(config.Spec.Build)
+
+	u.SetNamespace(config.Namespace)
+	u.SetName(names.Build(config))
+	u.SetOwnerReferences([]metav1.OwnerReference{*kmeta.NewControllerRef(config)})
+	return u
+}
+
+func WithBuildSpec(build *unstructured.Unstructured) *unstructured.Unstructured {
+	u := &unstructured.Unstructured{}
+
+	spec, ok := build.Object["spec"]
+	if !ok {
+		s := build.DeepCopy().Object
+		delete(s, "apiVersion")
+		delete(s, "kind")
+		spec = s
+	}
+	u.SetUnstructuredContent(map[string]interface{}{"spec": spec})
+
+	// Assume {apiVersion: build.knative.dev/v1alpha1, kind: Build} if not set
+	if build.GroupVersionKind().Empty() {
+		u.SetGroupVersionKind(schema.GroupVersionKind{Group: "build.knative.dev", Version: "v1alpha1", Kind: "Build"})
+	} else {
+		u.SetGroupVersionKind(build.GroupVersionKind())
+	}
+
+	return u
+}
+
+func UnstructuredWithContent(content map[string]interface{}) *unstructured.Unstructured {
+	if content == nil {
+		return nil
 	}
+	u := &unstructured.Unstructured{}
+	u.SetUnstructuredContent(content)
+	return u.DeepCopy()
 }