Skip to content

Commit

Permalink
don't require user input to account for limitrange min
Browse files Browse the repository at this point in the history
  • Loading branch information
danielhelfand committed Feb 12, 2020
1 parent a1a3280 commit d615273
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 131 deletions.
32 changes: 5 additions & 27 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Creation of a `PipelineRun` will trigger the creation of
- [Cancelling a PipelineRun](#cancelling-a-pipelinerun)
- [Examples](https://github.com/tektoncd/pipeline/tree/master/examples/pipelineruns)
- [Logs](logs.md)
- [LimitRange Name](#limitrange-name)
- [LimitRanges](#limitrange-name)

## Syntax

Expand Down Expand Up @@ -51,10 +51,6 @@ following fields:
follow the instruction [here](taskruns.md#Configuring-default-timeout) to configure the
default timeout, the same way as `TaskRun`.
- [`podTemplate`](#pod-template) - Specifies a [pod template](./podtemplates.md) that will be used as the basis for the `Task` pod.
- [`limitRangeName`](#limitrange-name) - Specifies the name of a LimitRange that exists in the namespace of the `PipelineRun`. This LimitRange's minimum
for container resource requests will be used as part of requesting the appropriate amount of CPU, memory, and ephemeral storage for containers that are
part of the `TaskRuns` of a `PipelineRun`. This property only needs to be specified if the `PipelineRun` is happening in a namespace with a LimitRange
minimum specified for container resource requests.

[kubernetes-overview]:
https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields
Expand Down Expand Up @@ -377,7 +373,7 @@ spec:
status: "PipelineRunCancelled"
```

## LimitRange Name
## LimitRanges

In order to request the minimum amount of resources needed to support the containers
for `steps` that are part of a `TaskRun`, Tekton only requests the maximum values for CPU,
Expand All @@ -387,28 +383,10 @@ All requests that are not the max values are set to zero as a result.

When a [LimitRange](https://kubernetes.io/docs/concepts/policy/limit-range/) is present in a namespace
with a minimum set for container resource requests (i.e. CPU, memory, and ephemeral storage) where `PipelineRuns`
are attempting to run, the `limitRangeName` property must be specified to appropriately apply the LimitRange's
minimum values to containers that are part of a `TaskRun` associated with a `PipelineRun`. This property helps
prevent failures of `TaskRuns` not meeting the minimum requirements specified by a LimitRange for containers.
are attempting to run, Tekton will search through all LimitRanges present in the namespace and use the minimum
set for container resource requests instead of requesting 0.

In the example below, the LimitRange `limit-mem-cpu-per-container` will be applied to all `TaskRuns` associated
with the `PipelineRun`:

```yaml
apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
creationTimestamp: null
generateName: deploy-pipeline-run-
namespace: default
spec:
pipelineRef:
name: deploy-pipeline-hello
limitRangeName: "limit-mem-cpu-per-container"
status: {}
```

An example `PipelineRun` using `limitRangeName` is available [here](../examples/pipelineruns/no-ci/limitrange.yaml).
An example `PipelineRun` with a LimitRange is available [here](../examples/pipelineruns/no-ci/limitrange.yaml).

---

Expand Down
34 changes: 5 additions & 29 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ A `TaskRun` runs until all `steps` have completed or until a failure occurs.
- [Examples](#examples)
- [Sidecars](#sidecars)
- [Logs](logs.md)
- [LimitRange Name](#limitrange-name)
- [LimitRanges](#limitrange-name)

---

Expand Down Expand Up @@ -61,9 +61,6 @@ following fields:
- [`podTemplate`](#pod-template) - Specifies a [pod template](./podtemplates.md) that will be used as the basis for the `Task` pod.
- [`workspaces`](#workspaces) - Specify the actual volumes to use for the
[workspaces](tasks.md#workspaces) declared by a `Task`
- [`limitRangeName`](#limitrange-name) - Specifies the name of a LimitRange that exists in the namespace of the `TaskRun`. This LimitRange's minimum
for container resource requests will be used as part of requesting the appropriate amount of CPU, memory, and ephemeral storage for containers that are
part of a `TaskRun`. This property only needs to be specified if the `TaskRun` is happening in a namespace with a LimitRange minimum specified for container resource requests.

[kubernetes-overview]:
https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields
Expand Down Expand Up @@ -704,7 +701,7 @@ with the `get pods` command. The Pod description will instead show a Status of
Failed and the individual container statuses will correctly reflect how and why
they exited.

## LimitRange Name
## LimitRanges

In order to request the minimum amount of resources needed to support the containers
for `steps` that are part of a `TaskRun`, Tekton only requests the maximum values for CPU,
Expand All @@ -714,31 +711,10 @@ All requests that are not the max values are set to zero as a result.

When a [LimitRange](https://kubernetes.io/docs/concepts/policy/limit-range/) is present in a namespace
with a minimum set for container resource requests (i.e. CPU, memory, and ephemeral storage) where `TaskRuns`
are attempting to run, the `limitRangeName` property must be specified to appropriately apply the LimitRange's
minimum values to `steps` that are part of a `TaskRun`. This property helps prevent failures of `TaskRuns` not
meeting the minimum requirements specified by a LimitRange for containers.
are attempting to run, Tekton will search through all LimitRanges present in the namespace and use the minimum
set for container resource requests instead of requesting 0.

In the example below, the LimitRange `limit-mem-cpu-per-container` will be applied to all `steps` associated with
a `TaskRun` in namespace `default`:

```yaml
apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
creationTimestamp: null
generateName: echo-hello-world-run-
namespace: default
spec:
inputs: {}
outputs: {}
serviceAccountName: ""
taskRef:
name: echo-hello-world
timeout: 1h0m0s
limitRangeName: "limit-mem-cpu-per-container"
```

An example `TaskRun` using `limitRangeName` is available [here](../examples/taskruns/no-ci/limitrange.yaml).
An example `TaskRun` with a LimitRange is available [here](../examples/taskruns/no-ci/limitrange.yaml).

---

Expand Down
1 change: 0 additions & 1 deletion examples/pipelineruns/no-ci/limitrange.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,4 @@ metadata:
spec:
pipelineRef:
name: pipeline-hello
limitRangeName: "limit-mem-cpu-per-container"
status: {}
1 change: 0 additions & 1 deletion examples/taskruns/no-ci/limitrange.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,5 @@ spec:
taskRef:
name: echo-hello-world
timeout: 1h0m0s
limitRangeName: "limit-mem-cpu-per-container"
status:
podName: ""
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.13

require (
cloud.google.com/go v0.47.0 // indirect
cloud.google.com/go/storage v1.0.0
contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
github.com/GoogleCloudPlatform/cloud-builders/gcs-fetcher v0.0.0-20191203181535-308b93ad1f39
github.com/cloudevents/sdk-go v1.0.0
Expand Down Expand Up @@ -38,6 +39,7 @@ require (
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
golang.org/x/sys v0.0.0-20191210023423-ac6580df4449 // indirect
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
google.golang.org/api v0.15.0
google.golang.org/appengine v1.6.5 // indirect
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
gopkg.in/yaml.v2 v2.2.5 // indirect
Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ type PipelineRunSpec struct {
// with those declared in the pipeline.
// +optional
Workspaces []WorkspaceBinding `json:"workspaces,omitempty"`
// Used to specify name of LimitRange that exists in namespace
// where PipelineRun will run so that the LimitRange's minimum for
// container requests can be used by containers of TaskRuns associated
// with PipelineRun
// +optional
LimitRangeName string `json:"limitRangeName"`
}

// PipelineRunSpecStatus defines the pipelinerun spec status the user can provide
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1alpha1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ type TaskRunSpec struct {
// Workspaces is a list of WorkspaceBindings from volumes to workspaces.
// +optional
Workspaces []WorkspaceBinding `json:"workspaces,omitempty"`
// Used to specify name of LimitRange that exists in namespace
// where TaskRun will run so that the LimitRange's minimum for
// container requests can be used by containers of TaskRun
// +optional
LimitRangeName string `json:"limitRangeName"`
}

// TaskRunSpecStatus defines the taskrun spec status the user can provide
Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/pipeline/v1alpha2/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,6 @@ type PipelineRunSpec struct {
// with those declared in the pipeline.
// +optional
Workspaces []WorkspaceBinding `json:"workspaces,omitempty"`
// Used to specify name of LimitRange that exists in namespace
// where PipelineRun will run so that the LimitRange's minimum for
// container requests can be used by containers of TaskRuns associated
// with PipelineRun
// +optional
LimitRangeName string `json:"limitRangeName"`
}

// PipelineRunSpecStatus defines the pipelinerun spec status the user can provide
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1alpha2/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ type TaskRunSpec struct {
// Workspaces is a list of WorkspaceBindings from volumes to workspaces.
// +optional
Workspaces []WorkspaceBinding `json:"workspaces,omitempty"`
// Used to specify name of LimitRange that exists in namespace
// where TaskRun will run so that the LimitRange's minimum for
// container requests can be used by containers of TaskRun
// +optional
LimitRangeName string `json:"limitRangeName"`
}

// TaskRunSpecStatus defines the taskrun spec status the user can provide
Expand Down
43 changes: 34 additions & 9 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,13 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
initContainers = append(initContainers, entrypointInit)
volumes = append(volumes, toolsVolume, downwardVolume)

// If present on TaskRunSpec, use LimitRangeName to get LimitRange
// so it can be used in resolveResourceRequests
var limitRange *corev1.LimitRange
if taskRun.Spec.LimitRangeName != "" {
limitRange, err = kubeclient.CoreV1().LimitRanges(taskRun.Namespace).Get(taskRun.Spec.LimitRangeName, metav1.GetOptions{})
if err != nil {
return nil, err
}
limitRangeMin, err := getLimitRangeMinimum(taskRun.Namespace, kubeclient)
if err != nil {
return nil, err
}

// Zero out non-max resource requests.
stepContainers = resolveResourceRequests(stepContainers, limitRange)
stepContainers = resolveResourceRequests(stepContainers, limitRangeMin)

// Add implicit env vars.
// They're prepended to the list, so that if the user specified any
Expand Down Expand Up @@ -262,3 +257,33 @@ func MakeLabels(s *v1alpha1.TaskRun) map[string]string {
labels[taskRunLabelKey] = s.Name
return labels
}

// getLimitRangeMinimum gets all LimitRanges in a namespace and
// searches for if a container minimum is specified. Due to
// https://github.com/kubernetes/kubernetes/issues/79496, the
// max LimitRange minimum must be found in the event of conflicting
// container minimums specified.
func getLimitRangeMinimum(namespace string, kubeclient kubernetes.Interface) (corev1.ResourceList, error) {
limitRanges, err := kubeclient.CoreV1().LimitRanges(namespace).List(metav1.ListOptions{})
if err != nil {
return nil, err
}

min := allZeroQty()
for _, lr := range limitRanges.Items {
lrItems := lr.Spec.Limits
for _, lrItem := range lrItems {
if lrItem.Type == corev1.LimitTypeContainer {
if lrItem.Min != nil {
for k, v := range lrItem.Min {
if v.Cmp(min[k]) > 0 {
min[k] = v
}
}
}
}
}
}

return min, nil
}
34 changes: 9 additions & 25 deletions pkg/pod/resource_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func allZeroQty() corev1.ResourceList {
}
}

func resolveResourceRequests(containers []corev1.Container, limitRange *corev1.LimitRange) []corev1.Container {
func resolveResourceRequests(containers []corev1.Container, limitRangeMin corev1.ResourceList) []corev1.Container {
max := allZeroQty()
resourceNames := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage}
maxIndicesByResource := make(map[corev1.ResourceName]int, len(resourceNames))
Expand All @@ -51,43 +51,27 @@ func resolveResourceRequests(containers []corev1.Container, limitRange *corev1.L
}
}

// Get limitrange minimum for container requests so they won't
// be zeroed out if minimum is specified in namespace
var limitRangeItems []corev1.LimitRangeItem
if limitRange != nil {
limitRangeItems = limitRange.Spec.Limits
}
min := allZeroQty()
for _, limitRangeItem := range limitRangeItems {
if limitRangeItem.Type == corev1.LimitTypeContainer {
if limitRangeItem.Min != nil {
min = limitRangeItem.Min
}
break
}
}

// Use zeroQty if request value is not set for min
if min[corev1.ResourceCPU] == emptyResourceQuantity {
min[corev1.ResourceCPU] = zeroQty
if limitRangeMin[corev1.ResourceCPU] == emptyResourceQuantity {
limitRangeMin[corev1.ResourceCPU] = zeroQty
}
if min[corev1.ResourceMemory] == emptyResourceQuantity {
min[corev1.ResourceMemory] = zeroQty
if limitRangeMin[corev1.ResourceMemory] == emptyResourceQuantity {
limitRangeMin[corev1.ResourceMemory] = zeroQty
}
if min[corev1.ResourceEphemeralStorage] == emptyResourceQuantity {
min[corev1.ResourceEphemeralStorage] = zeroQty
if limitRangeMin[corev1.ResourceEphemeralStorage] == emptyResourceQuantity {
limitRangeMin[corev1.ResourceEphemeralStorage] = zeroQty
}

// Set all non max resource requests to 0. Leave max request at index
// originally defined to account for limit of step.
for i := range containers {
if containers[i].Resources.Requests == nil {
containers[i].Resources.Requests = min
containers[i].Resources.Requests = limitRangeMin
continue
}
for _, resourceName := range resourceNames {
if maxIndicesByResource[resourceName] != i {
containers[i].Resources.Requests[resourceName] = min[resourceName]
containers[i].Resources.Requests[resourceName] = limitRangeMin[resourceName]
}
}
}
Expand Down
23 changes: 7 additions & 16 deletions pkg/pod/resource_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func TestResolveResourceRequests_No_LimitRange(t *testing.T) {
},
} {
t.Run(c.desc, func(t *testing.T) {
got := resolveResourceRequests(c.in, nil)
got := resolveResourceRequests(c.in, allZeroQty())
if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" {
t.Errorf("Diff(-want, +got): %s", d)
}
Expand Down Expand Up @@ -331,21 +331,12 @@ func TestResolveResourceRequests_LimitRange(t *testing.T) {
},
} {
t.Run(c.desc, func(t *testing.T) {
got := resolveResourceRequests(c.in,
&corev1.LimitRange{
Spec: corev1.LimitRangeSpec{
Limits: []corev1.LimitRangeItem{
{
Type: "Container",
Min: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("99Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
},
},
},
},
})
got := resolveResourceRequests(c.in, corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("99Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("100m"),
},
)
if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" {
t.Errorf("Diff(-want, +got): %s", d)
}
Expand Down
1 change: 0 additions & 1 deletion pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,6 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr *
ServiceAccountName: pr.GetServiceAccountName(rprt.PipelineTask.Name),
Timeout: getTaskRunTimeout(pr),
PodTemplate: pr.Spec.PodTemplate,
LimitRangeName: pr.Spec.LimitRangeName,
}}

if rprt.ResolvedTaskResources.TaskName != "" {
Expand Down

0 comments on commit d615273

Please sign in to comment.