From dd9194a671fcf87a5c2468add0ae38a511a86f32 Mon Sep 17 00:00:00 2001 From: Dylan Orzel Date: Tue, 24 Sep 2024 13:18:32 -0600 Subject: [PATCH] Tests for NodeSelectors on Build and BuildRun objects --- pkg/reconciler/build/build_test.go | 15 +++++++ pkg/reconciler/buildrun/buildrun_test.go | 14 ++++++ .../buildrun/resources/taskrun_test.go | 22 +++++++++ test/integration/build_to_taskruns_test.go | 45 +++++++++++++++++++ .../integration/buildruns_to_taskruns_test.go | 42 +++++++++++++++++ test/v1beta1_samples/build_samples.go | 40 ++++++++++++++++- test/v1beta1_samples/buildrun_samples.go | 27 +++++++++++ 7 files changed, 203 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go index 02b70bb15..4d50e71aa 100644 --- a/pkg/reconciler/build/build_test.go +++ b/pkg/reconciler/build/build_test.go @@ -7,6 +7,7 @@ package build_test import ( "context" "fmt" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -613,5 +614,19 @@ var _ = Describe("Reconcile Build", func() { Expect(err).ToNot(HaveOccurred()) }) }) + + Context("when nodeSelector is specified", func() { + It("should fail to validate when the nodeSelector is invalid", func() { + // set nodeSelector to be invalid + buildSample.Spec.NodeSelector = map[string]string{strings.Repeat("s", 64): "amd64"} + + statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "must be no more than 63 characters") + statusWriter.UpdateCalls(statusCall) + + _, err := reconciler.Reconcile(context.TODO(), request) + Expect(err).To(BeNil()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + }) + }) }) }) diff --git a/pkg/reconciler/buildrun/buildrun_test.go b/pkg/reconciler/buildrun/buildrun_test.go index 8d7246679..98458abaa 100644 --- a/pkg/reconciler/buildrun/buildrun_test.go +++ b/pkg/reconciler/buildrun/buildrun_test.go @@ -1631,5 +1631,19 @@ var _ = Describe("Reconcile BuildRun", func() { Expect(statusWriter.UpdateCallCount()).To(Equal(1)) }) }) + + 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"} + + statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "must be no more than 63 characters") + statusWriter.UpdateCalls(statusCall) + + _, err := reconciler.Reconcile(context.TODO(), buildRunRequest) + Expect(err).To(BeNil()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + }) + }) }) }) diff --git a/pkg/reconciler/buildrun/resources/taskrun_test.go b/pkg/reconciler/buildrun/resources/taskrun_test.go index bf0709377..d134ca602 100644 --- a/pkg/reconciler/buildrun/resources/taskrun_test.go +++ b/pkg/reconciler/buildrun/resources/taskrun_test.go @@ -632,5 +632,27 @@ var _ = Describe("GenerateTaskrun", func() { Expect(paramOutputImageFound).To(BeTrue()) }) }) + + Context("when the build and buildrun both specify a nodeSelector in the PodTemplate", func() { + BeforeEach(func() { + build, err = ctl.LoadBuildYAML([]byte(test.MinimalBuildRunWithNodeSelector)) + Expect(err).To(BeNil()) + + buildRun, err = ctl.LoadBuildRunFromBytes([]byte(test.MinimalBuildRunWithNodeSelector)) + 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 nodeSelector specified in the buildRun", func() { + Expect(got.Spec.PodTemplate.NodeSelector).To(Equal(buildRun.Spec.NodeSelector)) + }) + }) }) }) diff --git a/test/integration/build_to_taskruns_test.go b/test/integration/build_to_taskruns_test.go index ee3b43ae9..a1fffdd27 100644 --- a/test/integration/build_to_taskruns_test.go +++ b/test/integration/build_to_taskruns_test.go @@ -5,11 +5,14 @@ package integration_test import ( + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" "github.com/shipwright-io/build/pkg/apis/build/v1beta1" + "github.com/shipwright-io/build/pkg/reconciler/buildrun/resources" utils "github.com/shipwright-io/build/test/utils/v1beta1" test "github.com/shipwright-io/build/test/v1beta1_samples" ) @@ -202,4 +205,46 @@ var _ = Describe("Integration tests Build and TaskRun", func() { }) }) }) + + Context("when a build with nodeSelector is defined", func() { + BeforeEach(func() { + buildSample = []byte(test.MinimalBuildahBuildWithNodeSelector) + buildRunSample = []byte(test.MinimalBuildahBuildRun) + }) + + Context("when the TaskRun is created", func() { + It("should have the nodeSelector 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()) + + tr, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + Expect(buildObject.Spec.NodeSelector).To(Equal(tr.Spec.PodTemplate.NodeSelector)) + }) + }) + + Context("when the nodeSelector is invalid", func() { + It("fails the build with a proper error in Reason", func() { + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + // set nodeSelector label to be invalid + buildRunObject.Spec.NodeSelector = map[string]string{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(resources.ConditionBuildRegistrationFailed)) + Expect(buildObject.Status.Reason).To(Equal(v1beta1.NodeSelectorNotValid)) + }) + }) + }) }) diff --git a/test/integration/buildruns_to_taskruns_test.go b/test/integration/buildruns_to_taskruns_test.go index ecae76947..65e188465 100644 --- a/test/integration/buildruns_to_taskruns_test.go +++ b/test/integration/buildruns_to_taskruns_test.go @@ -536,4 +536,46 @@ var _ = Describe("Integration tests BuildRuns and TaskRuns", func() { Expect(err).To(HaveOccurred()) }) }) + + Context("when a buildrun is created with a nodeSelector defined", func() { + BeforeEach(func() { + buildSample = []byte(test.MinimalBuildahBuild) + buildRunSample = []byte(test.MinimalBuildahBuildRunWithNodeSelector) + }) + + Context("when the taskrun is created", func() { + It("should have the nodeSelector 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()) + + tr, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + Expect(buildObject.Spec.NodeSelector).To(Equal(tr.Spec.PodTemplate.NodeSelector)) + }) + }) + + Context("when the nodeSelector 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 nodeSelector label to be invalid + buildRunObject.Spec.NodeSelector = map[string]string{strings.Repeat("s", 64): "amd64"} + 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(resources.ConditionBuildRegistrationFailed)) + Expect(*buildObject.Status.Reason).To(Equal(v1beta1.NodeSelectorNotValid)) + }) + }) + }) }) diff --git a/test/v1beta1_samples/build_samples.go b/test/v1beta1_samples/build_samples.go index 325eee108..46a892c14 100644 --- a/test/v1beta1_samples/build_samples.go +++ b/test/v1beta1_samples/build_samples.go @@ -51,6 +51,28 @@ spec: value: Dockerfile ` +// MinimalBuildahBuildWithNodeSelector defines a simple +// Build with a source, strategy, and nodeSelector +const MinimalBuildahBuildWithNodeSelector = ` +apiVersion: shipwright.io/v1beta1 +kind: Build +metadata: + name: buildah +spec: + source: + type: Git + git: + url: "https://github.com/shipwright-io/sample-go" + strategy: + name: buildah + kind: ClusterBuildStrategy + paramValues: + - name: dockerfile + value: Dockerfile + nodeSelector: + kubernetes.io/arch: amd64 +` + // BuildahBuildWithOutput defines a simple // Build with a source, strategy and output const BuildahBuildWithOutput = ` @@ -520,8 +542,8 @@ spec: image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app ` -// BuildWithRestrictedParam defines a Build using params that are reserved only -// for shipwright +// MinimalBuild defines a simple +// Build with a strategy output const MinimalBuild = ` apiVersion: shipwright.io/v1beta1 kind: Build @@ -532,6 +554,20 @@ spec: image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app ` +// MinimalBuildWithNodeSelector defines a simple +// Build with a strategy, output, and NodeSelector +const MinimalBuildWithNodeSelector = ` +apiVersion: shipwright.io/v1beta1 +kind: Build +spec: + strategy: + kind: ClusterBuildStrategy + output: + image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app + nodeSelector: + kubernetes.io/arch: amd64 +` + // 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 27ba30a0c..7e5503358 100644 --- a/test/v1beta1_samples/buildrun_samples.go +++ b/test/v1beta1_samples/buildrun_samples.go @@ -56,6 +56,20 @@ spec: name: buildah ` +// MinimalBuildahBuildRunWithNodeSelector defines a simple +// BuildRun with a referenced Build and nodeSelector +const MinimalBuildahBuildRunWithNodeSelector = ` +apiVersion: shipwright.io/v1beta1 +kind: BuildRun +metadata: + name: buildah-run +spec: + build: + name: buildah + nodeSelector: + kubernetes.io/arch: amd64 +` + // BuildahBuildRunWithSA defines a BuildRun // with a service-account const BuildahBuildRunWithSA = ` @@ -213,6 +227,19 @@ spec: name: foobar ` +// MinimalBuildRunWithNodeSelector defines a minimal BuildRun +// with a reference to a not existing Build, +// and a nodeSelector +const MinimalBuildRunWithNodeSelector = ` +apiVersion: shipwright.io/v1beta1 +kind: BuildRun +spec: + build: + name: foobar + nodeSelector: + kubernetes.io/arch: amd64 +` + // MinimalBuildRunWithVulnerabilityScan defines a BuildRun with // an override for the Build Output const MinimalBuildRunWithVulnerabilityScan = `