diff --git a/pkg/reconciler/builder/builder.go b/pkg/reconciler/builder/builder.go index 2029a7619..bc2c8b7fd 100644 --- a/pkg/reconciler/builder/builder.go +++ b/pkg/reconciler/builder/builder.go @@ -5,6 +5,8 @@ import ( "go.uber.org/zap" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/logging/logkey" "github.com/google/go-containerregistry/pkg/authn" @@ -14,6 +16,7 @@ import ( "k8s.io/client-go/tools/cache" "knative.dev/pkg/controller" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" "github.com/pivotal/kpack/pkg/client/clientset/versioned" @@ -110,22 +113,40 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { } func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Builder) (buildapi.BuilderRecord, error) { - clusterStore, err := c.ClusterStoreLister.Get(builder.Spec.Store.Name) + err := c.Tracker.Track(reconciler.Key{ + NamespacedName: types.NamespacedName{ + Name: builder.Spec.Store.Name, + Namespace: v1.NamespaceAll, + }, + GroupKind: schema.GroupKind{ + Group: "kpack.io", + Kind: v1alpha2.ClusterStoreKind, + }, + }, builder.NamespacedName()) if err != nil { return buildapi.BuilderRecord{}, err } - err = c.Tracker.Track(reconciler.KeyForObject(clusterStore), builder.NamespacedName()) + err = c.Tracker.Track(reconciler.Key{ + NamespacedName: types.NamespacedName{ + Name: builder.Spec.Stack.Name, + Namespace: v1.NamespaceAll, + }, + GroupKind: schema.GroupKind{ + Group: "kpack.io", + Kind: v1alpha2.ClusterStackKind, + }, + }, builder.NamespacedName()) if err != nil { return buildapi.BuilderRecord{}, err } - clusterStack, err := c.ClusterStackLister.Get(builder.Spec.Stack.Name) + clusterStore, err := c.ClusterStoreLister.Get(builder.Spec.Store.Name) if err != nil { return buildapi.BuilderRecord{}, err } - err = c.Tracker.Track(reconciler.KeyForObject(clusterStack), builder.NamespacedName()) + clusterStack, err := c.ClusterStackLister.Get(builder.Spec.Stack.Name) if err != nil { return buildapi.BuilderRecord{}, err } diff --git a/pkg/reconciler/builder/builder_test.go b/pkg/reconciler/builder/builder_test.go index e4e487128..578817b73 100644 --- a/pkg/reconciler/builder/builder_test.go +++ b/pkg/reconciler/builder/builder_test.go @@ -414,5 +414,77 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(notReadyClusterStack), builder.NamespacedName())) require.Len(t, builderCreator.CreateBuilderCalls, 0) }) + + it("updates status and doesn't build builder when the store doesn't exist", func() { + rt.Test(rtesting.TableRow{ + Key: builderKey, + Objects: []runtime.Object{ + clusterStack, + builder, + }, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{ + { + Object: &buildapi.Builder{ + ObjectMeta: builder.ObjectMeta, + Spec: builder.Spec, + Status: buildapi.BuilderStatus{ + Status: corev1alpha1.Status{ + ObservedGeneration: 1, + Conditions: corev1alpha1.Conditions{ + { + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionFalse, + Message: `clusterstore.kpack.io "some-store" not found`, + }, + }, + }, + }, + }, + }, + }, + }) + + // still track resources + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStore), builder.NamespacedName())) + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStack), builder.NamespacedName())) + require.Len(t, builderCreator.CreateBuilderCalls, 0) + }) + + it("updates status and doesn't build builder when the stack doesn't exist", func() { + rt.Test(rtesting.TableRow{ + Key: builderKey, + Objects: []runtime.Object{ + clusterStore, + builder, + }, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{ + { + Object: &buildapi.Builder{ + ObjectMeta: builder.ObjectMeta, + Spec: builder.Spec, + Status: buildapi.BuilderStatus{ + Status: corev1alpha1.Status{ + ObservedGeneration: 1, + Conditions: corev1alpha1.Conditions{ + { + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionFalse, + Message: `clusterstack.kpack.io "some-stack" not found`, + }, + }, + }, + }, + }, + }, + }, + }) + + // still track resources + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStore), builder.NamespacedName())) + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStack), builder.NamespacedName())) + require.Len(t, builderCreator.CreateBuilderCalls, 0) + }) }) } diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder.go b/pkg/reconciler/clusterbuilder/clusterbuilder.go index ad4f2251b..d0ecb90c2 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder.go @@ -4,7 +4,10 @@ import ( "context" "go.uber.org/zap" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/logging/logkey" "github.com/google/go-containerregistry/pkg/authn" @@ -14,6 +17,7 @@ import ( "k8s.io/client-go/tools/cache" "knative.dev/pkg/controller" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" "github.com/pivotal/kpack/pkg/client/clientset/versioned" @@ -108,22 +112,40 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { } func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.ClusterBuilder) (buildapi.BuilderRecord, error) { - clusterStore, err := c.ClusterStoreLister.Get(builder.Spec.Store.Name) + err := c.Tracker.Track(reconciler.Key{ + NamespacedName: types.NamespacedName{ + Name: builder.Spec.Store.Name, + Namespace: v1.NamespaceAll, + }, + GroupKind: schema.GroupKind{ + Group: "kpack.io", + Kind: v1alpha2.ClusterStoreKind, + }, + }, builder.NamespacedName()) if err != nil { return buildapi.BuilderRecord{}, err } - err = c.Tracker.Track(reconciler.KeyForObject(clusterStore), builder.NamespacedName()) + err = c.Tracker.Track(reconciler.Key{ + NamespacedName: types.NamespacedName{ + Name: builder.Spec.Stack.Name, + Namespace: v1.NamespaceAll, + }, + GroupKind: schema.GroupKind{ + Group: "kpack.io", + Kind: v1alpha2.ClusterStackKind, + }, + }, builder.NamespacedName()) if err != nil { return buildapi.BuilderRecord{}, err } - clusterStack, err := c.ClusterStackLister.Get(builder.Spec.Stack.Name) + clusterStore, err := c.ClusterStoreLister.Get(builder.Spec.Store.Name) if err != nil { return buildapi.BuilderRecord{}, err } - err = c.Tracker.Track(reconciler.KeyForObject(clusterStack), builder.NamespacedName()) + clusterStack, err := c.ClusterStackLister.Get(builder.Spec.Stack.Name) if err != nil { return buildapi.BuilderRecord{}, err } diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go index b33b48c18..de8829677 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go @@ -418,13 +418,82 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { }) // still track resources - require.True(t, fakeTracker.IsTracking( - reconciler.KeyForObject(clusterStore), - builder.NamespacedName())) - require.True(t, fakeTracker.IsTracking( - reconciler.KeyForObject(notReadyClusterStack), - builder.NamespacedName())) + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStore), builder.NamespacedName())) + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(notReadyClusterStack), builder.NamespacedName())) + require.Len(t, builderCreator.CreateBuilderCalls, 0) + }) + + it("updates status and doesn't build builder when the store does not exist", func() { + rt.Test(rtesting.TableRow{ + Key: builderKey, + Objects: []runtime.Object{ + clusterStack, + builder, + }, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{ + { + Object: &buildapi.ClusterBuilder{ + ObjectMeta: builder.ObjectMeta, + TypeMeta: builder.TypeMeta, + Spec: builder.Spec, + Status: buildapi.BuilderStatus{ + Status: corev1alpha1.Status{ + ObservedGeneration: 1, + Conditions: corev1alpha1.Conditions{ + { + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionFalse, + Message: `clusterstore.kpack.io "some-store" not found`, + }, + }, + }, + }, + }, + }, + }, + }) + // still track resources + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStore), builder.NamespacedName())) + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStack), builder.NamespacedName())) + require.Len(t, builderCreator.CreateBuilderCalls, 0) + }) + + it("updates status and doesn't build builder when the stack does not exist", func() { + rt.Test(rtesting.TableRow{ + Key: builderKey, + Objects: []runtime.Object{ + clusterStore, + builder, + }, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{ + { + Object: &buildapi.ClusterBuilder{ + ObjectMeta: builder.ObjectMeta, + TypeMeta: builder.TypeMeta, + Spec: builder.Spec, + Status: buildapi.BuilderStatus{ + Status: corev1alpha1.Status{ + ObservedGeneration: 1, + Conditions: corev1alpha1.Conditions{ + { + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionFalse, + Message: `clusterstack.kpack.io "some-stack" not found`, + }, + }, + }, + }, + }, + }, + }, + }) + + // still track resources + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStore), builder.NamespacedName())) + require.True(t, fakeTracker.IsTracking(reconciler.KeyForObject(clusterStack), builder.NamespacedName())) require.Len(t, builderCreator.CreateBuilderCalls, 0) }) }) diff --git a/pkg/reconciler/image/image.go b/pkg/reconciler/image/image.go index 8e478eaab..659bb61ee 100644 --- a/pkg/reconciler/image/image.go +++ b/pkg/reconciler/image/image.go @@ -11,7 +11,9 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/types" coreinformers "k8s.io/client-go/informers/core/v1" k8sclient "k8s.io/client-go/kubernetes" corelisters "k8s.io/client-go/listers/core/v1" @@ -122,6 +124,20 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { } func (c *Reconciler) reconcileImage(ctx context.Context, image *buildapi.Image) (*buildapi.Image, error) { + err := c.Tracker.Track(reconciler.Key{ + NamespacedName: types.NamespacedName{ + Name: image.Spec.Builder.Name, + Namespace: image.Namespace, + }, + GroupKind: schema.GroupKind{ + Group: "kpack.io", + Kind: image.Spec.Builder.Kind, + }, + }, image.NamespacedName()) + if err != nil { + return nil, err + } + builder, err := c.DuckBuilderLister.Namespace(image.Namespace).Get(image.Spec.Builder) if err != nil && !k8serrors.IsNotFound(err) { return nil, err @@ -131,11 +147,6 @@ func (c *Reconciler) reconcileImage(ctx context.Context, image *buildapi.Image) return image, nil } - err = c.Tracker.Track(reconciler.KeyForObject(builder), image.NamespacedName()) - if err != nil { - return nil, err - } - lastBuild, err := c.fetchLastBuild(image) if err != nil { return nil, err diff --git a/pkg/reconciler/image/image_test.go b/pkg/reconciler/image/image_test.go index 4b22868e8..e452429a6 100644 --- a/pkg/reconciler/image/image_test.go +++ b/pkg/reconciler/image/image_test.go @@ -267,6 +267,11 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, }, }) + + // still track resource + require.True(t, fakeTracker.IsTracking( + reconciler.KeyForObject(builder).WithNamespace(namespace), + image.NamespacedName())) }) when("reconciling source resolvers", func() {