From f9d54c0d537f53fa38d622c639add0108c890979 Mon Sep 17 00:00:00 2001 From: Tom Kennedy Date: Sat, 11 Nov 2023 09:03:01 -0500 Subject: [PATCH] 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/duckbuilder/duck_builder.go | 5 + pkg/reconciler/image/build_required_test.go | 5 + pkg/reconciler/image/image_test.go | 199 ++++++++++++++++++- pkg/reconciler/image/reconcile_build.go | 31 ++- 8 files changed, 241 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/duckbuilder/duck_builder.go b/pkg/duckbuilder/duck_builder.go index 0b61a4960..59aa2f01d 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/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), } }