From 16f1dd9ee9d34221bd37cd5173e7a8386aa97a56 Mon Sep 17 00:00:00 2001 From: Enrique Encalada Date: Fri, 19 Mar 2021 14:32:51 +0100 Subject: [PATCH] Ensure we default to a namespaced strategy When a Build do not specify an strategy Kind, ensure we default to a namespaced scope one. This logic was already in placed, a previous commit removed and this commit added it again in a different function. Also, it should fix #657 --- docs/buildrun.md | 1 - pkg/reconciler/buildrun/buildrun.go | 17 +++++++++++------ pkg/reconciler/buildrun/buildrun_test.go | 9 +++++---- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/docs/buildrun.md b/docs/buildrun.md index 3e0bc1c5eb..d1d8c3e1aa 100644 --- a/docs/buildrun.md +++ b/docs/buildrun.md @@ -131,7 +131,6 @@ The following table illustrates the different states a BuildRun can have under i | False | Failed | Yes | The BuildRun failed in one of the steps. | | False | BuildRunTimeout | Yes | The BuildRun timed out. | | False | UnknownStrategyKind | Yes | The Build specified strategy Kind is unknown. (_options: ClusterBuildStrategy or BuildStrategy_) | -| False | StrategyKindIsMissing | Yes | The Build strategy Kind was not specified. (_options: ClusterBuildStrategy or BuildStrategy_) | | False | ClusterBuildStrategyNotFound | Yes | The referenced cluster strategy was not found in the cluster. | | False | BuildStrategyNotFound | Yes | The referenced namespaced strategy was not found in the cluster. | | False | SetOwnerReferenceFailed | Yes | Setting ownerreferences from the BuildRun to the related TaskRun failed. | diff --git a/pkg/reconciler/buildrun/buildrun.go b/pkg/reconciler/buildrun/buildrun.go index d2106a85cf..0743888bb5 100644 --- a/pkg/reconciler/buildrun/buildrun.go +++ b/pkg/reconciler/buildrun/buildrun.go @@ -372,14 +372,19 @@ func (r *ReconcileBuildRun) VerifyRequestName(ctx context.Context, request recon } func (r *ReconcileBuildRun) getReferencedStrategy(ctx context.Context, build *buildv1alpha1.Build, buildRun *buildv1alpha1.BuildRun) (strategy buildv1alpha1.BuilderStrategy, err error) { - if build.Spec.StrategyRef.Kind == nil { - err = fmt.Errorf("illegal state, strategy kind is unset") - if updateErr := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, fmt.Errorf("undefined strategy Kind").Error(), resources.ConditionStrategyKindIsMissing); updateErr != nil { - return nil, resources.HandleError("failed to get referenced strategy", err, updateErr) + if build.Spec.StrategyRef.Kind == nil { + // If the strategy Kind is not specified, we default to a namespaced-scope strategy + ctxlog.Info(ctx, "missing strategy Kind, defaulting to a namespaced-scope one", buildRun.Name, build.Name, namespace) + strategy, err = resources.RetrieveBuildStrategy(ctx, r.client, build) + if err != nil { + if apierrors.IsNotFound(err) { + if updateErr := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, err.Error(), resources.BuildStrategyNotFound); updateErr != nil { + return nil, resources.HandleError("failed to get referenced strategy", err, updateErr) + } + } } - - return nil, err + return strategy, err } switch *build.Spec.StrategyRef.Kind { diff --git a/pkg/reconciler/buildrun/buildrun_test.go b/pkg/reconciler/buildrun/buildrun_test.go index bfee9b3198..a0bfedfb3f 100644 --- a/pkg/reconciler/buildrun/buildrun_test.go +++ b/pkg/reconciler/buildrun/buildrun_test.go @@ -759,7 +759,7 @@ var _ = Describe("Reconcile BuildRun", func() { Expect(client.StatusCallCount()).To(Equal(2)) }) - It("fails on a TaskRun creation due to undefined buildStrategy kind", func() { + It("fails on a TaskRun creation due to an undefined default strategy kind", func() { // use a Build object that does not defines the strategy Kind field buildSample = ctl.BuildWithoutStrategyKind(buildName, strategyName) @@ -773,12 +773,13 @@ var _ = Describe("Reconcile BuildRun", func() { // Stub that asserts the BuildRun status fields when // Status updates for a BuildRun take place + // We fail here because the default strategy was not found statusCall := ctl.StubBuildRunStatus( - "undefined strategy Kind", + fmt.Sprintf(" \"%v\" not found", strategyName), emptyTaskRunName, build.Condition{ Type: build.Succeeded, - Reason: "StrategyKindIsMissing", + Reason: "BuildStrategyNotFound", Status: corev1.ConditionFalse, }, corev1.ConditionFalse, @@ -790,7 +791,7 @@ var _ = Describe("Reconcile BuildRun", func() { // we mark the BuildRun as Failed and do not reconcile again _, err := reconciler.Reconcile(buildRunRequest) Expect(err).ToNot(HaveOccurred()) - Expect(client.GetCallCount()).To(Equal(4)) + Expect(client.GetCallCount()).To(Equal(5)) Expect(client.StatusCallCount()).To(Equal(2)) })