Skip to content

Commit 972a7e2

Browse files
Use builderReadyCondition.IsTrue() to check readiness
Previously we used `builderReadyCondition.IsFalse()` to check whether the builder is ready. However, `IsFalse` returns `false` when the condition is `nil` which is not what we want - when the ready condition is `nil` we should consider that the builder is not ready. Instead, we now use `builderReadyCondition.IsTrue()` which only returns `true` if the ready condition is set, and its status is `True`. We also change the way we check for builder ready timeout - we base the check on the build workload creation time instead of the builder ready status condition transition time as the condition itself might be nil. Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
1 parent 30973d7 commit 972a7e2

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

kpack-image-builder/controllers/buildworkload_controller.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,8 @@ func (r *BuildWorkloadReconciler) ReconcileResource(ctx context.Context, buildWo
209209
return ctrl.Result{}, ignoreDoNotRetryError(fmt.Errorf("failed getting builder readiness condition"))
210210
}
211211

212-
if builderReadyCondition.IsFalse() {
213-
if time.Since(builderReadyCondition.LastTransitionTime.Inner.Time) < r.builderReadinessTimeout {
212+
if !builderReadyCondition.IsTrue() {
213+
if time.Since(buildWorkload.CreationTimestamp.Time) < r.builderReadinessTimeout {
214214
log.Info("waiting for builder to be ready")
215215
return ctrl.Result{RequeueAfter: time.Second}, nil
216216
}

kpack-image-builder/controllers/buildworkload_controller_test.go

+44
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,50 @@ var _ = Describe("BuildWorkloadReconciler", func() {
815815
})
816816
})
817817

818+
When("the cluster builder readiness is not set", func() {
819+
BeforeEach(func() {
820+
Expect(k8s.Patch(ctx, adminClient, clusterBuilder, func() {
821+
clusterBuilder.Status.Conditions = corev1alpha1.Conditions{}
822+
})).To(Succeed())
823+
824+
buildWorkload = buildWorkloadObject(buildWorkloadGUID, namespaceGUID, source, env, services, reconcilerName, buildpacks)
825+
Expect(adminClient.Create(ctx, buildWorkload)).To(Succeed())
826+
})
827+
828+
It("sets the Succeeded condition to False", func() {
829+
Eventually(func(g Gomega) {
830+
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(buildWorkload), buildWorkload)).To(Succeed())
831+
g.Expect(mustHaveCondition(g, buildWorkload.Status.Conditions, "Succeeded").Status).To(Equal(metav1.ConditionFalse))
832+
g.Expect(mustHaveCondition(g, buildWorkload.Status.Conditions, "Succeeded").Reason).To(Equal("BuilderNotReady"))
833+
}).Should(Succeed())
834+
})
835+
})
836+
837+
When("the cluster builder readiness is unknown", func() {
838+
BeforeEach(func() {
839+
Expect(k8s.Patch(ctx, adminClient, clusterBuilder, func() {
840+
clusterBuilder.Status.Conditions = corev1alpha1.Conditions{
841+
{
842+
Type: corev1alpha1.ConditionType("Ready"),
843+
Status: corev1.ConditionStatus(metav1.ConditionUnknown),
844+
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
845+
},
846+
}
847+
})).To(Succeed())
848+
849+
buildWorkload = buildWorkloadObject(buildWorkloadGUID, namespaceGUID, source, env, services, reconcilerName, buildpacks)
850+
Expect(adminClient.Create(ctx, buildWorkload)).To(Succeed())
851+
})
852+
853+
It("sets the Succeeded condition to False", func() {
854+
Eventually(func(g Gomega) {
855+
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(buildWorkload), buildWorkload)).To(Succeed())
856+
g.Expect(mustHaveCondition(g, buildWorkload.Status.Conditions, "Succeeded").Status).To(Equal(metav1.ConditionFalse))
857+
g.Expect(mustHaveCondition(g, buildWorkload.Status.Conditions, "Succeeded").Reason).To(Equal("BuilderNotReady"))
858+
}).Should(Succeed())
859+
})
860+
})
861+
818862
When("multiple BuildWorkloads exist for the same app", func() {
819863
var buildWorkload2 *korifiv1alpha1.BuildWorkload
820864

0 commit comments

Comments
 (0)