From 483b3aa2e6c1d997be6a57baee44c434b2ad9e6d Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Wed, 15 May 2024 09:53:42 -0400 Subject: [PATCH 1/4] SHIP-0039: Build Scheduler Options Provisional proposal to add API options to control where build pods are scheduled. These will rely on core Kubernetes features related to pod scheduling. This proposal was partially motivated by the multi-arch feature discussions, where it was revealed that we currently have no means of controlling where build pods are scheduled. While these features may support a future proof of concept for multi-arch builds, orchestrating multi-arch builds end to end is out of scope. --- ships/0039-build-scheduler-opts.md | 214 +++++++++++++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 ships/0039-build-scheduler-opts.md diff --git a/ships/0039-build-scheduler-opts.md b/ships/0039-build-scheduler-opts.md new file mode 100644 index 0000000..47af77e --- /dev/null +++ b/ships/0039-build-scheduler-opts.md @@ -0,0 +1,214 @@ + + +--- +title: build-scheduler-options +authors: + - "@adambkaplan" +reviewers: + - "@apoorvajagtap" + - "@HeavyWombat" +approvers: + - "@qu1queee" + - "@SaschaSchwarze0" +creation-date: 2024-05-15 +last-updated: 2024-05-15 +status: provisional +see-also: [] +replaces: [] +superseded-by: [] +--- + +# Build Scheduler Options + + + +This is the title of the enhancement. Keep it simple and descriptive. A good title can help +communicate what the enhancement is and should be considered as part of any review. + +The YAML `title` should be lowercased and spaces/punctuation should be replaced with `-`. + +To get started with this template: + +1. **Make a copy of this template.** Copy this template into the main + `proposals` directory, with a filename like `NNNN-neat-enhancement-idea.md` + where `NNNN` is an incrementing number associated with this SHIP. +2. **Fill out the "overview" sections.** This includes the Summary and Motivation sections. These + should be easy and explain why the community should desire this enhancement. +3. **Create a PR.** Assign it to folks with expertise in that domain to help + sponsor the process. The PR title should be like "SHIP-NNNN: Neat + Enhancement Idea", where "NNNN" is the number associated with this SHIP. +4. **Merge at each milestone.** Merge when the design is able to transition to a new status + (provisional, implementable, implemented, etc.). View anything marked as `provisional` as an idea + worth exploring in the future, but not accepted as ready to execute. Anything marked as + `implementable` should clearly communicate how an enhancement is coded up and delivered. Aim for + single topic PRs to keep discussions focused. If you disagree with what is already in a document, + open a new PR with suggested changes. + +The `Metadata` section above is intended to support the creation of tooling around the enhancement +process. + +<--> + +## Release Signoff Checklist + +- [ ] Enhancement is `implementable` +- [ ] Design details are appropriately documented from clear requirements +- [ ] Test plan is defined +- [ ] Graduation criteria for dev preview, tech preview, GA +- [ ] User-facing documentation is created in [docs](/docs/) + +## Open Questions [optional] + +TBD + +## Summary + +Add API options that influece where `BuildRun` pods are scheduled on Kubernetes. This can be +acomplished through the following mechanisms: + +- [Node Selectors](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#nodeselector) +- [Affinity/anti-affinity](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) +- [Taints and Tolerations](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/) + +## Motivation + +Today, `BuildRun` pods will run on arbitrary nodes - developers, platform engineers, and admins do +not have the ability to control where a specific build pod will be scheduled. Teams may have +several motivations for controlling where a build pod is scheduled: + +- Builds can be CPU/memory/storage intensive. Scheduling on larger worker nodes with additional + memory or compute can help ensure builds succeed. +- Clusters may have mutiple worker node architectures and even OS (Windows nodes). Container images + are by their nature specific to the OS and CPU architecture, and default to the host operating + system and architecture. Builds may need to specify OS and architecture through node selectors. +- Left unchecked, builds may congregate on a set of nodes, impacting overall cluster utilization + and stability. + +### Goals + +- Allow build pods to run on specific nodes using node selectors. +- Allow build pods to set node affinity/anti-affinity rules. +- Allow build pods to tolerate node taints. +- Allow node selection, pod affinity, and taint toleration to be set at the cluster level. + +### Non-Goals + +- Primary feature support for multi-arch builds. + +## Proposal + +This is where we get down to the nitty gritty of what the proposal actually is. + +### User Stories [optional] + +#### Node Selection - platform engineer + +As a platform engineer, I want builds to use node selectors to ensure they are scheduled on nodes +optimized for builds so that builds are more likely to succeed + +#### Node Selection - arch-specific images + +As a developer, I want to select the OS and architecture of my build's node so that I can run +builds on worker nodes with multiple architectures. + +#### Pod affinity - platform engineer/admin + +As a platform engineer/cluster admin, I want to set anti-affinity rules for build pods so that +running builds are not scheduled/clustered on the same node. + +#### Taint toleration - cluster admin + +As a cluster admin, I want builds to be able to tolerate provided node taints so that they can +be scheduled on nodes that are not suitable/designated for application workloads. + +### Implementation Notes + +TBD + + +**Note:** *Section not required until feature is ready to be marked 'implementable'.* + +Describe in detail what you propose to change. Be specific as to how you intend to implement this +feature. If you plan to introduce a new API field, provide examples of how the new API will fit in +the broader context and how end users could invoke the new behavior. +<--> + +### Test Plan + +TBD + + +**Note:** *Section not required until targeted at a release.* + +Consider the following in developing a test plan for this enhancement: + +- Will there be e2e and integration tests, in addition to unit tests? +- How will it be tested in isolation vs with other components? + +No need to outline all of the test cases, just the general strategy. Anything that would count as +tricky in the implementation and anything particularly challenging to test should be called out. + +All code is expected to have adequate tests (eventually with coverage expectations). +<--> + +### Release Criteria + +TBD + +**Note:** *Section not required until targeted at a release.* + +#### Removing a deprecated feature [if necessary] + +Not applicable. + +#### Upgrade Strategy [if necessary] + + + +If applicable, how will the component be upgraded? Make sure this is in the test +plan. + +Consider the following in developing an upgrade strategy for this enhancement: + +- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to + make on upgrade in order to keep previous behavior? +- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to + make on upgrade in order to make use of the enhancement? +<--> + +### Risks and Mitigations + +TBD + + +What are the risks of this proposal and how do we mitigate? Think broadly. For example, consider +both security and how this will impact the larger Shipwright ecosystem. + +How will security be reviewed and by whom? How will UX be reviewed and by whom? +<--> + +## Drawbacks + +TBD - The idea is to find the best form of an argument why this enhancement should _not_ be implemented. + +## Alternatives + +TBD + +Similar to the `Drawbacks` section the `Alternatives` section is used to highlight and record other +possible approaches to delivering the value proposed by an enhancement. + +## Infrastructure Needed [optional] + +No additional infrastructure antipated. +Test KinD clusters may need to deploy with additional nodes where these features can be verified. + +## Implementation History + +Major milestones in the life cycle of a proposal should be tracked in `Implementation History`. + + From 1ac0bf50e6e5f2985c0a7dcee1c6f25b7c9700f8 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Mon, 3 Jun 2024 15:37:19 -0400 Subject: [PATCH 2/4] Updates from proposal feedback - Removed the goal to set pod scheduling options at the cluster level. This adds significant scope since the build controller would need to support this configuration via a file or large set of environment variables. This goal is not ruled out completely - I'm recommending it as a follow up feature so the community can iterate and deliver quickly. - Added support for custom schedulers - this feature has been in Kubernetes for quite some time, though its adoption is somewhat limited due to the challenges of writing a custom scheduler. Signed-off-by: Adam Kaplan --- ships/0039-build-scheduler-opts.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ships/0039-build-scheduler-opts.md b/ships/0039-build-scheduler-opts.md index 47af77e..1debcd0 100644 --- a/ships/0039-build-scheduler-opts.md +++ b/ships/0039-build-scheduler-opts.md @@ -93,11 +93,18 @@ several motivations for controlling where a build pod is scheduled: - Allow build pods to run on specific nodes using node selectors. - Allow build pods to set node affinity/anti-affinity rules. - Allow build pods to tolerate node taints. -- Allow node selection, pod affinity, and taint toleration to be set at the cluster level. +- Allow build pods to use a [custom scheduler](https://kubernetes.io/docs/tasks/extend-kubernetes/configure-multiple-schedulers/). ### Non-Goals - Primary feature support for multi-arch builds. +- Allow node selection, pod affinity, and taint toleration to be set at the cluster level. + While this may be desirable, it requires a more sophisticated means of configuring the build + controller. Setting default values for scheduling options can be considered as a follow-up + feature. +- Prevent use of build pod scheduling fields. This is best left to an admission controller like + [OPA Gatekeeper](https://www.openpolicyagent.org/docs/latest/kubernetes-introduction/) or + [Kyverno](https://kyverno.io/). ## Proposal @@ -125,6 +132,11 @@ running builds are not scheduled/clustered on the same node. As a cluster admin, I want builds to be able to tolerate provided node taints so that they can be scheduled on nodes that are not suitable/designated for application workloads. +#### Custom Scheduler + +As a platform engineer/cluster admin, I want builds to use a custom scheduler so that I can provide +my own scheduler that is optimized for my build workloads. + ### Implementation Notes TBD From ac0a19dcce0c1499132c4b52a17d9bc6f5143fb2 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Fri, 14 Jun 2024 19:51:29 -0400 Subject: [PATCH 3/4] Drop Build Affinity Rules Affinity and anti-affinity is an incredibly rich and complex API in Kubernetes. Most of the other scheduler tuning APIs in this SHIP are simpler to understand and can be wired directly to underlying APIs (Tekton today, potentially another implementation in the future). Exposing the entire affinity rule API will make it challenging for developers to utilize. We should instead strive for a simpler API that makes it easy for developers to express their intent. This abstraction can then be translated to the underlying scheduler APIs. Such an effort is not trivial and warrants a separate feature/SHIP. Signed-off-by: Adam Kaplan --- ships/0039-build-scheduler-opts.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ships/0039-build-scheduler-opts.md b/ships/0039-build-scheduler-opts.md index 1debcd0..0c0b791 100644 --- a/ships/0039-build-scheduler-opts.md +++ b/ships/0039-build-scheduler-opts.md @@ -91,7 +91,6 @@ several motivations for controlling where a build pod is scheduled: ### Goals - Allow build pods to run on specific nodes using node selectors. -- Allow build pods to set node affinity/anti-affinity rules. - Allow build pods to tolerate node taints. - Allow build pods to use a [custom scheduler](https://kubernetes.io/docs/tasks/extend-kubernetes/configure-multiple-schedulers/). @@ -105,6 +104,11 @@ several motivations for controlling where a build pod is scheduled: - Prevent use of build pod scheduling fields. This is best left to an admission controller like [OPA Gatekeeper](https://www.openpolicyagent.org/docs/latest/kubernetes-introduction/) or [Kyverno](https://kyverno.io/). +- Allow build pods to set node affinity/anti-affinity rules. Affinity/anti-affinity is an + incredibly rich and complex API (see [docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#inter-pod-affinity-and-anti-affinity) + for more information). We should strive to provide a simpler interface that is tailored + specifically to builds. For the sake of keeping the scope of this SHIP narrow, this feature is + being dropped. Build affinity rules can/should be addressed in a follow up feature. ## Proposal @@ -122,11 +126,6 @@ optimized for builds so that builds are more likely to succeed As a developer, I want to select the OS and architecture of my build's node so that I can run builds on worker nodes with multiple architectures. -#### Pod affinity - platform engineer/admin - -As a platform engineer/cluster admin, I want to set anti-affinity rules for build pods so that -running builds are not scheduled/clustered on the same node. - #### Taint toleration - cluster admin As a cluster admin, I want builds to be able to tolerate provided node taints so that they can From f17810f616c8a7e8b04a2a5c7cc86887f9d723a2 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Fri, 14 Jun 2024 19:42:32 -0400 Subject: [PATCH 4/4] Move to Implementable These changes complete all sections of the SHIP needed to reach the "Implementable" state. This included some cleanup in the top summary, concrete implementations of new API fields with their respective types, and justification for implementing our own Tolerations API. As part of thinking through risks, mitigations, and drawbacks, I recommend that the Shipwright community publish a hardening guide alongside this feature's implementation. Exposing node selectors and tolerating taints can risk system availability, especially if builds are scheduled on control plane nodes. Kubernetes tries to prevent this by default through node labels and taints. There are some scenarios (ex: single node or small clusters) where scheduling builds on the control plane is desirable or necessary; publishing a hardening guide preserves flexibility while enabling informed choices for security. The drawbacks and alternatives also call out potential future concerns related to multi-arch support in Shipwright. This is under active discussion, and SHIP-0039 may add additional complexity to any future design. Signed-off-by: Adam Kaplan --- ships/0039-build-scheduler-opts.md | 229 ++++++++++++++++++----------- 1 file changed, 143 insertions(+), 86 deletions(-) diff --git a/ships/0039-build-scheduler-opts.md b/ships/0039-build-scheduler-opts.md index 0c0b791..7e25bb4 100644 --- a/ships/0039-build-scheduler-opts.md +++ b/ships/0039-build-scheduler-opts.md @@ -15,8 +15,8 @@ approvers: - "@qu1queee" - "@SaschaSchwarze0" creation-date: 2024-05-15 -last-updated: 2024-05-15 -status: provisional +last-updated: 2024-06-20 +status: Implementable see-also: [] replaces: [] superseded-by: [] @@ -24,46 +24,17 @@ superseded-by: [] # Build Scheduler Options - - -This is the title of the enhancement. Keep it simple and descriptive. A good title can help -communicate what the enhancement is and should be considered as part of any review. - -The YAML `title` should be lowercased and spaces/punctuation should be replaced with `-`. - -To get started with this template: - -1. **Make a copy of this template.** Copy this template into the main - `proposals` directory, with a filename like `NNNN-neat-enhancement-idea.md` - where `NNNN` is an incrementing number associated with this SHIP. -2. **Fill out the "overview" sections.** This includes the Summary and Motivation sections. These - should be easy and explain why the community should desire this enhancement. -3. **Create a PR.** Assign it to folks with expertise in that domain to help - sponsor the process. The PR title should be like "SHIP-NNNN: Neat - Enhancement Idea", where "NNNN" is the number associated with this SHIP. -4. **Merge at each milestone.** Merge when the design is able to transition to a new status - (provisional, implementable, implemented, etc.). View anything marked as `provisional` as an idea - worth exploring in the future, but not accepted as ready to execute. Anything marked as - `implementable` should clearly communicate how an enhancement is coded up and delivered. Aim for - single topic PRs to keep discussions focused. If you disagree with what is already in a document, - open a new PR with suggested changes. - -The `Metadata` section above is intended to support the creation of tooling around the enhancement -process. - -<--> - ## Release Signoff Checklist -- [ ] Enhancement is `implementable` -- [ ] Design details are appropriately documented from clear requirements -- [ ] Test plan is defined +- [x] Enhancement is `implementable` +- [x] Design details are appropriately documented from clear requirements +- [x] Test plan is defined - [ ] Graduation criteria for dev preview, tech preview, GA - [ ] User-facing documentation is created in [docs](/docs/) ## Open Questions [optional] -TBD +- Should this be enabled always? Should we consider an alpha -> beta lifecycle for this feature? (ex: off by default -> on by default) ## Summary @@ -71,8 +42,8 @@ Add API options that influece where `BuildRun` pods are scheduled on Kubernetes. acomplished through the following mechanisms: - [Node Selectors](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#nodeselector) -- [Affinity/anti-affinity](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) - [Taints and Tolerations](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/) +- [Custom Schedulers](https://kubernetes.io/docs/tasks/extend-kubernetes/configure-multiple-schedulers/) ## Motivation @@ -85,14 +56,15 @@ several motivations for controlling where a build pod is scheduled: - Clusters may have mutiple worker node architectures and even OS (Windows nodes). Container images are by their nature specific to the OS and CPU architecture, and default to the host operating system and architecture. Builds may need to specify OS and architecture through node selectors. -- Left unchecked, builds may congregate on a set of nodes, impacting overall cluster utilization - and stability. +- The default Kubernetes scheduler may not efficiently schedule build workloads - especially + considering how Tekton implements step containers and sidecars. A custom scheduler optimized for + Tekton or other batch workloads may lead to better cluster utulization. ### Goals - Allow build pods to run on specific nodes using node selectors. - Allow build pods to tolerate node taints. -- Allow build pods to use a [custom scheduler](https://kubernetes.io/docs/tasks/extend-kubernetes/configure-multiple-schedulers/). +- Allow build pods to use a custom scheduler. ### Non-Goals @@ -107,14 +79,12 @@ several motivations for controlling where a build pod is scheduled: - Allow build pods to set node affinity/anti-affinity rules. Affinity/anti-affinity is an incredibly rich and complex API (see [docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#inter-pod-affinity-and-anti-affinity) for more information). We should strive to provide a simpler interface that is tailored - specifically to builds. For the sake of keeping the scope of this SHIP narrow, this feature is - being dropped. Build affinity rules can/should be addressed in a follow up feature. + specifically to builds. This feature is being dropped to narrow the scope of this SHIP. Build + affinity rules can/should be addressed in a follow up feature. ## Proposal -This is where we get down to the nitty gritty of what the proposal actually is. - -### User Stories [optional] +### User Stories #### Node Selection - platform engineer @@ -138,33 +108,107 @@ my own scheduler that is optimized for my build workloads. ### Implementation Notes -TBD +#### API Updates - -**Note:** *Section not required until feature is ready to be marked 'implementable'.* +The `BuildSpec` API for Build and BuildRun will be updated to add the following fields: -Describe in detail what you propose to change. Be specific as to how you intend to implement this -feature. If you plan to introduce a new API field, provide examples of how the new API will fit in -the broader context and how end users could invoke the new behavior. -<--> +```yaml +spec: + ... + nodeSelector: # map[string]string + : "label-value" + tolerations: # []Toleration + - key: "taint-key" + operator: Exists|Equal + value: "taint-value" + schedulerName: "custom-scheduler-name" # string +``` -### Test Plan +The `nodeSelector` and `schedulerName` fields will use golang primitives that match their k8s +equivalents. -TBD +#### Tolerations - -**Note:** *Section not required until targeted at a release.* +The Tolerations API for Shipwright will support a limited subset of the upstream Kubernetes +Tolerations API. For simplicity, any Shipwright Build or BuildRun with a toleration set will use +the `NoSchedule` [taint effect](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/). + +```yaml +spec: + tolerations: # Optional array + - key: "taint-key" # Aligns with upstream k8s taint labels. Required + operator: Exists|Equal # Aligns with upstream k8s - key exists or node label key = value. Required + value: "taint-value" # Alights with upstream k8s taint value. Optional. +``` + +As with upstream k8s, the Shipwright Tolerations API array should support +[strategic merge JSON patching](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/#notes-on-the-strategic-merge-patch). + +#### Precedence Ordering and Value Merging + +Values in `BuildRun` will override those in the referenced `Build` object (if present). Values for +`nodeSelector` and `tolerations` should use strategic merge logic when possible: + +- `nodeSelector` merges using map keys. If the map key is present in the `Build` and `BuildRun` + object, the `BuildRun` overrides the value. +- `tolerations` merges using the taint key. If the taint key is present in the `Build` and + `BuildRun` object, the `BuildRun` overrides the value. + +This allows the `BuildRun` object to "inherit" values from a parent `Build` object. -Consider the following in developing a test plan for this enhancement: +#### Impact on Tekton TaskRun -- Will there be e2e and integration tests, in addition to unit tests? -- How will it be tested in isolation vs with other components? +Tekton supports tuning the pod of the `TaskRun` using the +[podTemplate](https://tekton.dev/docs/pipelines/taskruns/#specifying-a-pod-template) field. When +Shipwright creates the `TaskRun` for a build, the respective node selector, tolerations, and +scheduler name can be passed through. -No need to outline all of the test cases, just the general strategy. Anything that would count as -tricky in the implementation and anything particularly challenging to test should be called out. +#### Command Line Enhancements + +The `shp` CLI _may_ be enhanced to add flags that set the node selector, tolerations, and custom +scheduler for a `BuildRun`. For example, `shp build run` can have the following new options: + +- `--node==`: Use the node label key/value pair in the selector. Can be set more than + once for multiple key/value pairs.. +- `--tolerate=` or `--tolerate==`: Tolerate the taint key, in one of two ways: + - First form: taint key `Exists`. + - Second form: taint key `Equals` provided value. + - In both cases, this flag can be set more than once. +- `--scheduler=`: use custom scheduler with given name. Can only be set once. + + +#### Hardening Guidelines + +Exposing `nodeSelector` and `tolerations` to end developers adds risk with respect to overall +system availability. Some platform teams may not want these Kubernetes internals exposed or +modifiable by end developers at all. To address these concerns, a hardening guideline for +Shipwright Builds should also be published alongside documentation for this feature. This guideline +should recommend the use of third party admission controllers (ex: OPA, Kyverno) to prevent builds +from using values that impact system availability and performance. For example: + +- Block toleration of `node.kubernetes.io/*` taints. These are reserved for nodes that are not + ready to receive workloads for scheduling. +- Block node selectors with the `node-role.kubernetes.io/control-plane` label key. This is reserved + for control plane components (`kube-apiserver`, `kube-controller-manager`, etc.) +- Block toleration of the `node-role.kubernetes.io/control-plane` taint key. Same as above. + +See the [well known labels](https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane) +documentation for more information. + +### Test Plan + +- Unit testing can verify that the generated `TaskRun` object for a build contains the desired pod + template fields. +- End to end tests using `KinD` is possible for the `nodeSelector` and `tolerations` fields: + - KinD has support for configuring multiple [nodes](https://kind.sigs.k8s.io/docs/user/configuration/#nodes) + - Once set up, KinD nodes can simulate real nodes when it comes to pod scheduling, node labeling, + and node taints. +- End to end testing for the `schedulerName` field requires the deployment of a custom scheduler, + plus code to verify that the given scheduler was used. This is non-trivial (see + [upstream example](https://kubernetes.io/docs/tasks/extend-kubernetes/configure-multiple-schedulers/#specify-schedulers-for-pods)) + and adds a potential failure point to the test suite. Relying on unit testing alone is our best + option. -All code is expected to have adequate tests (eventually with coverage expectations). -<--> ### Release Criteria @@ -178,40 +222,54 @@ Not applicable. #### Upgrade Strategy [if necessary] - +The top-level API fields will be optional and default to Golang empty values. +On upgrade, these values will remain empty on existing `Build`/`BuildRun` objects. -If applicable, how will the component be upgraded? Make sure this is in the test -plan. - -Consider the following in developing an upgrade strategy for this enhancement: - -- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to - make on upgrade in order to keep previous behavior? -- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to - make on upgrade in order to make use of the enhancement? -<--> ### Risks and Mitigations -TBD +**Risk:** Node selector field allows disruptive workloads (builds) to be scheduled on control plane +nodes. - -What are the risks of this proposal and how do we mitigate? Think broadly. For example, consider -both security and how this will impact the larger Shipwright ecosystem. +*Mitigation*: Hardening guideline added as a requirement for this feature. There may be some +cluster topologies (ex: single node clusters) where scheduling builds on the "control plane" is not +only desirable, but necessary. Hardening guidelines referencing third party admission controllers +preserves flexibility while giving cluster administrators/platform teams the knowledge needed to +harden their environments as they see fit. -How will security be reviewed and by whom? How will UX be reviewed and by whom? -<--> ## Drawbacks -TBD - The idea is to find the best form of an argument why this enhancement should _not_ be implemented. +Exposing these fields leaks - to a certain extent - our abstraction over Kubernetes. This proposal +places k8s pod scheduling fields up front in the API for `Build` and `BuildRun`, a deviation from +Tekton which exposes the fields through a `PodTemplate` sub-field. Cluster administrators may not +want end developers to have control over where these pods are scheduled - they may instead wish to +control pod scheduling through Tekton's +[default pod template](https://github.com/tektoncd/pipeline/blob/main/docs/podtemplates.md#supported-fields) +mechanism at the controller level. + +Exposing `nodeSelector` may also conflict with future enhancements to support +[multi-architecture image builds](https://github.com/shipwright-io/build/issues/1119). A +hypothetical build that fans out individual image builds to nodes with desired OS/architecture +pairs may need to explicitly set the `k8s.io/os` and `k8s.io/architecture` node selector fields on +generated `TaskRuns`. With that said, there is currently no mechanism for Shipwright to control +where builds execute on clusters with multiple worker node architectures and operating systems. + ## Alternatives -TBD +An earlier draft of this proposal included `affinity` for setting pod affinity/anti-affinity rules. +This was rejected due to the complexities of Kubernetes pod affinity and anti-affinity. We need +more concrete user stories from the community to understand what - if anything - we should do with +respect to distributing build workloads through affinity rules. This may also conflict with +Tekton's [affinity assistant](https://tekton.dev/docs/pipelines/affinityassistants/) feature - an optional configuration that is enabled by default in upstream Tekton. + +An earlier draft also included the ability to set default values for these fields at the cluster +level. This would be similar to Tekton's capability with the Pipeline controller configuration. +Since this option is available at the Tekton pipeline level, adding nearly identical features to +Shipwright is being deferred. Tuning pod template values with the Tekton pipeline controller may +also be an acceptable alternative to this feature in some circumstances. -Similar to the `Drawbacks` section the `Alternatives` section is used to highlight and record other -possible approaches to delivering the value proposed by an enhancement. ## Infrastructure Needed [optional] @@ -220,6 +278,5 @@ Test KinD clusters may need to deploy with additional nodes where these features ## Implementation History -Major milestones in the life cycle of a proposal should be tracked in `Implementation History`. - - +- 2024-05-15: Created as `provisional` +- 2024-06-20: Draft updated to `implementable`