From 6ef46d5fa695794aadd36291c3c3e20f289dfb26 Mon Sep 17 00:00:00 2001 From: Dylan Orzel Date: Wed, 30 Oct 2024 15:22:44 -0600 Subject: [PATCH] Add Tolerations to Build and BuildRun objects Signed-off-by: Dylan Orzel --- pkg/apis/build/v1beta1/build_types.go | 6 ++ pkg/apis/build/v1beta1/buildrun_types.go | 4 ++ pkg/reconciler/build/build.go | 1 + pkg/reconciler/buildrun/buildrun.go | 1 + pkg/reconciler/buildrun/resources/taskrun.go | 35 ++++++++++- pkg/validate/tolerations.go | 61 ++++++++++++++++++++ pkg/validate/validate.go | 4 ++ 7 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 pkg/validate/tolerations.go diff --git a/pkg/apis/build/v1beta1/build_types.go b/pkg/apis/build/v1beta1/build_types.go index 22a6e0b7f..e421313e8 100644 --- a/pkg/apis/build/v1beta1/build_types.go +++ b/pkg/apis/build/v1beta1/build_types.go @@ -78,6 +78,8 @@ const ( OutputTimestampNotValid BuildReason = "OutputTimestampNotValid" // NodeSelectorNotValid indicates that the nodeSelector value is not valid NodeSelectorNotValid BuildReason = "NodeSelectorNotValid" + // TolerationNotValid indicates that the Toleration value is not valid + TolerationNotValid BuildReason = "TolerationNotValid" // AllValidationsSucceeded indicates a Build was successfully validated AllValidationsSucceeded = "all validations succeeded" @@ -183,6 +185,10 @@ type BuildSpec struct { // // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // If specified, the pod's tolerations. + // +optional + Tolerations []corev1.Toleration `json:"tolerations,omitempty"` } // BuildVolume is a volume that will be mounted in build pod during build step diff --git a/pkg/apis/build/v1beta1/buildrun_types.go b/pkg/apis/build/v1beta1/buildrun_types.go index 05bc7723f..d752041ee 100644 --- a/pkg/apis/build/v1beta1/buildrun_types.go +++ b/pkg/apis/build/v1beta1/buildrun_types.go @@ -115,6 +115,10 @@ type BuildRunSpec struct { // // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // If specified, the pod's tolerations. + // +optional + Tolerations []corev1.Toleration `json:"tolerations,omitempty"` } // BuildRunRequestedState defines the buildrun state the user can provide to override whatever is the current state. diff --git a/pkg/reconciler/build/build.go b/pkg/reconciler/build/build.go index cfdb15db5..805d13a63 100644 --- a/pkg/reconciler/build/build.go +++ b/pkg/reconciler/build/build.go @@ -34,6 +34,7 @@ var validationTypes = [...]string{ validate.Envs, validate.Triggers, validate.NodeSelector, + validate.Tolerations, } // ReconcileBuild reconciles a Build object diff --git a/pkg/reconciler/buildrun/buildrun.go b/pkg/reconciler/buildrun/buildrun.go index e17255bab..b5945cda9 100644 --- a/pkg/reconciler/buildrun/buildrun.go +++ b/pkg/reconciler/buildrun/buildrun.go @@ -161,6 +161,7 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req validate.NewBuildName(build), validate.NewEnv(build), validate.NewNodeSelector(build), + validate.NewTolerations(build), ) // an internal/technical error during validation happened diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index 17b790368..3e7695689 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -7,6 +7,7 @@ package resources import ( "fmt" "path" + "slices" "strconv" "strings" @@ -235,12 +236,21 @@ func GenerateTaskRun( }, } + taskRunPodTemplate := &pod.PodTemplate{} // Merge Build and BuildRun NodeSelectors, giving preference to BuildRun NodeSelector taskRunNodeSelector := mergeMaps(build.Spec.NodeSelector, buildRun.Spec.NodeSelector) if len(taskRunNodeSelector) > 0 { - expectedTaskRun.Spec.PodTemplate = &pod.PodTemplate{ - NodeSelector: taskRunNodeSelector, - } + taskRunPodTemplate.NodeSelector = taskRunNodeSelector + } + + // Merge Build and BuildRun Tolerations, giving preference to BuildRun Tolerations values + taskRunTolerations := mergeTolerations(build.Spec.Tolerations, buildRun.Spec.Tolerations) + if len(taskRunTolerations) > 0 { + taskRunPodTemplate.Tolerations = taskRunTolerations + } + + if !(taskRunPodTemplate == &pod.PodTemplate{}) { + expectedTaskRun.Spec.PodTemplate = taskRunPodTemplate } // assign the annotations from the build strategy, filter out those that should not be propagated @@ -354,6 +364,25 @@ func effectiveTimeout(build *buildv1beta1.Build, buildRun *buildv1beta1.BuildRun return nil } +// mergeTolerations merges the values for Spec.Tolerations in the given Build and BuildRun objects, with values in the BuildRun object overriding values +// in the Build object (if present). +func mergeTolerations(buildTolerations []corev1.Toleration, buildRunTolerations []corev1.Toleration) []corev1.Toleration { + mergedTolerations := []corev1.Toleration{} + mergedTolerations = append(mergedTolerations, buildRunTolerations...) + for _, toleration := range buildTolerations { + if !slices.ContainsFunc(mergedTolerations, func(t corev1.Toleration) bool { + return t.Key == toleration.Key + }) { + mergedTolerations = append(mergedTolerations, toleration) + } + } + // only support "NoSchedule" effect, make sure it is set + for i := range mergedTolerations { + mergedTolerations[i].Effect = corev1.TaintEffectNoSchedule + } + return mergedTolerations +} + // isPropagatableAnnotation filters the last-applied-configuration annotation from kubectl because this would break the meaning of this annotation on the target object; // also, annotations using our own custom resource domains are filtered out because we have no annotations with a semantic for both TaskRun and Pod func isPropagatableAnnotation(key string) bool { diff --git a/pkg/validate/tolerations.go b/pkg/validate/tolerations.go new file mode 100644 index 000000000..3730cbcf7 --- /dev/null +++ b/pkg/validate/tolerations.go @@ -0,0 +1,61 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "context" + "fmt" + "strings" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/utils/ptr" + + build "github.com/shipwright-io/build/pkg/apis/build/v1beta1" +) + +// TolerationsRef contains all required fields +// to validate tolerations +type TolerationsRef struct { + Build *build.Build // build instance for analysis +} + +func NewTolerations(build *build.Build) *TolerationsRef { + return &TolerationsRef{build} +} + +// ValidatePath implements BuildPath interface and validates +// that tolerations key/operator/value are valid +func (b *TolerationsRef) ValidatePath(_ context.Context) error { + for _, toleration := range b.Build.Spec.Tolerations { + // validate Key + if errs := validation.IsValidLabelValue(toleration.Key); errs != nil { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To(strings.Join(errs, ", ")) + } + // validate Operator + if !((toleration.Operator == v1.TolerationOpExists) || (toleration.Operator == v1.TolerationOpEqual)) { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To(fmt.Sprintf("Toleration operator not valid. Must be one of: '%v', '%v'", v1.TolerationOpExists, v1.TolerationOpEqual)) + } + // validate Value + if errs := validation.IsValidLabelValue(toleration.Value); errs != nil { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To(strings.Join(errs, ", ")) + } + // validate Effect, which should not be specified + if len(toleration.Effect) > 0 { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To("Specifying Toleration Effect is not supported.") + } + // validate TolerationSeconds, which should not be specified + if !(toleration.TolerationSeconds == ptr.To(int64(0))) { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To("Specifying TolerationSeconds is not supported.") + } + } + + return nil +} diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index d39396652..219d84c0b 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -37,6 +37,8 @@ const ( Triggers = "triggers" // NodeSelector for validating `spec.nodeSelector` entry NodeSelector = "nodeselector" + // Tolerations for validating `spec.tolerations` entry + Tolerations = "tolerations" ) const ( @@ -79,6 +81,8 @@ func NewValidation( return &Trigger{build: build}, nil case NodeSelector: return &NodeSelectorRef{Build: build}, nil + case Tolerations: + return &TolerationsRef{Build: build}, nil default: return nil, fmt.Errorf("unknown validation type") }