Skip to content

Commit

Permalink
Merge pull request #1683 from dorzel/MULTIARCH-4975
Browse files Browse the repository at this point in the history
Add NodeSelectors to Build and BuildRun objects
  • Loading branch information
openshift-merge-bot[bot] authored Oct 23, 2024
2 parents b55f25f + bc98f8a commit a2b53df
Show file tree
Hide file tree
Showing 19 changed files with 367 additions and 34 deletions.
24 changes: 24 additions & 0 deletions deploy/crds/shipwright.io_buildruns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6916,6 +6916,14 @@ spec:
- name
type: object
type: array
nodeSelector:
additionalProperties:
type: string
description: |-
NodeSelector is a selector which must be true for the pod to fit on a node.
Selector which must match a node's labels for the pod to be scheduled on that node.
More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
type: object
output:
description: Output refers to the location where the built
image would be pushed.
Expand Down Expand Up @@ -9120,6 +9128,14 @@ spec:
- name
type: object
type: array
nodeSelector:
additionalProperties:
type: string
description: |-
NodeSelector is a selector which must be true for the pod to fit on a node.
Selector which must match a node's labels for the pod to be scheduled on that node.
More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
type: object
output:
description: |-
Output refers to the location where the generated
Expand Down Expand Up @@ -11161,6 +11177,14 @@ spec:
- name
type: object
type: array
nodeSelector:
additionalProperties:
type: string
description: |-
NodeSelector is a selector which must be true for the pod to fit on a node.
Selector which must match a node's labels for the pod to be scheduled on that node.
More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
type: object
output:
description: Output refers to the location where the built image
would be pushed.
Expand Down
8 changes: 8 additions & 0 deletions deploy/crds/shipwright.io_builds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2488,6 +2488,14 @@ spec:
- name
type: object
type: array
nodeSelector:
additionalProperties:
type: string
description: |-
NodeSelector is a selector which must be true for the pod to fit on a node.
Selector which must match a node's labels for the pod to be scheduled on that node.
More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
type: object
output:
description: Output refers to the location where the built image would
be pushed.
Expand Down
34 changes: 21 additions & 13 deletions docs/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,25 @@ SPDX-License-Identifier: Apache-2.0

# Build

