Skip to content

Commit

Permalink
Track dependency objects even if they don't exist
Browse files Browse the repository at this point in the history 
Issue: #1028
  • Loading branch information
@gcapizzi
gcapizzi committed Dec 2, 2022
1 parent 54faaa3 commit 284d88a
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 19 deletions.
29 changes: 25 additions & 4 deletions pkg/reconciler/builder/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
72 changes: 72 additions & 0 deletions pkg/reconciler/builder/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
}
30 changes: 26 additions & 4 deletions pkg/reconciler/clusterbuilder/clusterbuilder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
81 changes: 75 additions & 6 deletions pkg/reconciler/clusterbuilder/clusterbuilder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
Expand Down
21 changes: 16 additions & 5 deletions pkg/reconciler/image/image.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/reconciler/image/image_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 284d88a

Please sign in to comment.