From 1172c3904e0cdb9dcb12c11d225d8baf7e03c6b2 Mon Sep 17 00:00:00 2001 From: Tom Kennedy Date: Fri, 3 Nov 2023 12:04:56 -0400 Subject: [PATCH 1/3] Add builder ConditionUpToDate status - ConditionReady indicates that the builder can be used in a build (has a latestImage) - ConditionUpToDate indicates the status of the builder reconciler Signed-off-by: Tom Kennedy --- config/builder.yaml | 3 ++ config/clusterbuilder.yaml | 3 ++ pkg/apis/build/v1alpha2/builder_lifecycle.go | 21 +++++++++- pkg/apis/build/v1alpha2/builder_types.go | 8 +++- pkg/duckbuilder/duck_builder.go | 5 +++ pkg/duckbuilder/duck_builder_test.go | 33 +++++++++++++++ pkg/reconciler/builder/builder_test.go | 40 ++++++++++++++++++ .../clusterbuilder/clusterbuilder_test.go | 41 ++++++++++++++++++- test/cosign_e2e_test.go | 39 ++++++++++++------ test/execute_build_test.go | 13 +++--- 10 files changed, 184 insertions(+), 22 deletions(-) diff --git a/config/builder.yaml b/config/builder.yaml index 841b9db87..ab9d3b2d2 100644 --- a/config/builder.yaml +++ b/config/builder.yaml @@ -37,6 +37,9 @@ spec: - name: Ready type: string jsonPath: ".status.conditions[?(@.type==\"Ready\")].status" + - name: UpToDate + type: string + jsonPath: ".status.conditions[?(@.type==\"UpToDate\")].status" conversion: strategy: Webhook webhook: diff --git a/config/clusterbuilder.yaml b/config/clusterbuilder.yaml index eb9fd1d5d..b1305204e 100644 --- a/config/clusterbuilder.yaml +++ b/config/clusterbuilder.yaml @@ -37,6 +37,9 @@ spec: - name: Ready type: string jsonPath: ".status.conditions[?(@.type==\"Ready\")].status" + - name: UpToDate + type: string + jsonPath: ".status.conditions[?(@.type==\"UpToDate\")].status" names: kind: ClusterBuilder listKind: ClusterBuilderList diff --git a/pkg/apis/build/v1alpha2/builder_lifecycle.go b/pkg/apis/build/v1alpha2/builder_lifecycle.go index 5a642f3e5..c2913fc1f 100644 --- a/pkg/apis/build/v1alpha2/builder_lifecycle.go +++ b/pkg/apis/build/v1alpha2/builder_lifecycle.go @@ -29,6 +29,11 @@ func (bs *BuilderStatus) BuilderRecord(record BuilderRecord) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: ConditionUpToDate, + Status: corev1.ConditionTrue, + LastTransitionTime: corev1alpha1.VolatileTime{Inner: v1.Now()}, + }, } bs.Order = record.Order bs.ObservedStoreGeneration = record.ObservedStoreGeneration @@ -38,12 +43,26 @@ func (bs *BuilderStatus) BuilderRecord(record BuilderRecord) { } func (bs *BuilderStatus) ErrorCreate(err error) { + + readyCondition := corev1alpha1.Condition{ + LastTransitionTime: corev1alpha1.VolatileTime{Inner: v1.Now()}, + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionTrue, + } + if bs.LatestImage == "" { + readyCondition.Status = corev1.ConditionFalse + readyCondition.Message = NoLatestImageMessage + readyCondition.Reason = NoLatestImageReason + } + bs.Status = corev1alpha1.Status{ Conditions: corev1alpha1.Conditions{ + readyCondition, { - Type: corev1alpha1.ConditionReady, + Type: ConditionUpToDate, Status: corev1.ConditionFalse, LastTransitionTime: corev1alpha1.VolatileTime{Inner: v1.Now()}, + Reason: ReconcileFailedReason, Message: err.Error(), }, }, diff --git a/pkg/apis/build/v1alpha2/builder_types.go b/pkg/apis/build/v1alpha2/builder_types.go index 3281722cd..343d5966f 100644 --- a/pkg/apis/build/v1alpha2/builder_types.go +++ b/pkg/apis/build/v1alpha2/builder_types.go @@ -10,8 +10,12 @@ import ( ) const ( - BuilderKind = "Builder" - BuilderCRName = "builders.kpack.io" + BuilderKind = "Builder" + BuilderCRName = "builders.kpack.io" + ConditionUpToDate corev1alpha1.ConditionType = "UpToDate" + NoLatestImageReason string = "NoLatestImage" + NoLatestImageMessage string = "Builder has no latestImage" + ReconcileFailedReason string = "ReconcileFailed" ) // +genclient diff --git a/pkg/duckbuilder/duck_builder.go b/pkg/duckbuilder/duck_builder.go index 0b61a4960..db0c21bc5 100644 --- a/pkg/duckbuilder/duck_builder.go +++ b/pkg/duckbuilder/duck_builder.go @@ -37,6 +37,11 @@ func (b *DuckBuilder) Ready() bool { (b.Generation == b.Status.ObservedGeneration) } +func (b *DuckBuilder) UpToDate() bool { + return b.Status.GetCondition(buildapi.ConditionUpToDate).IsTrue() && + (b.Generation == b.Status.ObservedGeneration) +} + func (b *DuckBuilder) BuildBuilderSpec() corev1alpha1.BuildBuilderSpec { return corev1alpha1.BuildBuilderSpec{ Image: b.Status.LatestImage, diff --git a/pkg/duckbuilder/duck_builder_test.go b/pkg/duckbuilder/duck_builder_test.go index 80b6b8f33..8426eb625 100644 --- a/pkg/duckbuilder/duck_builder_test.go +++ b/pkg/duckbuilder/duck_builder_test.go @@ -36,6 +36,10 @@ func testDuckBuilder(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: corev1alpha1.BuildpackMetadataList{ @@ -81,6 +85,35 @@ func testDuckBuilder(t *testing.T, when spec.G, it spec.S) { }) }) + when("UpToDate", func() { + + it("ready when UpToDate condition is true", func() { + require.True(t, duckBuilder.UpToDate()) + }) + + it("not ready without conditions", func() { + duckBuilder.Status.Conditions = nil + + require.False(t, duckBuilder.UpToDate()) + }) + + it("not ready when not ready", func() { + duckBuilder.Status.Conditions = corev1alpha1.Conditions{ + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + }, + } + + require.False(t, duckBuilder.UpToDate()) + }) + + it("not UpToDate when generation does not match observed generation", func() { + duckBuilder.Generation = duckBuilder.Status.ObservedGeneration + 1 + + require.False(t, duckBuilder.UpToDate()) + }) + }) it("BuildBuilderSpec provides latest image and pull secrets", func() { require.Equal(t, corev1alpha1.BuildBuilderSpec{ Image: "some/builder@sha256:12345678", diff --git a/pkg/reconciler/builder/builder_test.go b/pkg/reconciler/builder/builder_test.go index 3db00fcf5..19a9366d1 100644 --- a/pkg/reconciler/builder/builder_test.go +++ b/pkg/reconciler/builder/builder_test.go @@ -245,6 +245,10 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: []corev1alpha1.BuildpackMetadata{ @@ -320,6 +324,10 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: []corev1alpha1.BuildpackMetadata{}, @@ -383,6 +391,10 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: []corev1alpha1.BuildpackMetadata{ @@ -436,6 +448,13 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Reason: buildapi.NoLatestImageReason, + Message: buildapi.NoLatestImageMessage, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.ReconcileFailedReason, Message: "create error", }, }, @@ -491,6 +510,13 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Reason: buildapi.NoLatestImageReason, + Message: buildapi.NoLatestImageMessage, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.ReconcileFailedReason, Message: "Error: clusterstack 'some-stack' is not ready", }, }, @@ -526,6 +552,13 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Reason: buildapi.NoLatestImageReason, + Message: buildapi.NoLatestImageMessage, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.ReconcileFailedReason, Message: `clusterstore.kpack.io "some-store" not found`, }, }, @@ -561,6 +594,13 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Reason: buildapi.NoLatestImageReason, + Message: buildapi.NoLatestImageMessage, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.ReconcileFailedReason, Message: `clusterstack.kpack.io "some-stack" not found`, }, }, diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go index 42f6d48e7..64e0c1cec 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go @@ -136,7 +136,6 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { APIVersion: "kpack.io/v1alpha2", }, } - builder := &buildapi.ClusterBuilder{ ObjectMeta: metav1.ObjectMeta{ Name: builderName, @@ -233,6 +232,10 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: []corev1alpha1.BuildpackMetadata{ @@ -308,6 +311,10 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: []corev1alpha1.BuildpackMetadata{}, @@ -363,6 +370,10 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, BuilderMetadata: []corev1alpha1.BuildpackMetadata{ @@ -405,7 +416,14 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Message: buildapi.NoLatestImageMessage, + Reason: buildapi.NoLatestImageReason, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, Message: "create error", + Reason: buildapi.ReconcileFailedReason, }, }, }, @@ -472,7 +490,14 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Message: buildapi.NoLatestImageMessage, + Reason: buildapi.NoLatestImageReason, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, Message: "stack some-stack is not ready", + Reason: buildapi.ReconcileFailedReason, }, }, }, @@ -510,6 +535,13 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Message: buildapi.NoLatestImageMessage, + Reason: buildapi.NoLatestImageReason, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.ReconcileFailedReason, Message: `clusterstore.kpack.io "some-store" not found`, }, }, @@ -548,6 +580,13 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionFalse, + Message: buildapi.NoLatestImageMessage, + Reason: buildapi.NoLatestImageReason, + }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.ReconcileFailedReason, Message: `clusterstack.kpack.io "some-stack" not found`, }, }, diff --git a/test/cosign_e2e_test.go b/test/cosign_e2e_test.go index 665d9b1e8..9ad5a8567 100644 --- a/test/cosign_e2e_test.go +++ b/test/cosign_e2e_test.go @@ -8,14 +8,15 @@ import ( cosigntesting "github.com/pivotal/kpack/pkg/cosign/testing" cosignutil "github.com/pivotal/kpack/pkg/cosign/util" - buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" - corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" "github.com/sclevine/spec" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" + corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" ) func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { @@ -274,7 +275,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, builder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, builder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, builder) updatedBuilder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Get(ctx, builderName, metav1.GetOptions{}) require.NoError(t, err) @@ -390,7 +392,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, builder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, builder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, builder) updatedBuilder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Get(ctx, builderName, metav1.GetOptions{}) require.NoError(t, err) @@ -514,7 +517,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, builder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, builder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, builder) updatedBuilder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Get(ctx, builderName, metav1.GetOptions{}) require.NoError(t, err) @@ -641,7 +645,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilFailed(t, ctx, clients, expectedErrorMessage, builder) + waitUntilFailed(t, ctx, clients, buildapi.ConditionUpToDate, expectedErrorMessage, builder) + waitUntilFailed(t, ctx, clients, corev1alpha1.ConditionReady, buildapi.NoLatestImageMessage, builder) updatedBuilder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Get(ctx, builderName, metav1.GetOptions{}) require.NoError(t, err) @@ -649,7 +654,10 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { readyConditionBuilder := updatedBuilder.Status.GetCondition(corev1alpha1.ConditionReady) require.False(t, readyConditionBuilder.IsTrue()) - require.Contains(t, readyConditionBuilder.Message, expectedErrorMessage) + + upToDateConditionBuilder := updatedBuilder.Status.GetCondition(buildapi.ConditionUpToDate) + require.False(t, upToDateConditionBuilder.IsTrue()) + require.Contains(t, upToDateConditionBuilder.Message, expectedErrorMessage) }) it("Signs a ClusterBuilder image successfully when the key is not password-protected", func() { @@ -751,7 +759,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, clusterBuilder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, clusterBuilder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, clusterBuilder) updatedBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Get(ctx, clusterBuilderName, metav1.GetOptions{}) require.NoError(t, err) @@ -864,7 +873,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, clusterBuilder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, clusterBuilder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, clusterBuilder) updatedBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Get(ctx, clusterBuilderName, metav1.GetOptions{}) require.NoError(t, err) @@ -986,7 +996,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, clusterBuilder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, clusterBuilder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, clusterBuilder) updatedBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Get(ctx, clusterBuilderName, metav1.GetOptions{}) require.NoError(t, err) @@ -1110,7 +1121,8 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilFailed(t, ctx, clients, expectedErrorMessage, clusterBuilder) + waitUntilFailed(t, ctx, clients, buildapi.ConditionUpToDate, expectedErrorMessage, clusterBuilder) + waitUntilFailed(t, ctx, clients, corev1alpha1.ConditionReady, buildapi.NoLatestImageMessage, clusterBuilder) updatedBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Get(ctx, clusterBuilderName, metav1.GetOptions{}) require.NoError(t, err) @@ -1118,6 +1130,9 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { readyConditionBuilder := updatedBuilder.Status.GetCondition(corev1alpha1.ConditionReady) require.False(t, readyConditionBuilder.IsTrue()) - require.Contains(t, readyConditionBuilder.Message, expectedErrorMessage) + + upToDateConditionBuilder := updatedBuilder.Status.GetCondition(buildapi.ConditionUpToDate) + require.False(t, upToDateConditionBuilder.IsTrue()) + require.Contains(t, upToDateConditionBuilder.Message, expectedErrorMessage) }) } diff --git a/test/execute_build_test.go b/test/execute_build_test.go index 55120506c..e404c75b3 100644 --- a/test/execute_build_test.go +++ b/test/execute_build_test.go @@ -453,7 +453,8 @@ func testCreateImage(t *testing.T, _ spec.G, it spec.S) { }, metav1.CreateOptions{}) require.NoError(t, err) - waitUntilReady(t, ctx, clients, builder, clusterBuilder) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, builder, clusterBuilder) + waitUntilCondition(t, ctx, clients, buildapi.ConditionUpToDate, builder, clusterBuilder) }) it("builds and rebases git, blob, and registry images from unauthenticated sources", func() { @@ -639,7 +640,7 @@ func readNamespaceLabelsFromEnv() map[string]string { return labelsToSet } -func waitUntilReady(t *testing.T, ctx context.Context, clients *clients, objects ...kmeta.OwnerRefable) { +func waitUntilCondition(t *testing.T, ctx context.Context, clients *clients, condition corev1alpha1.ConditionType, objects ...kmeta.OwnerRefable) { for _, ob := range objects { namespace := ob.GetObjectMeta().GetNamespace() name := ob.GetObjectMeta().GetName() @@ -653,12 +654,12 @@ func waitUntilReady(t *testing.T, ctx context.Context, clients *clients, objects err = duck.FromUnstructured(unstructured, kResource) require.NoError(t, err) - return kResource.Status.GetCondition(apis.ConditionReady).IsTrue() + return kResource.Status.GetCondition(apis.ConditionType(condition)).IsTrue() }, 1*time.Second, 8*time.Minute) } } -func waitUntilFailed(t *testing.T, ctx context.Context, clients *clients, expectedMessage string, objects ...kmeta.OwnerRefable) { +func waitUntilFailed(t *testing.T, ctx context.Context, clients *clients, condition corev1alpha1.ConditionType, expectedMessage string, objects ...kmeta.OwnerRefable) { for _, ob := range objects { namespace := ob.GetObjectMeta().GetNamespace() name := ob.GetObjectMeta().GetName() @@ -672,7 +673,7 @@ func waitUntilFailed(t *testing.T, ctx context.Context, clients *clients, expect err = duck.FromUnstructured(unstructured, kResource) require.NoError(t, err) - condition := kResource.Status.GetCondition(apis.ConditionReady) + condition := kResource.Status.GetCondition(apis.ConditionType(condition)) return condition.IsFalse() && "" != condition.Message && strings.Contains(condition.Message, expectedMessage) }, 1*time.Second, 8*time.Minute) } @@ -689,7 +690,7 @@ func validateImageCreate(t *testing.T, clients *clients, image *buildapi.Image, }() t.Logf("Waiting for image '%s' to be created", image.Name) - waitUntilReady(t, ctx, clients, image) + waitUntilCondition(t, ctx, clients, corev1alpha1.ConditionReady, image) registryClient := ®istry.Client{} _, identifier, err := registryClient.Fetch(authn.DefaultKeychain, image.Spec.Tag) From cea5d68d7a64bec5ca61417a460ce4a16292a5b8 Mon Sep 17 00:00:00 2001 From: Tom Kennedy Date: Fri, 3 Nov 2023 13:54:04 -0400 Subject: [PATCH 2/3] Add docs for builder status conditions Signed-off-by: Tom Kennedy --- docs/builders.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/builders.md b/docs/builders.md index c407d0129..47cca0071 100644 --- a/docs/builders.md +++ b/docs/builders.md @@ -161,3 +161,13 @@ version) in the following order: `paketo-buildpacks/gradle` is a sub-buildpack of `paketo-buildpacks/java`) 1. As a sub-buildpack of any ClusterBuildpacks 1. As a sub-buildpack in the ClusterStore specified in the Builder spec. + +### Builder Status Conditions + +Builders and ClusterBuilders have two Conditions that represent the overall status of the Builder. + +The 'Ready' condition is used to show that the Builder is able to be used in Builds + +The 'UpToDate' condition indicates whether the most recent reconcile of the Builder +was successful. When this condition is false, that means that the Builder may not have the +latest Stack or Buildpacks due to ongoing reconcile failures. \ No newline at end of file From 1a042b8b416698ea69a6900c323995174a9441d1 Mon Sep 17 00:00:00 2001 From: Tom Kennedy Date: Sat, 11 Nov 2023 09:03:01 -0500 Subject: [PATCH 3/3] Add Builder UpToDate condition to image status Signed-off-by: Tom Kennedy --- pkg/apis/build/v1alpha2/builder_resource.go | 1 + pkg/apis/build/v1alpha2/image_builds_test.go | 5 + pkg/apis/build/v1alpha2/image_lifecycle.go | 8 +- pkg/apis/build/v1alpha2/image_types.go | 1 + pkg/reconciler/image/build_required_test.go | 5 + pkg/reconciler/image/image_test.go | 199 ++++++++++++++++++- pkg/reconciler/image/reconcile_build.go | 31 ++- 7 files changed, 236 insertions(+), 14 deletions(-) diff --git a/pkg/apis/build/v1alpha2/builder_resource.go b/pkg/apis/build/v1alpha2/builder_resource.go index ea280d683..f34c64abb 100644 --- a/pkg/apis/build/v1alpha2/builder_resource.go +++ b/pkg/apis/build/v1alpha2/builder_resource.go @@ -7,6 +7,7 @@ type BuilderResource interface { GetNamespace() string BuildBuilderSpec() corev1alpha1.BuildBuilderSpec Ready() bool + UpToDate() bool BuildpackMetadata() corev1alpha1.BuildpackMetadataList RunImage() string GetKind() string diff --git a/pkg/apis/build/v1alpha2/image_builds_test.go b/pkg/apis/build/v1alpha2/image_builds_test.go index c33233df2..d4dd42924 100644 --- a/pkg/apis/build/v1alpha2/image_builds_test.go +++ b/pkg/apis/build/v1alpha2/image_builds_test.go @@ -372,6 +372,7 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) { type TestBuilderResource struct { BuilderReady bool + BuilderUpToDate bool BuilderMetadata []corev1alpha1.BuildpackMetadata ImagePullSecrets []corev1.LocalObjectReference Kind string @@ -396,6 +397,10 @@ func (t TestBuilderResource) Ready() bool { return t.BuilderReady } +func (t TestBuilderResource) UpToDate() bool { + return t.BuilderUpToDate +} + func (t TestBuilderResource) BuildpackMetadata() corev1alpha1.BuildpackMetadataList { return t.BuilderMetadata } diff --git a/pkg/apis/build/v1alpha2/image_lifecycle.go b/pkg/apis/build/v1alpha2/image_lifecycle.go index f86686c67..6cda5f05c 100644 --- a/pkg/apis/build/v1alpha2/image_lifecycle.go +++ b/pkg/apis/build/v1alpha2/image_lifecycle.go @@ -10,9 +10,11 @@ import ( ) const ( - BuilderNotFound = "BuilderNotFound" - BuilderNotReady = "BuilderNotReady" - BuilderReady = "BuilderReady" + BuilderNotFound = "BuilderNotFound" + BuilderNotReady = "BuilderNotReady" + BuilderReady = "BuilderReady" + BuilderNotUpToDate = "BuilderNotUpToDate" + BuilderUpToDate = "BuilderUpToDate" ) func (im *Image) BuilderNotFound() corev1alpha1.Conditions { diff --git a/pkg/apis/build/v1alpha2/image_types.go b/pkg/apis/build/v1alpha2/image_types.go index f8553df62..cb488f535 100644 --- a/pkg/apis/build/v1alpha2/image_types.go +++ b/pkg/apis/build/v1alpha2/image_types.go @@ -136,3 +136,4 @@ func (i *Image) NamespacedName() types.NamespacedName { } const ConditionBuilderReady corev1alpha1.ConditionType = "BuilderReady" +const ConditionBuilderUpToDate corev1alpha1.ConditionType = "BuilderUpToDate" diff --git a/pkg/reconciler/image/build_required_test.go b/pkg/reconciler/image/build_required_test.go index 617c44eed..c39444c16 100644 --- a/pkg/reconciler/image/build_required_test.go +++ b/pkg/reconciler/image/build_required_test.go @@ -800,6 +800,7 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) { type TestBuilderResource struct { BuilderReady bool + BuilderUpToDate bool BuilderMetadata []corev1alpha1.BuildpackMetadata ImagePullSecrets []corev1.LocalObjectReference LatestImage string @@ -820,6 +821,10 @@ func (t TestBuilderResource) Ready() bool { return t.BuilderReady } +func (t TestBuilderResource) UpToDate() bool { + return t.BuilderUpToDate +} + func (t TestBuilderResource) BuildpackMetadata() corev1alpha1.BuildpackMetadataList { return t.BuilderMetadata } diff --git a/pkg/reconciler/image/image_test.go b/pkg/reconciler/image/image_test.go index 0343e53ee..ea46f3344 100644 --- a/pkg/reconciler/image/image_test.go +++ b/pkg/reconciler/image/image_test.go @@ -75,7 +75,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { return r, actionRecorderList, eventList }) - imageWithBuilder := &buildapi.Image{ +imageWithBuilder := &buildapi.Image{ ObjectMeta: metav1.ObjectMeta{ Name: imageName, Namespace: namespace, @@ -172,6 +172,10 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, }, @@ -208,6 +212,10 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, }, @@ -778,11 +786,18 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Key: key, Objects: []runtime.Object{ imageWithBuilder, - builderWithCondition(builder, corev1alpha1.Condition{ - Type: corev1alpha1.ConditionReady, - Status: corev1.ConditionFalse, - Message: "something went wrong", - }), + builderWithCondition( + builder, + corev1alpha1.Condition{ + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionFalse, + Message: "something went wrong", + }, + corev1alpha1.Condition{ + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + }, + ), resolvedSourceResolver(imageWithBuilder), }, WantErr: false, @@ -807,6 +822,129 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Reason: buildapi.BuilderNotReady, Message: "Error: Builder 'builder-name' is not ready in namespace 'some-namespace'; Message: something went wrong", }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.BuilderNotUpToDate, + Message: "Builder is not up to date. The latest stack and buildpacks may not be in use.", + }, + }, + }, + }, + }, + }, + }, + }) + }) + + it("includes builder UpToDate condition in status", func() { + sourceResolver := resolvedSourceResolver(imageWithBuilder) + rt.Test(rtesting.TableRow{ + Key: key, + Objects: []runtime.Object{ + imageWithBuilder, + builderWithCondition( + builder, + corev1alpha1.Condition{ + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionTrue, + }, + corev1alpha1.Condition{ + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionFalse, + Message: "Builder failed to reconcile", + Reason: buildapi.ReconcileFailedReason, + }, + ), + sourceResolver, + }, + WantErr: false, + WantCreates: []runtime.Object{ + &buildapi.Build{ + ObjectMeta: metav1.ObjectMeta{ + Name: imageName + "-build-1", + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + *kmeta.NewControllerRef(imageWithBuilder), + }, + Labels: map[string]string{ + buildapi.BuildNumberLabel: "1", + buildapi.ImageLabel: imageName, + buildapi.ImageGenerationLabel: generation(imageWithBuilder), + someLabelKey: someValueToPassThrough, + }, + Annotations: map[string]string{ + buildapi.BuilderNameAnnotation: builderName, + buildapi.BuilderKindAnnotation: buildapi.BuilderKind, + buildapi.BuildReasonAnnotation: buildapi.BuildReasonConfig, + buildapi.BuildChangesAnnotation: testhelpers.CompactJSON(` +[ + { + "reason": "CONFIG", + "old": { + "resources": {}, + "source": {} + }, + "new": { + "resources": {}, + "source": { + "git": { + "url": "https://some.git/url-resolved", + "revision": "1234567-resolved" + } + } + } + } +]`), + }, + }, + Spec: buildapi.BuildSpec{ + Tags: []string{imageWithBuilder.Spec.Tag}, + Builder: corev1alpha1.BuildBuilderSpec{ + Image: builder.Status.LatestImage, + }, + ServiceAccountName: imageWithBuilder.Spec.ServiceAccountName, + Cache: &buildapi.BuildCacheConfig{}, + RunImage: builderRunImage, + Source: corev1alpha1.SourceConfig{ + Git: &corev1alpha1.Git{ + URL: sourceResolver.Status.Source.Git.URL, + Revision: sourceResolver.Status.Source.Git.Revision, + }, + }, + }, + }, + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{ + { + Object: &buildapi.Image{ + ObjectMeta: imageWithBuilder.ObjectMeta, + Spec: imageWithBuilder.Spec, + Status: buildapi.ImageStatus{ + LatestBuildRef: "image-name-build-1", + LatestBuildImageGeneration: 1, + BuildCounter: 1, + LatestBuildReason: "CONFIG", + Status: corev1alpha1.Status{ + ObservedGeneration: originalGeneration, + Conditions: corev1alpha1.Conditions{ + { + Type: corev1alpha1.ConditionReady, + Status: corev1.ConditionUnknown, + Reason: image.BuildRunningReason, + Message: "Build 'image-name-build-1' is executing", + }, + { + Type: buildapi.ConditionBuilderReady, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderReady, + }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.BuilderNotUpToDate, + Message: "Builder is not up to date. The latest stack and buildpacks may not be in use.", + }, }, }, }, @@ -1587,6 +1725,10 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, LatestImage: updatedBuilderImage, @@ -1757,6 +1899,10 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Type: corev1alpha1.ConditionReady, Status: corev1.ConditionTrue, }, + { + Type: buildapi.ConditionUpToDate, + Status: corev1.ConditionTrue, + }, }, }, LatestImage: updatedBuilderImage, @@ -2110,6 +2256,11 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, }, }, LatestBuildRef: "image-name-build-1", @@ -2222,6 +2373,11 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, }, }, LatestBuildRef: "image-name-build-1", @@ -2309,6 +2465,12 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Reason: buildapi.BuilderNotReady, Message: "Error: Builder 'builder-name' is not ready in namespace 'some-namespace'", }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.BuilderNotUpToDate, + Message: "Builder is not up to date. The latest stack and buildpacks may not be in use.", + }, }, }, LatestBuildRef: "image-name-build-1", @@ -2394,6 +2556,11 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, }, }, LatestBuildRef: "image-name-build-1", @@ -2613,6 +2780,11 @@ func conditionReadyUnknown() corev1alpha1.Conditions { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, } } @@ -2629,6 +2801,11 @@ func conditionBuildExecuting(buildName string) corev1alpha1.Conditions { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, } } @@ -2644,6 +2821,11 @@ func conditionReady() corev1alpha1.Conditions { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, } } @@ -2660,5 +2842,10 @@ func conditionNotReady(failedBuild *buildapi.Image) corev1alpha1.Conditions { Status: corev1.ConditionTrue, Reason: buildapi.BuilderReady, }, + { + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + }, } } diff --git a/pkg/reconciler/image/reconcile_build.go b/pkg/reconciler/image/reconcile_build.go index caac7af46..9d8442864 100644 --- a/pkg/reconciler/image/reconcile_build.go +++ b/pkg/reconciler/image/reconcile_build.go @@ -19,6 +19,7 @@ const ( UnknownStateReason = "UnknownState" BuildFailedReason = "BuildFailed" UpToDateReason = "UpToDate" + NotUpToDateMessage = "Builder is not up to date. The latest stack and buildpacks may not be in use." ) func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image, latestBuild *buildapi.Build, sourceResolver *buildapi.SourceResolver, builder buildapi.BuilderResource, buildCacheName string) (buildapi.ImageStatus, error) { @@ -46,7 +47,7 @@ func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image, return buildapi.ImageStatus{ Status: corev1alpha1.Status{ - Conditions: scheduledBuildCondition(build), + Conditions: scheduledBuildCondition(build, builder), }, BuildCounter: nextBuildNumber, BuildCacheName: buildCacheName, @@ -106,7 +107,7 @@ func noScheduledBuild(buildNeeded corev1.ConditionStatus, builder buildapi.Build ready.Message = fmt.Sprintf("Error: Build '%s' in namespace '%s' failed: %s", build.Name, build.Namespace, defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded), "unknown error")) } - return corev1alpha1.Conditions{ready, builderCondition(builder)} + return corev1alpha1.Conditions{ready, builderReadyCondition(builder), builderUpToDateCondition(builder)} } @@ -127,7 +128,7 @@ func defaultMessageIfNil(condition *corev1alpha1.Condition, defaultMessage strin return condition.Message } -func builderCondition(builder buildapi.BuilderResource) corev1alpha1.Condition { +func builderReadyCondition(builder buildapi.BuilderResource) corev1alpha1.Condition { if !builder.Ready() { return corev1alpha1.Condition{ Type: buildapi.ConditionBuilderReady, @@ -145,6 +146,24 @@ func builderCondition(builder buildapi.BuilderResource) corev1alpha1.Condition { } } +func builderUpToDateCondition(builder buildapi.BuilderResource) corev1alpha1.Condition { + if !builder.UpToDate() { + return corev1alpha1.Condition{ + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionFalse, + Reason: buildapi.BuilderNotUpToDate, + Message: NotUpToDateMessage, + LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()}, + } + } + return corev1alpha1.Condition{ + Type: buildapi.ConditionBuilderUpToDate, + Status: corev1.ConditionTrue, + Reason: buildapi.BuilderUpToDate, + LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()}, + } +} + func builderError(builder buildapi.BuilderResource) string { errorMessage := fmt.Sprintf("Error: Builder '%s' is not ready in namespace '%s'", builder.GetName(), builder.GetNamespace()) @@ -155,7 +174,7 @@ func builderError(builder buildapi.BuilderResource) string { return errorMessage } -func scheduledBuildCondition(build *buildapi.Build) corev1alpha1.Conditions { +func scheduledBuildCondition(build *buildapi.Build, builder buildapi.BuilderResource) corev1alpha1.Conditions { return corev1alpha1.Conditions{ { Type: corev1alpha1.ConditionReady, @@ -170,6 +189,7 @@ func scheduledBuildCondition(build *buildapi.Build) corev1alpha1.Conditions { Reason: buildapi.BuilderReady, LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()}, }, + builderUpToDateCondition(builder), } } @@ -193,6 +213,7 @@ func buildRunningCondition(build *buildapi.Build, builder buildapi.BuilderResour Message: message, LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()}, }, - builderCondition(builder), + builderReadyCondition(builder), + builderUpToDateCondition(builder), } }