From 43382a64c949670c92465a83782d5e14bfb35695 Mon Sep 17 00:00:00 2001 From: Dylan Orzel Date: Wed, 20 Nov 2024 15:06:49 -0700 Subject: [PATCH] Tests for Tolerations on Build and BuildRun objects Signed-off-by: Dylan Orzel --- .github/workflows/ci.yml | 30 ++++++++++- pkg/reconciler/build/build_test.go | 18 ++++++- pkg/reconciler/buildrun/buildrun_test.go | 19 ++++++- .../buildrun/resources/taskrun_test.go | 26 +++++++++- .../v1beta1/build_buildah_tolerations_cr.yaml | 20 +++++++ .../buildrun_buildah_tolerations_cr.yaml | 8 +++ test/e2e/v1beta1/e2e_test.go | 52 +++++++++++++++++++ test/integration/build_to_taskruns_test.go | 46 ++++++++++++++++ .../integration/buildruns_to_taskruns_test.go | 51 ++++++++++++++++++ test/kind/config_three_node.yaml | 17 ++++++ test/utils/v1beta1/nodes.go | 51 ++++++++++++++++++ test/v1beta1_samples/build_samples.go | 16 ++++++ test/v1beta1_samples/buildrun_samples.go | 15 ++++++ 13 files changed, 364 insertions(+), 5 deletions(-) create mode 100644 test/data/v1beta1/build_buildah_tolerations_cr.yaml create mode 100644 test/data/v1beta1/buildrun_buildah_tolerations_cr.yaml create mode 100644 test/kind/config_three_node.yaml create mode 100644 test/utils/v1beta1/nodes.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e3ffc550e..d877601f53 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -181,7 +181,7 @@ jobs: version: v0.24.0 node_image: kindest/node:${{ matrix.kubernetes }} cluster_name: kind - config: test/kind/config.yaml + config: test/kind/config_three_node.yaml wait: 120s - name: Verify kind cluster run: | @@ -202,6 +202,34 @@ jobs: exit 1 fi + + kubectl wait --for=condition=Ready=true node/kind-worker --timeout=60s + NODE_STATUS=$(kubectl get node kind-worker -o json | jq -r .'status.conditions[] | select(.type == "Ready") | .status') + if [ "${NODE_STATUS}" != "True" ]; then + echo "# Node is not ready:" + kubectl describe node kind-worker + + echo "# Pods:" + kubectl get pod -A + echo "# Events:" + kubectl get events -A + + exit 1 + fi + + kubectl wait --for=condition=Ready=true node/kind-worker2 --timeout=60s + NODE_STATUS=$(kubectl get node kind-worker2 -o json | jq -r .'status.conditions[] | select(.type == "Ready") | .status') + if [ "${NODE_STATUS}" != "True" ]; then + echo "# Node is not ready:" + kubectl describe node kind-worker2 + + echo "# Pods:" + kubectl get pod -A + echo "# Events:" + kubectl get events -A + + exit 1 + fi - name: Install Tekton env: TEKTON_VERSION: ${{ matrix.tekton }} diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go index c621010fcc..1d9a21db5d 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, "name part "+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 ae3ef627b5..18771f3a95 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 268be4865b..d53e05580c 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/data/v1beta1/build_buildah_tolerations_cr.yaml b/test/data/v1beta1/build_buildah_tolerations_cr.yaml new file mode 100644 index 0000000000..026014fda2 --- /dev/null +++ b/test/data/v1beta1/build_buildah_tolerations_cr.yaml @@ -0,0 +1,20 @@ +--- +apiVersion: shipwright.io/v1beta1 +kind: Build +metadata: + name: buildah-tolerations-build +spec: + source: + type: Git + git: + url: https://github.com/shipwright-io/sample-go + contextDir: docker-build + strategy: + name: buildah-shipwright-managed-push + kind: ClusterBuildStrategy + output: + image: image-registry.openshift-image-registry.svc:5000/build-examples/taxi-app + tolerations: + - key: "test-key" + value: "test-value" + operator: "Equal" \ No newline at end of file diff --git a/test/data/v1beta1/buildrun_buildah_tolerations_cr.yaml b/test/data/v1beta1/buildrun_buildah_tolerations_cr.yaml new file mode 100644 index 0000000000..d6571d03bf --- /dev/null +++ b/test/data/v1beta1/buildrun_buildah_tolerations_cr.yaml @@ -0,0 +1,8 @@ +--- +apiVersion: shipwright.io/v1beta1 +kind: BuildRun +metadata: + name: buildah-tolerations-buildrun +spec: + build: + name: buildah-tolerations-build \ No newline at end of file diff --git a/test/e2e/v1beta1/e2e_test.go b/test/e2e/v1beta1/e2e_test.go index 490df8c961..184b9cd445 100644 --- a/test/e2e/v1beta1/e2e_test.go +++ b/test/e2e/v1beta1/e2e_test.go @@ -15,6 +15,8 @@ import ( buildv1beta1 "github.com/shipwright-io/build/pkg/apis/build/v1beta1" shpgit "github.com/shipwright-io/build/pkg/git" + + corev1 "k8s.io/api/core/v1" ) var _ = Describe("For a Kubernetes cluster with Tekton and build installed", func() { @@ -640,4 +642,54 @@ var _ = Describe("For a Kubernetes cluster with Tekton and build installed", fun }) }) + Context("when tolerations are specified", func() { + + BeforeEach(func() { + testID = generateTestID("tolerations") + + // create the build definition + build = createBuild( + testBuild, + testID, + "test/data/v1beta1/build_buildah_tolerations_cr.yaml", + ) + + // Add a taint to both of the kind worker nodes + nodes, err := testBuild.GetNodes() + Expect(err).ToNot(HaveOccurred()) + for _, node := range nodes.Items { + if node.Name == "kind-worker" || node.Name == "kind-worker2" { + taint := corev1.Taint{Key: "test-key", Value: "test-vaue", Effect: "NoSchedule"} + testBuild.AddNodeTaint(node.Name, &taint) + } + } + }) + + AfterEach(func() { + err := testBuild.RemoveNodeTaints("kind-worker") + Expect(err).ToNot(HaveOccurred()) + err = testBuild.RemoveNodeTaints("kind-worker2") + Expect(err).ToNot(HaveOccurred()) + }) + + It("successfully runs a build when it tolerates the node taint", func() { + buildRun, err = buildRunTestData(testBuild.Namespace, testID, "test/data/v1beta1/buildrun_buildah_tolerations_cr.yaml") + Expect(err).ToNot(HaveOccurred(), "Error retrieving buildrun test data") + + buildRun = validateBuildRunToSucceed(testBuild, buildRun) + testBuild.ValidateImageDigest(buildRun) + }) + + It("fails to schedule when the build does not tolerate the node taint", func() { + buildRun, err = buildRunTestData(testBuild.Namespace, testID, "test/data/v1beta1/buildrun_buildah_tolerations_cr.yaml") + Expect(err).ToNot(HaveOccurred(), "Error retrieving buildrun test data") + buildRun.Spec.Tolerations[0].Key = "untolerated" + + validateBuildRunToFail(testBuild, buildRun) + buildRun, err = testBuild.LookupBuildRun(types.NamespacedName{Name: buildRun.Name, Namespace: testBuild.Namespace}) + + Expect(buildRun.Status.FailureDetails.Message).To(Equal(shpgit.AuthPrompted.ToMessage())) + Expect(buildRun.Status.FailureDetails.Reason).To(Equal(shpgit.AuthPrompted.String())) + }) + }) }) diff --git a/test/integration/build_to_taskruns_test.go b/test/integration/build_to_taskruns_test.go index 943c711532..ba8fa03827 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 59dc9abe52..bdb1f8b340 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/kind/config_three_node.yaml b/test/kind/config_three_node.yaml new file mode 100644 index 0000000000..c968e608fa --- /dev/null +++ b/test/kind/config_three_node.yaml @@ -0,0 +1,17 @@ +kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +kubeadmConfigPatches: +- | + kind: ClusterConfiguration + metadata: + name: config + apiServer: + extraArgs: + enable-admission-plugins: PodSecurity +nodes: +- role: control-plane + extraPortMappings: + - containerPort: 32222 + hostPort: 32222 +- role: worker +- role: worker diff --git a/test/utils/v1beta1/nodes.go b/test/utils/v1beta1/nodes.go new file mode 100644 index 0000000000..c5f169ec8f --- /dev/null +++ b/test/utils/v1beta1/nodes.go @@ -0,0 +1,51 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apply "k8s.io/client-go/applyconfigurations/core/v1" +) + +// GetNodes returns all Nodes for the TestBuild object +func (t *TestBuild) GetNodes() (*corev1.NodeList, error) { + client := t.Clientset.CoreV1().Nodes() + nodes, err := client.List(t.Context, metav1.ListOptions{}) + return nodes, err +} + +// AddNodeTaint sets a taint on the given Node name +func (t *TestBuild) AddNodeTaint(name string, taint *corev1.Taint) error { + client := t.Clientset.CoreV1().Nodes() + taintApplyCfg := apply.Taint() + taintApplyCfg.WithKey(taint.Key) + taintApplyCfg.WithValue(taint.Value) + taintApplyCfg.WithEffect(taint.Effect) + nodeSpecApplyCfg := apply.NodeSpec() + nodeSpecApplyCfg.WithTaints(taintApplyCfg) + applyCfg := apply.Node(name) + applyCfg.WithSpec(nodeSpecApplyCfg) + _, err := client.Apply(t.Context, applyCfg, metav1.ApplyOptions{}) + if err != nil { + return err + } + return nil +} + +// RemoveNodeTaints removes all taints on the given Node name +func (t *TestBuild) RemoveNodeTaints(name string) error { + client := t.Clientset.CoreV1().Nodes() + applyCfg := apply.Node(name) + taintApplyCfg := apply.Taint() + nodeSpecApplyCfg := apply.NodeSpec() + nodeSpecApplyCfg.WithTaints(taintApplyCfg) + applyCfg.WithSpec(nodeSpecApplyCfg) + _, err := client.Apply(t.Context, applyCfg, metav1.ApplyOptions{}) + if err != nil { + return err + } + return nil +} diff --git a/test/v1beta1_samples/build_samples.go b/test/v1beta1_samples/build_samples.go index a46f917fe9..eff97c3258 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 7e55033585..cb728e0dc6 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 = `