From 16a016b2a8ece2eae29e75b7a7a96dfadd45ac80 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Fri, 14 Sep 2018 16:56:06 +0000 Subject: [PATCH] Have the ClusterBuildTemplate controller instantiate Image resources. Unlike the BuildTemplate controller, which colocates the Image references, this places the Image resources (which are namespaced) in the knative-build namespace. --- pkg/reconciler/buildtemplate/buildtemplate.go | 39 +-- .../buildtemplate/resources/imagecache.go | 4 +- .../clusterbuildtemplate.go | 47 +++- .../resources/imagecache.go | 28 +++ .../resources/imagecache_test.go | 231 ++++++++++++++++++ pkg/system/names.go | 23 ++ 6 files changed, 350 insertions(+), 22 deletions(-) create mode 100644 pkg/reconciler/clusterbuildtemplate/resources/imagecache.go create mode 100644 pkg/reconciler/clusterbuildtemplate/resources/imagecache_test.go create mode 100644 pkg/system/names.go diff --git a/pkg/reconciler/buildtemplate/buildtemplate.go b/pkg/reconciler/buildtemplate/buildtemplate.go index 1ef36a37..8fdad1b9 100644 --- a/pkg/reconciler/buildtemplate/buildtemplate.go +++ b/pkg/reconciler/buildtemplate/buildtemplate.go @@ -153,21 +153,8 @@ func (c *Reconciler) reconcileImageCaches(ctx context.Context, bt *v1alpha1.Buil } // Make sure we have all of the desired caching resources. - if missing := allCached(ics, eics); len(missing) > 0 { - grp, _ := errgroup.WithContext(ctx) - - for _, m := range missing { - m := m - grp.Go(func() error { - _, err := c.cachingclientset.CachingV1alpha1().Images(m.Namespace).Create(&m) - return err - }) - } - - // Wait for the creates to complete. - if err := grp.Wait(); err != nil { - return err - } + if err := CreateMissingImageCaches(ctx, c.cachingclientset, ics, eics); err != nil { + return err } // Delete any Image caches relevant to older versions of this resource. @@ -178,7 +165,7 @@ func (c *Reconciler) reconcileImageCaches(ctx context.Context, bt *v1alpha1.Buil ) } -func allCached(desired []caching.Image, observed []*caching.Image) (missing []caching.Image) { +func missingImageCaches(desired []caching.Image, observed []*caching.Image) (missing []caching.Image) { for _, d := range desired { found := false for _, o := range observed { @@ -193,3 +180,23 @@ func allCached(desired []caching.Image, observed []*caching.Image) (missing []ca } return missing } + +func CreateMissingImageCaches(ctx context.Context, client cachingclientset.Interface, + desired []caching.Image, observed []*caching.Image) error { + missing := missingImageCaches(desired, observed) + if len(missing) == 0 { + return nil + } + + grp, _ := errgroup.WithContext(ctx) + for _, m := range missing { + m := m + grp.Go(func() error { + _, err := client.CachingV1alpha1().Images(m.Namespace).Create(&m) + return err + }) + } + + // Wait for the creates to complete. + return grp.Wait() +} diff --git a/pkg/reconciler/buildtemplate/resources/imagecache.go b/pkg/reconciler/buildtemplate/resources/imagecache.go index 68890213..c4fe840e 100644 --- a/pkg/reconciler/buildtemplate/resources/imagecache.go +++ b/pkg/reconciler/buildtemplate/resources/imagecache.go @@ -30,7 +30,7 @@ import ( // Note: namespace is passed separately because this may be used for // cluster-scoped stuff as well. -func makeImageCachesFromSpec( +func MakeImageCachesFromSpec( namespace string, bt names.ImageCacheable, ) []caching.Image { @@ -67,5 +67,5 @@ func makeImageCachesFromSpec( } func MakeImageCaches(bt *v1alpha1.BuildTemplate) []caching.Image { - return makeImageCachesFromSpec(bt.Namespace, bt) + return MakeImageCachesFromSpec(bt.Namespace, bt) } diff --git a/pkg/reconciler/clusterbuildtemplate/clusterbuildtemplate.go b/pkg/reconciler/clusterbuildtemplate/clusterbuildtemplate.go index 7a00b009..3d89e871 100644 --- a/pkg/reconciler/clusterbuildtemplate/clusterbuildtemplate.go +++ b/pkg/reconciler/clusterbuildtemplate/clusterbuildtemplate.go @@ -21,18 +21,24 @@ import ( "go.uber.org/zap" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/cache" "github.com/knative/pkg/controller" + "github.com/knative/pkg/kmeta" "github.com/knative/pkg/logging" "github.com/knative/pkg/logging/logkey" + "github.com/knative/build/pkg/apis/build/v1alpha1" clientset "github.com/knative/build/pkg/client/clientset/versioned" buildscheme "github.com/knative/build/pkg/client/clientset/versioned/scheme" informers "github.com/knative/build/pkg/client/informers/externalversions/build/v1alpha1" listers "github.com/knative/build/pkg/client/listers/build/v1alpha1" + "github.com/knative/build/pkg/reconciler/buildtemplate" + "github.com/knative/build/pkg/reconciler/clusterbuildtemplate/resources" + "github.com/knative/build/pkg/system" cachingclientset "github.com/knative/caching/pkg/client/clientset/versioned" cachinginformers "github.com/knative/caching/pkg/client/informers/externalversions/caching/v1alpha1" cachinglisters "github.com/knative/caching/pkg/client/listers/caching/v1alpha1" @@ -50,7 +56,7 @@ type Reconciler struct { cachingclientset cachingclientset.Interface clusterBuildTemplatesLister listers.ClusterBuildTemplateLister - imageLister cachinglisters.ImageLister + imagesLister cachinglisters.ImageLister // Sugared logger is easier to use but is not as performant as the // raw logger. In performance critical paths, call logger.Desugar() @@ -87,7 +93,7 @@ func NewController( buildclientset: buildclientset, cachingclientset: cachingclientset, clusterBuildTemplatesLister: clusterBuildTemplateInformer.Lister(), - imageLister: imageInformer.Lister(), + imagesLister: imageInformer.Lister(), Logger: logger, } impl := controller.NewImpl(r, logger, "ClusterBuildTemplates") @@ -99,6 +105,14 @@ func NewController( UpdateFunc: controller.PassNew(impl.Enqueue), }) + imageInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.Filter(v1alpha1.SchemeGroupVersion.WithKind("ClusterBuildTemplate")), + Handler: cache.ResourceEventHandlerFuncs{ + AddFunc: impl.EnqueueControllerOf, + UpdateFunc: controller.PassNew(impl.EnqueueControllerOf), + }, + }) + return impl } @@ -107,7 +121,8 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { logger := logging.FromContext(ctx) // Get the ClusterBuildTemplate resource with this key - if _, err := c.clusterBuildTemplatesLister.Get(key); errors.IsNotFound(err) { + cbt, err := c.clusterBuildTemplatesLister.Get(key) + if errors.IsNotFound(err) { // The ClusterBuildTemplate resource may no longer exist, in which case we stop processing. logger.Errorf("clusterbuildtemplate %q in work queue no longer exists", key) return nil @@ -115,6 +130,30 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { return err } - // TODO: Meaningful reconciliation. + if err := c.reconcileImageCaches(ctx, cbt); err != nil { + return err + } + return nil } + +func (c *Reconciler) reconcileImageCaches(ctx context.Context, cbt *v1alpha1.ClusterBuildTemplate) error { + ics := resources.MakeImageCaches(cbt) + + eics, err := c.imagesLister.Images(system.Namespace).List(kmeta.MakeVersionLabelSelector(cbt)) + if err != nil { + return err + } + + // Make sure we have all of the desired caching resources. + if err := buildtemplate.CreateMissingImageCaches(ctx, c.cachingclientset, ics, eics); err != nil { + return err + } + + // Delete any Image caches relevant to older versions of this resource. + propPolicy := metav1.DeletePropagationForeground + return c.cachingclientset.CachingV1alpha1().Images(system.Namespace).DeleteCollection( + &metav1.DeleteOptions{PropagationPolicy: &propPolicy}, + metav1.ListOptions{LabelSelector: kmeta.MakeOldVersionLabelSelector(cbt).String()}, + ) +} diff --git a/pkg/reconciler/clusterbuildtemplate/resources/imagecache.go b/pkg/reconciler/clusterbuildtemplate/resources/imagecache.go new file mode 100644 index 00000000..9eb59e0f --- /dev/null +++ b/pkg/reconciler/clusterbuildtemplate/resources/imagecache.go @@ -0,0 +1,28 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "github.com/knative/build/pkg/apis/build/v1alpha1" + buildtemplateresources "github.com/knative/build/pkg/reconciler/buildtemplate/resources" + "github.com/knative/build/pkg/system" + caching "github.com/knative/caching/pkg/apis/caching/v1alpha1" +) + +func MakeImageCaches(bt *v1alpha1.ClusterBuildTemplate) []caching.Image { + return buildtemplateresources.MakeImageCachesFromSpec(system.Namespace, bt) +} diff --git a/pkg/reconciler/clusterbuildtemplate/resources/imagecache_test.go b/pkg/reconciler/clusterbuildtemplate/resources/imagecache_test.go new file mode 100644 index 00000000..fb52f56b --- /dev/null +++ b/pkg/reconciler/clusterbuildtemplate/resources/imagecache_test.go @@ -0,0 +1,231 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/knative/build/pkg/apis/build/v1alpha1" + "github.com/knative/build/pkg/system" + caching "github.com/knative/caching/pkg/apis/caching/v1alpha1" +) + +func TestMakeImageCache(t *testing.T) { + boolTrue := true + + tests := []struct { + name string + bt *v1alpha1.ClusterBuildTemplate + want []caching.Image + }{{ + name: "no container", + bt: &v1alpha1.ClusterBuildTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + UID: "1234", + ResourceVersion: "asdf", + }, + Spec: v1alpha1.BuildTemplateSpec{}, + }, + }, { + name: "single container", + bt: &v1alpha1.ClusterBuildTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + UID: "1234", + ResourceVersion: "asdf", + }, + Spec: v1alpha1.BuildTemplateSpec{ + Steps: []corev1.Container{{ + Image: "busybox", + }}, + }, + }, + want: []caching.Image{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace, + Name: "bar-asdf-00000", + Labels: map[string]string{ + "controller": "1234", + "version": "asdf", + }, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "ClusterBuildTemplate", + Name: "bar", + UID: "1234", + Controller: &boolTrue, + BlockOwnerDeletion: &boolTrue, + }}, + }, + Spec: caching.ImageSpec{ + Image: "busybox", + }, + }}, + }, { + name: "duplicate container", + bt: &v1alpha1.ClusterBuildTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + UID: "1234", + ResourceVersion: "asdf", + }, + Spec: v1alpha1.BuildTemplateSpec{ + Steps: []corev1.Container{{ + Image: "busybox", + }, { + Image: "busybox", + }}, + }, + }, + want: []caching.Image{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace, + Name: "bar-asdf-00000", + Labels: map[string]string{ + "controller": "1234", + "version": "asdf", + }, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "ClusterBuildTemplate", + Name: "bar", + UID: "1234", + Controller: &boolTrue, + BlockOwnerDeletion: &boolTrue, + }}, + }, + Spec: caching.ImageSpec{ + Image: "busybox", + }, + }}, + }, { + name: "multiple containers", + bt: &v1alpha1.ClusterBuildTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + UID: "1234", + ResourceVersion: "asdf", + }, + Spec: v1alpha1.BuildTemplateSpec{ + Steps: []corev1.Container{{ + Image: "busybox", + }, { + Image: "helloworld", + }, { + Image: "busybox", + }}, + }, + }, + want: []caching.Image{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace, + Name: "bar-asdf-00000", + Labels: map[string]string{ + "controller": "1234", + "version": "asdf", + }, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "ClusterBuildTemplate", + Name: "bar", + UID: "1234", + Controller: &boolTrue, + BlockOwnerDeletion: &boolTrue, + }}, + }, + Spec: caching.ImageSpec{ + Image: "busybox", + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace, + Name: "bar-asdf-00001", + Labels: map[string]string{ + "controller": "1234", + "version": "asdf", + }, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "ClusterBuildTemplate", + Name: "bar", + UID: "1234", + Controller: &boolTrue, + BlockOwnerDeletion: &boolTrue, + }}, + }, + Spec: caching.ImageSpec{ + Image: "helloworld", + }, + }}, + }, { + name: "containers with substitutions", + bt: &v1alpha1.ClusterBuildTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + UID: "1234", + ResourceVersion: "asdf", + }, + Spec: v1alpha1.BuildTemplateSpec{ + Parameters: []v1alpha1.ParameterSpec{{ + Name: "TAG", + }}, + Steps: []corev1.Container{{ + Image: "busybox", + }, { + Image: "helloworld:${TAG}", + }, { + Image: "busybox", + }}, + }, + }, + want: []caching.Image{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace, + Name: "bar-asdf-00000", + Labels: map[string]string{ + "controller": "1234", + "version": "asdf", + }, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "ClusterBuildTemplate", + Name: "bar", + UID: "1234", + Controller: &boolTrue, + BlockOwnerDeletion: &boolTrue, + }}, + }, + Spec: caching.ImageSpec{ + Image: "busybox", + }, + }}, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := MakeImageCaches(test.bt) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("MakeImageCache (-want, +got) = %v", diff) + } + }) + } +} diff --git a/pkg/system/names.go b/pkg/system/names.go new file mode 100644 index 00000000..8d77d564 --- /dev/null +++ b/pkg/system/names.go @@ -0,0 +1,23 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package system + +const ( + // Namespace holds the K8s namespace where our build system + // components run. + Namespace = "knative-build" +)