- [Overview](#overview)
- [Build Controller](#build-controller)
- [Build Validations](#build-validations)
- [Configuring a Build](#configuring-a-build)
- [Defining the Source](#defining-the-source)
- [Defining the Strategy](#defining-the-strategy)
- [Defining ParamValues](#defining-paramvalues)
- [Defining the Builder or Dockerfile](#defining-the-builder-or-dockerfile)
- [Defining the Output](#defining-the-output)
- [Defining Retention Parameters](#defining-retention-parameters)
- [Defining Volumes](#defining-volumes)
- [Defining Triggers](#defining-triggers)
- [BuildRun deletion](#buildrun-deletion)
- [Build](#build)
- [Overview](#overview)
- [Build Controller](#build-controller)
- [Build Validations](#build-validations)
- [Configuring a Build](#configuring-a-build)
- [Defining the Source](#defining-the-source)
- [Defining the Strategy](#defining-the-strategy)
- [Defining ParamValues](#defining-paramvalues)
- [Example](#example)
- [Defining the Builder or Dockerfile](#defining-the-builder-or-dockerfile)
- [Defining the Output](#defining-the-output)
- [Defining the vulnerabilityScan](#defining-the-vulnerabilityscan)
- [Defining Retention Parameters](#defining-retention-parameters)
- [Defining Volumes](#defining-volumes)
- [Defining Triggers](#defining-triggers)
- [GitHub](#github)
- [Image](#image)
- [Tekton Pipeline](#tekton-pipeline)
- [BuildRun Deletion](#buildrun-deletion)

## Overview

Expand All @@ -33,6 +39,7 @@ A `Build` resource allows the user to define:
- env
- retention
- volumes
- nodeSelector

A `Build` is available within a namespace.

Expand Down Expand Up @@ -118,6 +125,7 @@ The `Build` definition supports the following fields:
- `spec.retention.ttlAfterSucceeded` - Specifies the duration for which a successful buildrun can exist.
- `spec.retention.failedLimit` - Specifies the number of failed buildrun that can exist.
- `spec.retention.succeededLimit` - Specifies the number of successful buildrun can exist.
- `spec.nodeSelector` - Specifies a selector which must match a node's labels for the build pod to be scheduled on that node.

### Defining the Source

Expand Down
42 changes: 23 additions & 19 deletions docs/buildrun.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,28 @@ SPDX-License-Identifier: Apache-2.0

# BuildRun

- [Overview](#overview)
- [BuildRun Controller](#buildrun-controller)
- [Configuring a BuildRun](#configuring-a-buildrun)
- [Defining the Build Reference](#defining-the-build-reference)
- [Defining the Build Specification](#defining-the-build-specification)
- [Defining ParamValues](#defining-paramvalues)
- [Defining the ServiceAccount](#defining-the-serviceaccount)
- [Defining Retention Parameters](#defining-retention-parameters)
- [Defining Volumes](#defining-volumes)
- [Canceling a `BuildRun`](#canceling-a-buildrun)
- [Automatic `BuildRun` deletion](#automatic-buildrun-deletion)
- [Specifying Environment Variables](#specifying-environment-variables)
- [BuildRun Status](#buildrun-status)
- [Understanding the state of a BuildRun](#understanding-the-state-of-a-buildrun)
- [Understanding failed BuildRuns](#understanding-failed-buildruns)
- [Understanding failed git-source step](#understanding-failed-git-source-step)
- [Step Results in BuildRun Status](#step-results-in-buildrun-status)
- [Build Snapshot](#build-snapshot)
- [Relationship with Tekton Tasks](#relationship-with-tekton-tasks)
- [BuildRun](#buildrun)
- [Overview](#overview)
- [BuildRun Controller](#buildrun-controller)
- [Configuring a BuildRun](#configuring-a-buildrun)
- [Defining the Build Reference](#defining-the-build-reference)
- [Defining the Build Specification](#defining-the-build-specification)
- [Defining the Build Source](#defining-the-build-source)
- [Defining ParamValues](#defining-paramvalues)
- [Defining the ServiceAccount](#defining-the-serviceaccount)
- [Defining Retention Parameters](#defining-retention-parameters)
- [Defining Volumes](#defining-volumes)
- [Canceling a `BuildRun`](#canceling-a-buildrun)
- [Automatic `BuildRun` deletion](#automatic-buildrun-deletion)
- [Specifying Environment Variables](#specifying-environment-variables)
- [BuildRun Status](#buildrun-status)
- [Understanding the state of a BuildRun](#understanding-the-state-of-a-buildrun)
- [Understanding failed BuildRuns](#understanding-failed-buildruns)
- [Understanding failed BuildRuns due to VulnerabilitiesFound](#understanding-failed-buildruns-due-to-vulnerabilitiesfound)
- [Understanding failed git-source step](#understanding-failed-git-source-step)
- [Step Results in BuildRun Status](#step-results-in-buildrun-status)
- [Build Snapshot](#build-snapshot)
- [Relationship with Tekton Tasks](#relationship-with-tekton-tasks)

## Overview

Expand Down Expand Up @@ -72,6 +75,7 @@ The `BuildRun` definition supports the following fields:
- `spec.output.timestamp` - Overrides the output timestamp configuration of the referenced build to instruct the build to change the output image creation timestamp to the specified value. When omitted, the respective build strategy tool defines the output image timestamp.
- `spec.output.vulnerabilityScan` - Overrides the output vulnerabilityScan configuration of the referenced build to run the vulnerability scan for the generated image.
- `spec.env` - Specifies additional environment variables that should be passed to the build container. Overrides any environment variables that are specified in the `Build` resource. The available variables depend on the tool used by the chosen build strategy.
- `spec.nodeSelector` - Specifies a selector which must match a node's labels for the build pod to be scheduled on that node.

**Note**: The `spec.build.name` and `spec.build.spec` are mutually exclusive. Furthermore, the overrides for `timeout`, `paramValues`, `output`, and `env` can only be combined with `spec.build.name`, but **not** with `spec.build.spec`.

Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/build/v1beta1/build_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ const (
OutputTimestampNotSupported BuildReason = "OutputTimestampNotSupported"
// OutputTimestampNotValid indicates that the output timestamp value is not valid
OutputTimestampNotValid BuildReason = "OutputTimestampNotValid"
// NodeSelectorNotValid indicates that the nodeSelector value is not valid
NodeSelectorNotValid BuildReason = "NodeSelectorNotValid"

// AllValidationsSucceeded indicates a Build was successfully validated
AllValidationsSucceeded = "all validations succeeded"
Expand Down Expand Up @@ -174,6 +176,13 @@ type BuildSpec struct {
//
// +optional
Volumes []BuildVolume `json:"volumes,omitempty"`

// NodeSelector is a selector which must be true for the pod to fit on a node.
// Selector which must match a node's labels for the pod to be scheduled on that node.
// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
//
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
}

// BuildVolume is a volume that will be mounted in build pod during build step
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/build/v1beta1/buildrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ type BuildRunSpec struct {
// to be overridden. Must only contain volumes that exist in the corresponding BuildStrategy
// +optional
Volumes []BuildVolume `json:"volumes,omitempty"`

// NodeSelector is a selector which must be true for the pod to fit on a node.
// Selector which must match a node's labels for the pod to be scheduled on that node.
// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
//
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
}

// BuildRunRequestedState defines the buildrun state the user can provide to override whatever is the current state.
Expand Down
14 changes: 14 additions & 0 deletions pkg/apis/build/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/reconciler/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var validationTypes = [...]string{
validate.BuildName,
validate.Envs,
validate.Triggers,
validate.NodeSelector,
}

// ReconcileBuild reconciles a Build object
Expand Down
16 changes: 16 additions & 0 deletions pkg/reconciler/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package build_test
import (
"context"
"fmt"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -613,5 +614,20 @@ 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"}
buildSample.Spec.Output.PushSecret = nil

statusCall := ctl.StubFunc(corev1.ConditionFalse, build.NodeSelectorNotValid, "name part 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))
})
})
})
})
1 change: 1 addition & 0 deletions pkg/reconciler/buildrun/buildrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
validate.NewSourceRef(build),
validate.NewBuildName(build),
validate.NewEnv(build),
validate.NewNodeSelector(build),
)

// an internal/technical error during validation happened
Expand Down
14 changes: 14 additions & 0 deletions pkg/reconciler/buildrun/buildrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
})
})
})
9 changes: 9 additions & 0 deletions pkg/reconciler/buildrun/resources/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/shipwright-io/build/pkg/env"
"github.com/shipwright-io/build/pkg/reconciler/buildrun/resources/steps"
"github.com/shipwright-io/build/pkg/volumes"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
pipelineapi "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
)

Expand Down Expand Up @@ -234,6 +235,14 @@ func GenerateTaskRun(
},
}

// 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,
}
}

// assign the annotations from the build strategy, filter out those that should not be propagated
taskRunAnnotations := make(map[string]string)
for key, value := range strategy.GetAnnotations() {
Expand Down
22 changes: 22 additions & 0 deletions pkg/reconciler/buildrun/resources/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -632,5 +632,27 @@ var _ = Describe("GenerateTaskrun", func() {
Expect(paramOutputImageFound).To(BeTrue())
})
})

Context("when the build and buildrun both specify a nodeSelector", 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))
})
})
})
})
42 changes: 42 additions & 0 deletions pkg/validate/nodeselector.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright The Shipwright Contributors
//
// SPDX-License-Identifier: Apache-2.0

package validate

import (
"context"
"strings"

"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/utils/ptr"

build "github.com/shipwright-io/build/pkg/apis/build/v1beta1"
)

// NodeSelectorRef contains all required fields
// to validate a node selector
type NodeSelectorRef struct {
Build *build.Build // build instance for analysis
}

func NewNodeSelector(build *build.Build) *NodeSelectorRef {
return &NodeSelectorRef{build}
}

// ValidatePath implements BuildPath interface and validates
// that NodeSelector keys/values are valid labels
func (b *NodeSelectorRef) ValidatePath(_ context.Context) error {
for key, value := range b.Build.Spec.NodeSelector {
if errs := validation.IsQualifiedName(key); len(errs) > 0 {
b.Build.Status.Reason = ptr.To(build.NodeSelectorNotValid)
b.Build.Status.Message = ptr.To(strings.Join(errs, ", "))
}
if errs := validation.IsValidLabelValue(value); len(errs) > 0 {
b.Build.Status.Reason = ptr.To(build.NodeSelectorNotValid)
b.Build.Status.Message = ptr.To(strings.Join(errs, ", "))
}
}

return nil
}
Loading

0 comments on commit a2b53df

Please sign in to comment.