Skip to content

Commit 69b7a17

Browse files
committed
fix: Address review feedback
- Kubernetes user namespaces are out of scope, can be considered in a follow-up feature request. - Removed requirement to fail fast if the pod schedule settings conflict with runtime class schedule settings. Signed-off-by: Adam Kaplan <[email protected]>
1 parent f911cfa commit 69b7a17

File tree

1 file changed

+14
-6
lines changed

1 file changed

+14
-6
lines changed

ships/0040-build-runtime-class.md

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ superseded-by: []
3636
## Open Questions [optional]
3737

3838
- Should user namespaces also be in scope for this feature?
39+
_No, this should be considered separately. Feature is beta as of k8s v1.30_.
3940

4041
## Summary
4142

@@ -56,7 +57,7 @@ hardware virtualization to the existing mechanisms for isolating containers.
5657

5758
- Automatic mechanisms for selecting the container runtime.
5859
- Network isolated builds.
59-
- Isolation with Kubernetes [user namespaces](https://kubernetes.io/docs/concepts/workloads/pods/user-namespaces/). _Should this be a goal as well?_
60+
- Isolation with Kubernetes [user namespaces](https://kubernetes.io/docs/concepts/workloads/pods/user-namespaces/).
6061

6162
## Proposal
6263

@@ -134,10 +135,11 @@ such as node selectors and tolerations. As of Kubernetes 1.32, this is a "beta"
134135
on most Kubernetes distributions by default. These scheduler options are merged on pod admission;
135136
there is a risk that pods are rejected and builds fail due to colliding pod scheduler values.
136137

137-
The reconcile loop for `BuildRun` could in theory catch this situation ahead of time and fail the
138-
build prior to pod creation. We could also test this scenario and see how Tekton `TaskRuns` behave;
139-
if the `TaskRun` fails with a reasonable error message, then Shipwright `BuildRun`s can simply echo
140-
the information in their status.
138+
In theory the controller for `BuildRuns` can anticipate this situation and fail the build quickly.
139+
However, other Kubernetes controllers do not appear to do this, and thus it may not be worthwhile
140+
to add this extra logic. Checking the `RuntimeClass` for every build also adds overhead to each
141+
reconcile loop, either by making kubernetes apiserver calls directly or adding an informer-based
142+
cache for `RuntimeClass` that provides little value.
141143

142144
## Drawbacks
143145

@@ -147,7 +149,13 @@ Similar drawbacks also exist with respect to cluster admin control over pod sche
147149

148150
## Alternatives
149151

150-
TBD
152+
### Kubernetes User Namespaces
153+
154+
An initial draft of this proposal included Kubernetes
155+
[user namespaces](https://kubernetes.io/docs/concepts/workloads/pods/user-namespaces/) as part of
156+
the scope. This was removed because user namespaces are a beta feature in Kubernetes (as of v1.32),
157+
and did not reach the beta milestone until v1.30. Even with achievement of the beta milestone, the
158+
feature is still disabled by default in Kubernetes v1.32, making it challenging to test.
151159

152160
## Infrastructure Needed [optional]
153161

0 commit comments

Comments
 (0)