Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix docs and remove redundant code for LimitRanges #5351

Merged
merged 1 commit into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions docs/compute-resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,23 +158,18 @@ will be applied to the init containers that Tekton injects into a `TaskRun`'s po

### Requests

If a container does not have requests defined, the resulting container's requests are the larger of:
- the LimitRange minimum resource requests
- the LimitRange default resource requests (for init and Sidecar containers) or the LimitRange default resource requests divided by the number of Step containers (for Step containers)
If a Step container does not have requests defined, Tekton will divide a LimitRange's `defaultRequests` by the number of Step containers and apply these requests to the Steps.
This results in a TaskRun with overall requests equal to LimitRange `defaultRequests`.
If this value is less than the LimitRange minimum, the LimitRange minimum will be used instead.
LimitRange `defaultRequests` are applied as-is to init containers or Sidecar containers that don't specify requests.

If a container has requests defined, the resulting container's requests are the larger of:
- the container's requests
- the LimitRange minimum resource requests
Containers that do specify requests will not be modified. If these requests are lower than LimitRange minimums, Kubernetes will reject the resulting TaskRun's pod.

### Limits

If a container does not have limits defined, the resulting container's limits are the smaller of:
- the LimitRange maximum resource limits
- the LimitRange default resource limits

If a container has limits defined, the resulting container's limits are the smaller of:
- the container's limits
- the LimitRange maximum resource limits
Tekton does not adjust container limits, regardless of whether a container is a Step, Sidecar, or init container.
If a container does not have limits defined, Kubernetes will apply the LimitRange `default` to the container's limits.
If a container does define limits, and they are less than the LimitRange `default`, Kubernetes will reject the resulting TaskRun's pod.

### Examples

Expand Down
72 changes: 5 additions & 67 deletions pkg/internal/computeresources/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,54 +67,24 @@ func transformPodBasedOnLimitRange(p *corev1.Pod, limitRange *corev1.LimitRange)
}

// FIXME(#4230) maxLimitRequestRatio to support later
defaultContainerLimits := getDefaultLimits(limitRange)
defaultContainerRequests := getDefaultContainerRequest(limitRange)
defaultStepContainerRequests := getDefaultStepContainerRequest(limitRange, nbStepContainers)

for i := range p.Spec.InitContainers {
// We are trying to set the smallest requests possible
if p.Spec.InitContainers[i].Resources.Requests == nil {
p.Spec.InitContainers[i].Resources.Requests = defaultContainerRequests
} else {
for _, name := range resourceNames {
setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Requests, defaultContainerRequests)
}
}
// We are trying to set the highest limits possible
if p.Spec.InitContainers[i].Resources.Limits == nil {
p.Spec.InitContainers[i].Resources.Limits = defaultContainerLimits
} else {
for _, name := range resourceNames {
setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Limits, defaultContainerLimits)
}
}
}

for i, c := range p.Spec.Containers {
var defaultRequests = defaultContainerRequests
if pod.IsContainerStep(c.Name) {
defaultRequests = defaultStepContainerRequests
if !pod.IsContainerStep(c.Name) {
continue
}

if p.Spec.Containers[i].Resources.Requests == nil {
p.Spec.Containers[i].Resources.Requests = defaultRequests
} else {
for _, name := range resourceNames {
setRequestsOrLimits(name, p.Spec.Containers[i].Resources.Requests, defaultRequests)
}
}
if p.Spec.Containers[i].Resources.Limits == nil {
p.Spec.Containers[i].Resources.Limits = defaultContainerLimits
p.Spec.Containers[i].Resources.Requests = defaultStepContainerRequests
} else {
for _, name := range resourceNames {
setRequestsOrLimits(name, p.Spec.Containers[i].Resources.Limits, defaultContainerLimits)
setRequests(name, p.Spec.Containers[i].Resources.Requests, defaultStepContainerRequests)
}
}
}
return p
}

func setRequestsOrLimits(name corev1.ResourceName, dst, src corev1.ResourceList) {
func setRequests(name corev1.ResourceName, dst, src corev1.ResourceList) {
if compare.IsZero(dst[name]) && !compare.IsZero(src[name]) {
dst[name] = src[name]
}
Expand Down Expand Up @@ -158,35 +128,3 @@ func getDefaultStepContainerRequest(limitRange *corev1.LimitRange, nbContainers
}
return r
}

// Returns the default requests to use for each init container, determined by the LimitRange default requests and minimums
func getDefaultContainerRequest(limitRange *corev1.LimitRange) corev1.ResourceList {
// Support only Type Container to start with
var r corev1.ResourceList
for _, item := range limitRange.Spec.Limits {
// Only support LimitTypeContainer
if item.Type == corev1.LimitTypeContainer {
if item.DefaultRequest != nil {
r = item.DefaultRequest
} else if item.Min != nil {
r = item.Min
}
}
}
return r
}

func getDefaultLimits(limitRange *corev1.LimitRange) corev1.ResourceList {
// Support only Type Container to start with
var l corev1.ResourceList
for _, item := range limitRange.Spec.Limits {
if item.Type == corev1.LimitTypeContainer {
if item.Default != nil {
l = item.Default
} else if item.Max != nil {
l = item.Max
}
}
}
return l
}
Loading