Skip to content

Commit

Permalink
Merge pull request #1082 from eirini-forks/track-by-key
Browse files Browse the repository at this point in the history 
Track dependency objects even if they don't exist
  • Loading branch information
@tomkennedy513
tomkennedy513 authored Dec 5, 2022
2 parents 8e4bce8 + 9cc64df commit d8a483e
Show file tree
Hide file tree
Showing 10 changed files with 357 additions and 63 deletions.
28 changes: 24 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 Down Expand Up @@ -110,22 +112,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: buildapi.ClusterStoreKind,
},
}, builder.NamespacedName())
if err != nil {
return buildapi.BuilderRecord{}, err
}

err = c.Tracker.Track(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: buildapi.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(clusterStack, builder.NamespacedName())
clusterStack, err := c.ClusterStackLister.Get(builder.Spec.Stack.Name)
if err != nil {
return buildapi.BuilderRecord{}, err
}
Expand Down
109 changes: 98 additions & 11 deletions pkg/reconciler/builder/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
clientgotesting "k8s.io/client-go/testing"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -62,17 +62,25 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
})

clusterStore := &buildapi.ClusterStore{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: "some-store",
},
TypeMeta: metav1.TypeMeta{
Kind: "ClusterStore",
APIVersion: "kpack.io/v1alpha2",
},
Spec: buildapi.ClusterStoreSpec{},
Status: buildapi.ClusterStoreStatus{},
}

clusterStack := &buildapi.ClusterStack{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: "some-stack",
},
TypeMeta: metav1.TypeMeta{
Kind: "ClusterStack",
APIVersion: "kpack.io/v1alpha2",
},
Status: buildapi.ClusterStackStatus{
Status: corev1alpha1.Status{
ObservedGeneration: 0,
Expand All @@ -87,7 +95,7 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
}

builder := &buildapi.Builder{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: builderName,
Generation: initialGeneration,
Namespace: testNamespace,
Expand Down Expand Up @@ -257,8 +265,12 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
WantErr: false,
})

require.True(t, fakeTracker.IsTracking(clusterStore, builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(clusterStack, builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(
kreconciler.KeyForObject(clusterStore),
builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(
kreconciler.KeyForObject(clusterStack),
builder.NamespacedName()))
})

it("does not update the status with no status change", func() {
Expand Down Expand Up @@ -342,14 +354,17 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
},
},
})

})

it("updates status and doesn't build builder when stack not ready", func() {
notReadyClusterStack := &buildapi.ClusterStack{
ObjectMeta: v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: "some-stack",
},
TypeMeta: metav1.TypeMeta{
Kind: "ClusterStack",
APIVersion: "kpack.io/v1alpha2",
},
Status: buildapi.ClusterStackStatus{
Status: corev1alpha1.Status{
ObservedGeneration: 0,
Expand All @@ -362,6 +377,7 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
},
},
}

rt.Test(rtesting.TableRow{
Key: builderKey,
Objects: []runtime.Object{
Expand Down Expand Up @@ -392,11 +408,82 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
},
})

//still track resources
require.True(t, fakeTracker.IsTracking(clusterStore, builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(notReadyClusterStack, builder.NamespacedName()))
// still track resources
require.True(t, fakeTracker.IsTracking(kreconciler.KeyForObject(clusterStore), builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(kreconciler.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(kreconciler.KeyForObject(clusterStore), builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(kreconciler.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(kreconciler.KeyForObject(clusterStore), builder.NamespacedName()))
require.True(t, fakeTracker.IsTracking(kreconciler.KeyForObject(clusterStack), builder.NamespacedName()))
require.Len(t, builderCreator.CreateBuilderCalls, 0)
})
})
}
29 changes: 25 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 Down Expand Up @@ -108,22 +111,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: buildapi.ClusterStoreKind,
},
}, builder.NamespacedName())
if err != nil {
return buildapi.BuilderRecord{}, err
}

err = c.Tracker.Track(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: buildapi.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(clusterStack, builder.NamespacedName())
clusterStack, err := c.ClusterStackLister.Get(builder.Spec.Stack.Name)
if err != nil {
return buildapi.BuilderRecord{}, err
}
Expand Down
Loading

0 comments on commit d8a483e

Please sign in to comment.