Don't Require User Input to Account for LimitRange Min #2020
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
the
size/L
|
👍 Nice improvement! This makes sense to me. |
Great! I am comfortable with this as stands and think it makes more sense to not include as part of taskrun/pipelinerun specs at least for initial release of this feature. If we find there is reason for allowing some level of user control on this, it can be added down the line. But this makes more sense to me as a starting point. |
ghost
mentioned this pull request
danielhelfand
force-pushed
the
remove_limitrange_name
branch
2 times, most recently
from
40266f6
to
d615273
Compare
|
/test pull-tekton-pipeline-integration-tests |
danielhelfand
force-pushed
the
remove_limitrange_name
branch
from
d615273
to
5820166
Compare
| for _, lrItem := range lrItems { | ||
| if lrItem.Type == corev1.LimitTypeContainer { | ||
| if lrItem.Min != nil { | ||
| for k, v := range lrItem.Min { |
There was a problem hiding this comment.
Open to not having this approach, which is to search for the max LimitRange container min in the event there are conflicting container mins.
An alternative approach would just be to break the loop after the first container min is found and let it potentially fail.
danielhelfand
force-pushed
the
remove_limitrange_name
branch
from
5820166
to
b9de4fb
Compare
tekton-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Opening this as a proposal to improve upon #1991. This pull request removes the requirement of having to use
limitRangeNameto specify a LimitRange minimum to account for with TaskRuns.This approach may be easier for users who do not have permissions to query LimitRanges or do not know which LimitRange has container requests defined in a namespace.
Submitter Checklist
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes