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

Check for LimitRange Minimum for TaskRun Container Requests #1991

Merged
merged 1 commit into from
Feb 5, 2020
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
2 changes: 1 addition & 1 deletion config/200-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ metadata:
name: tekton-pipelines-admin
rules:
- apiGroups: [""]
resources: ["pods", "pods/log", "namespaces", "secrets", "events", "serviceaccounts", "configmaps", "persistentvolumeclaims"]
resources: ["pods", "pods/log", "namespaces", "secrets", "events", "serviceaccounts", "configmaps", "persistentvolumeclaims", "limitranges"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
- apiGroups: ["apps"]
resources: ["deployments"]
Expand Down
37 changes: 36 additions & 1 deletion docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +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)

## Syntax

Expand All @@ -36,7 +37,6 @@ following fields:
your `PipelineRun` resource object.
- [`pipelineRef` or `pipelineSpec`](#specifiying-a-pipeline) - Specifies the [`Pipeline`](pipelines.md) you want to run.
- Optional:

- [`resources`](#resources) - Specifies which
[`PipelineResources`](resources.md) to use for this `PipelineRun`.
- [`serviceAccountName`](#service-account) - Specifies a `ServiceAccount` resource
Expand All @@ -51,6 +51,10 @@ 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 @@ -373,6 +377,37 @@ spec:
status: "PipelineRunCancelled"
```

## LimitRange Name

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,
memory, and ephemeral storage from the `steps` that are part of a TaskRun. Only the max
resource request values are needed since `steps` only execute one at a time in `TaskRun` pod.
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.

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: {}
```

---

Except as otherwise noted, the content of this page is licensed under the
Expand Down
38 changes: 38 additions & 0 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +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)

---

Expand Down Expand Up @@ -60,6 +61,9 @@ 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 @@ -700,6 +704,40 @@ 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

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,
memory, and ephemeral storage from the `steps` that are part of a TaskRun. Only the max
resource request values are needed since `steps` only execute one at a time in `TaskRun` pod.
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.

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"
```

---

Except as otherwise noted, the content of this page is licensed under the
Expand Down
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/prometheus v0.1.0 // indirect
contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
github.com/GoogleCloudPlatform/cloud-builders/gcs-fetcher v0.0.0-20191203181535-308b93ad1f39
Expand Down Expand Up @@ -40,6 +41,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.10.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: 6 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use a comment here describing the field's purpose. // LimitRangeName specifies the limit range to use for minimum resource values in task pods or similar.

Same comment for taskrun_types in v1alpha1 and 2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Added along with some docs for pipelineruns/taskruns.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice

LimitRangeName string `json:"limitRangeName"`
}

// PipelineRunSpecStatus defines the pipelinerun spec status the user can provide
Expand Down
7 changes: 5 additions & 2 deletions pkg/apis/pipeline/v1alpha1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,17 @@ type TaskRunSpec struct {
// Refer Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
// +optional
Timeout *metav1.Duration `json:"timeout,omitempty"`

// PodTemplate holds pod specific configuration
// +optional
PodTemplate *PodTemplate `json:"podTemplate,omitempty"`

// 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
7 changes: 5 additions & 2 deletions pkg/apis/pipeline/v1alpha2/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ type TaskRunSpec struct {
// Refer Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
// +optional
Timeout *metav1.Duration `json:"timeout,omitempty"`

// PodTemplate holds pod specific configuration
PodTemplate PodTemplate `json:"podTemplate,omitempty"`

// 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
12 changes: 11 additions & 1 deletion pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,18 @@ 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
}
}

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

// Add implicit env vars.
// They're prepended to the list, so that if the user specified any
Expand Down
34 changes: 31 additions & 3 deletions pkg/pod/resource_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
)

var emptyResourceQuantity = resource.Quantity{}
var zeroQty = resource.MustParse("0")

func allZeroQty() corev1.ResourceList {
Expand All @@ -31,7 +32,7 @@ func allZeroQty() corev1.ResourceList {
}
}

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

// 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 min[corev1.ResourceMemory] == emptyResourceQuantity {
min[corev1.ResourceMemory] = zeroQty
}
if min[corev1.ResourceEphemeralStorage] == emptyResourceQuantity {
min[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 = allZeroQty()
containers[i].Resources.Requests = min
continue
}
for _, resourceName := range resourceNames {
if maxIndicesByResource[resourceName] != i {
containers[i].Resources.Requests[resourceName] = zeroQty
containers[i].Resources.Requests[resourceName] = min[resourceName]
}
}
}
Expand Down
Loading