From 4ecfc21f5c1f7dfd14a905c7e5e0926dceb535ee Mon Sep 17 00:00:00 2001 From: Dylan Orzel Date: Wed, 13 Nov 2024 11:48:32 -0700 Subject: [PATCH] Tests for Tolerations on Build and BuildRun objects Signed-off-by: Dylan Orzel --- pkg/reconciler/build/build_test.go | 18 ++++++- pkg/reconciler/buildrun/buildrun_test.go | 19 ++++++- .../buildrun/resources/taskrun_test.go | 26 +++++++++- test/integration/build_to_taskruns_test.go | 46 +++++++++++++++++ .../integration/buildruns_to_taskruns_test.go | 51 +++++++++++++++++++ test/v1beta1_samples/build_samples.go | 16 ++++++ test/v1beta1_samples/buildrun_samples.go | 15 ++++++ 7 files changed, 187 insertions(+), 4 deletions(-) diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go index c621010fc..da622b945 100644 --- a/pkg/reconciler/build/build_test.go +++ b/pkg/reconciler/build/build_test.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/utils/ptr" crc "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -621,7 +622,22 @@ var _ = Describe("Reconcile Build", func() { buildSample.Spec.NodeSelector = map[string]string{strings.Repeat("s", 64): "amd64"} buildSample.Spec.Output.PushSecret = nil - statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "name part must be no more than 63 characters") + statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "name part "+validation.MaxLenError(63)) + statusWriter.UpdateCalls(statusCall) + + _, err := reconciler.Reconcile(context.TODO(), request) + Expect(err).To(BeNil()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + }) + }) + + Context("when Tolerations is specified", func() { + It("should fail to validate when the Toleration is invalid", func() { + // set Toleration to be invalid + buildSample.Spec.Tolerations = []corev1.Toleration{{Key: strings.Repeat("s", 64), Operator: "Equal", Value: "test-value"}} + buildSample.Spec.Output.PushSecret = nil + + statusCall := ctl.StubFunc(corev1.ConditionFalse, build.TolerationNotValid, validation.MaxLenError(63)) statusWriter.UpdateCalls(statusCall) _, err := reconciler.Reconcile(context.TODO(), request) diff --git a/pkg/reconciler/buildrun/buildrun_test.go b/pkg/reconciler/buildrun/buildrun_test.go index ae3ef627b..18771f3a9 100644 --- a/pkg/reconciler/buildrun/buildrun_test.go +++ b/pkg/reconciler/buildrun/buildrun_test.go @@ -20,6 +20,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" knativeapi "knative.dev/pkg/apis" @@ -1635,9 +1636,23 @@ var _ = Describe("Reconcile BuildRun", func() { Context("when nodeSelector is specified", func() { It("fails when the nodeSelector is invalid", func() { // set nodeSelector to be invalid - buildSample.Spec.NodeSelector = map[string]string{strings.Repeat("s", 64): "amd64"} + buildRunSample.Spec.NodeSelector = map[string]string{strings.Repeat("s", 64): "amd64"} - statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "must be no more than 63 characters") + statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "name part "+validation.MaxLenError(63)) + statusWriter.UpdateCalls(statusCall) + + _, err := reconciler.Reconcile(context.TODO(), buildRunRequest) + Expect(err).To(BeNil()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + }) + }) + + Context("when Tolerations is specified", func() { + It("should fail to validate when the Toleration is invalid", func() { + // set Toleration to be invalid + buildRunSample.Spec.Tolerations = []corev1.Toleration{{Key: strings.Repeat("s", 64), Operator: "Equal", Value: "test-value"}} + + statusCall := ctl.StubFunc(corev1.ConditionFalse, build.TolerationNotValid, validation.MaxLenError(63)) statusWriter.UpdateCalls(statusCall) _, err := reconciler.Reconcile(context.TODO(), buildRunRequest) diff --git a/pkg/reconciler/buildrun/resources/taskrun_test.go b/pkg/reconciler/buildrun/resources/taskrun_test.go index 268be4865..d53e05580 100644 --- a/pkg/reconciler/buildrun/resources/taskrun_test.go +++ b/pkg/reconciler/buildrun/resources/taskrun_test.go @@ -635,7 +635,7 @@ var _ = Describe("GenerateTaskrun", func() { Context("when the build and buildrun both specify a nodeSelector", func() { BeforeEach(func() { - build, err = ctl.LoadBuildYAML([]byte(test.MinimalBuildRunWithNodeSelector)) + build, err = ctl.LoadBuildYAML([]byte(test.MinimalBuildWithNodeSelector)) Expect(err).To(BeNil()) buildRun, err = ctl.LoadBuildRunFromBytes([]byte(test.MinimalBuildRunWithNodeSelector)) @@ -654,5 +654,29 @@ var _ = Describe("GenerateTaskrun", func() { Expect(got.Spec.PodTemplate.NodeSelector).To(Equal(buildRun.Spec.NodeSelector)) }) }) + + Context("when the build and buildrun both specify a Toleration", func() { + BeforeEach(func() { + build, err = ctl.LoadBuildYAML([]byte(test.MinimalBuildWithToleration)) + Expect(err).To(BeNil()) + + buildRun, err = ctl.LoadBuildRunFromBytes([]byte(test.MinimalBuildRunWithToleration)) + Expect(err).To(BeNil()) + + buildStrategy, err = ctl.LoadBuildStrategyFromBytes([]byte(test.ClusterBuildStrategyNoOp)) + Expect(err).To(BeNil()) + }) + + JustBeforeEach(func() { + got, err = resources.GenerateTaskRun(config.NewDefaultConfig(), build, buildRun, serviceAccountName, buildStrategy) + Expect(err).To(BeNil()) + }) + + It("should give precedence to the Toleration values specified in the buildRun", func() { + Expect(got.Spec.PodTemplate.Tolerations[0].Key).To(Equal(buildRun.Spec.Tolerations[0].Key)) + Expect(got.Spec.PodTemplate.Tolerations[0].Operator).To(Equal(buildRun.Spec.Tolerations[0].Operator)) + Expect(got.Spec.PodTemplate.Tolerations[0].Value).To(Equal(buildRun.Spec.Tolerations[0].Value)) + }) + }) }) }) diff --git a/test/integration/build_to_taskruns_test.go b/test/integration/build_to_taskruns_test.go index 943c71153..ba8fa0382 100644 --- a/test/integration/build_to_taskruns_test.go +++ b/test/integration/build_to_taskruns_test.go @@ -246,4 +246,50 @@ var _ = Describe("Integration tests Build and TaskRun", func() { }) }) }) + + Context("when a build with Tolerations is defined", func() { + BeforeEach(func() { + buildSample = []byte(test.MinimalBuildWithToleration) + buildRunSample = []byte(test.MinimalBuildRun) + }) + + Context("when the TaskRun is created", func() { + It("should have the Tolerations specified in the PodTemplate", func() { + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + Expect(*buildObject.Status.Message).To(Equal(v1beta1.AllValidationsSucceeded)) + Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionTrue)) + Expect(*buildObject.Status.Reason).To(Equal(v1beta1.SucceedStatus)) + + Expect(tb.CreateBR(buildRunObject)).To(BeNil()) + + _, err = tb.GetBRTillStartTime(buildRunObject.Name) + Expect(err).To(BeNil()) + + tr, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + Expect(buildObject.Spec.Tolerations[0].Key).To(Equal(tr.Spec.PodTemplate.Tolerations[0].Key)) + Expect(buildObject.Spec.Tolerations[0].Operator).To(Equal(tr.Spec.PodTemplate.Tolerations[0].Operator)) + Expect(buildObject.Spec.Tolerations[0].Value).To(Equal(tr.Spec.PodTemplate.Tolerations[0].Value)) + Expect(tr.Spec.PodTemplate.Tolerations[0].Effect).To(Equal(corev1.TaintEffectNoSchedule)) + Expect(tr.Spec.PodTemplate.Tolerations[0].TolerationSeconds).To(Equal(corev1.Toleration{}.TolerationSeconds)) + }) + }) + + Context("when the Toleration is invalid", func() { + It("fails the build with a proper error in Reason", func() { + // set Toleration Key to be invalid + buildObject.Spec.Tolerations[0].Key = strings.Repeat("s", 64) + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + + Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse)) + Expect(*buildObject.Status.Reason).To(Equal(v1beta1.TolerationNotValid)) + }) + }) + }) }) diff --git a/test/integration/buildruns_to_taskruns_test.go b/test/integration/buildruns_to_taskruns_test.go index 59dc9abe5..bdb1f8b34 100644 --- a/test/integration/buildruns_to_taskruns_test.go +++ b/test/integration/buildruns_to_taskruns_test.go @@ -21,6 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" ) @@ -582,4 +583,54 @@ var _ = Describe("Integration tests BuildRuns and TaskRuns", func() { }) }) }) + + Context("when a buildrun is created with a Toleration defined", func() { + BeforeEach(func() { + buildSample = []byte(test.MinimalBuild) + buildRunSample = []byte(test.MinimalBuildRunWithToleration) + }) + + Context("when the taskrun is created", func() { + It("should have the Toleration specified in the PodTemplate", func() { + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + + Expect(tb.CreateBR(buildRunObject)).To(BeNil()) + + br, err := tb.GetBRTillCompletion(buildRunObject.Name) + Expect(err).To(BeNil()) + + tr, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + Expect(br.Spec.Tolerations[0].Key).To(Equal(tr.Spec.PodTemplate.Tolerations[0].Key)) + Expect(br.Spec.Tolerations[0].Operator).To(Equal(tr.Spec.PodTemplate.Tolerations[0].Operator)) + Expect(br.Spec.Tolerations[0].Value).To(Equal(tr.Spec.PodTemplate.Tolerations[0].Value)) + Expect(tr.Spec.PodTemplate.Tolerations[0].Effect).To(Equal(corev1.TaintEffectNoSchedule)) + Expect(tr.Spec.PodTemplate.Tolerations[0].TolerationSeconds).To(Equal(corev1.Toleration{}.TolerationSeconds)) + }) + }) + + Context("when the Toleration is invalid", func() { + It("fails the buildrun with a proper error in Reason", func() { + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + + // set Toleration Key to be invalid + buildRunObject.Spec.Tolerations[0].Key = strings.Repeat("s", 64) + Expect(tb.CreateBR(buildRunObject)).To(BeNil()) + + br, err := tb.GetBRTillCompletion(buildRunObject.Name) + Expect(err).To(BeNil()) + + condition := br.Status.GetCondition(v1beta1.Succeeded) + Expect(condition.Status).To(Equal(corev1.ConditionFalse)) + Expect(condition.Reason).To(Equal("PodCreationFailed")) + Expect(condition.Message).To(ContainSubstring(validation.MaxLenError(63))) + }) + }) + }) }) diff --git a/test/v1beta1_samples/build_samples.go b/test/v1beta1_samples/build_samples.go index a46f917fe..eff97c325 100644 --- a/test/v1beta1_samples/build_samples.go +++ b/test/v1beta1_samples/build_samples.go @@ -565,6 +565,22 @@ spec: kubernetes.io/arch: amd64 ` +// MinimalBuildWithToleration defines a simple +// Build with a strategy, output, and a Toleration specified +const MinimalBuildWithToleration = ` +apiVersion: shipwright.io/v1beta1 +kind: Build +spec: + strategy: + kind: ClusterBuildStrategy + output: + image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app + tolerations: + - key: "build-test-key" + operator: "Equal" + value: "build-test-value" +` + // BuildWithUndefinedParameter defines a param that was not declared under the // strategy parameters const BuildWithUndefinedParam = ` diff --git a/test/v1beta1_samples/buildrun_samples.go b/test/v1beta1_samples/buildrun_samples.go index 7e5503358..cb728e0dc 100644 --- a/test/v1beta1_samples/buildrun_samples.go +++ b/test/v1beta1_samples/buildrun_samples.go @@ -240,6 +240,21 @@ spec: kubernetes.io/arch: amd64 ` +// MinimalBuildRunWithToleration defines a minimal BuildRun +// with a reference to a not existing Build, +// and a Toleration specified +const MinimalBuildRunWithToleration = ` +apiVersion: shipwright.io/v1beta1 +kind: BuildRun +spec: + build: + name: foobar + tolerations: + - key: "buildrun-test-key" + operator: "Equal" + value: "buildrun-test-value" +` + // MinimalBuildRunWithVulnerabilityScan defines a BuildRun with // an override for the Build Output const MinimalBuildRunWithVulnerabilityScan = `