Skip to content

Commit

Permalink
Merge pull request #267 from hashicorp/resource-limit
Browse files Browse the repository at this point in the history 
Support resource settings
  • Loading branch information
@lkysow
lkysow authored Jun 3, 2020
2 parents 55d42aa + d0a01a0 commit 8267732
Show file tree
Hide file tree
Showing 10 changed files with 454 additions and 9 deletions.
20 changes: 20 additions & 0 deletions connect-inject/container_init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ import (
"text/template"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

const (
initContainerCPULimit = "50m"
initContainerCPURequest = "50m"
initContainerMemoryLimit = "25Mi"
initContainerMemoryRequest = "25Mi"
)

type initContainerCommandData struct {
Expand Down Expand Up @@ -186,6 +194,17 @@ func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Co
return corev1.Container{}, err
}

resources := corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(initContainerCPULimit),
corev1.ResourceMemory: resource.MustParse(initContainerMemoryLimit),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(initContainerCPURequest),
corev1.ResourceMemory: resource.MustParse(initContainerMemoryRequest),
},
}

return corev1.Container{
Name: "consul-connect-inject-init",
Image: h.ImageConsul,
Expand Down Expand Up @@ -223,6 +242,7 @@ func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Co
Value: fmt.Sprintf("$(POD_NAME)-%s", data.ProxyServiceName),
},
},
Resources: resources,
VolumeMounts: volMounts,
Command: []string{"/bin/sh", "-ec", buf.String()},
}, nil
Expand Down
33 changes: 33 additions & 0 deletions connect-inject/container_init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -1401,3 +1402,35 @@ EOF`)
export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
export CONSUL_GRPC_ADDR="${HOST_IP}:8502"`)
}

func TestHandlerContainerInit_Resources(t *testing.T) {
require := require.New(t)
h := Handler{}
pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
},
},

Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "web",
},
},
},
}
container, err := h.containerInit(pod, k8sNamespace)
require.NoError(err)
require.Equal(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(initContainerCPULimit),
corev1.ResourceMemory: resource.MustParse(initContainerMemoryLimit),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(initContainerCPURequest),
corev1.ResourceMemory: resource.MustParse(initContainerMemoryRequest),
},
}, container.Resources)
}
74 changes: 74 additions & 0 deletions connect-inject/envoy_sidecar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package connectinject

import (
"bytes"
"fmt"
"strings"
"text/template"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

type sidecarContainerCommandData struct {
Expand All @@ -28,6 +30,11 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
return corev1.Container{}, err
}

resources, err := h.envoySidecarResources(pod)
if err != nil {
return corev1.Container{}, err
}

container := corev1.Container{
Name: "consul-connect-envoy-sidecar",
Image: h.ImageEnvoy,
Expand All @@ -39,6 +46,7 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
},
},
},
Resources: resources,
VolumeMounts: []corev1.VolumeMount{
{
Name: volumeName,
Expand Down Expand Up @@ -81,6 +89,72 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
return container, nil
}

func (h *Handler) envoySidecarResources(pod *corev1.Pod) (corev1.ResourceRequirements, error) {
resources := corev1.ResourceRequirements{
Limits: corev1.ResourceList{},
Requests: corev1.ResourceList{},
}
// zeroQuantity is used for comparison to see if a quantity was explicitly
// set.
var zeroQuantity resource.Quantity

// NOTE: We only want to set the limit/request if the default or annotation
// was explicitly set. If it's not explicitly set, it will be the zero value
// which would show up in the pod spec as being explicitly set to zero if we
// set that key, e.g. "cpu" to zero.
// We want it to not show up in the pod spec at all if if it's not explicitly
// set so that users aren't wondering why it's set to 0 when they didn't specify
// a request/limit. If they have explicitly set it to 0 then it will be set
// to 0 in the pod spec because we're doing a comparison to the zero-valued
// struct.

// CPU Limit.
if anno, ok := pod.Annotations[annotationSidecarProxyCPULimit]; ok {
cpuLimit, err := resource.ParseQuantity(anno)
if err != nil {
return corev1.ResourceRequirements{}, fmt.Errorf("parsing annotation %s:%q: %s", annotationSidecarProxyCPULimit, anno, err)
}
resources.Limits[corev1.ResourceCPU] = cpuLimit
} else if h.DefaultProxyCPULimit != zeroQuantity {
resources.Limits[corev1.ResourceCPU] = h.DefaultProxyCPULimit
}

// CPU Request.
if anno, ok := pod.Annotations[annotationSidecarProxyCPURequest]; ok {
cpuRequest, err := resource.ParseQuantity(anno)
if err != nil {
return corev1.ResourceRequirements{}, fmt.Errorf("parsing annotation %s:%q: %s", annotationSidecarProxyCPURequest, anno, err)
}
resources.Requests[corev1.ResourceCPU] = cpuRequest
} else if h.DefaultProxyCPURequest != zeroQuantity {
resources.Requests[corev1.ResourceCPU] = h.DefaultProxyCPURequest
}

// Memory Limit.
if anno, ok := pod.Annotations[annotationSidecarProxyMemoryLimit]; ok {
memoryLimit, err := resource.ParseQuantity(anno)
if err != nil {
return corev1.ResourceRequirements{}, fmt.Errorf("parsing annotation %s:%q: %s", annotationSidecarProxyMemoryLimit, anno, err)
}
resources.Limits[corev1.ResourceMemory] = memoryLimit
} else if h.DefaultProxyMemoryLimit != zeroQuantity {
resources.Limits[corev1.ResourceMemory] = h.DefaultProxyMemoryLimit
}

// Memory Request.
if anno, ok := pod.Annotations[annotationSidecarProxyMemoryRequest]; ok {
memoryRequest, err := resource.ParseQuantity(anno)
if err != nil {
return corev1.ResourceRequirements{}, fmt.Errorf("parsing annotation %s:%q: %s", annotationSidecarProxyMemoryRequest, anno, err)
}
resources.Requests[corev1.ResourceMemory] = memoryRequest
} else if h.DefaultProxyMemoryRequest != zeroQuantity {
resources.Requests[corev1.ResourceMemory] = h.DefaultProxyMemoryRequest
}

return resources, nil
}

const sidecarPreStopCommandTpl = `
/consul/connect-inject/consul services deregister \
{{- if .AuthMethod }}
Expand Down
180 changes: 180 additions & 0 deletions connect-inject/envoy_sidecar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -203,3 +204,182 @@ func TestHandlerEnvoySidecar_NamespacesAndAuthMethod(t *testing.T) {
/consul/connect-inject/consul logout \
-token-file="/consul/connect-inject/acl-token"`)
}

func TestHandlerEnvoySidecar_Resources(t *testing.T) {
mem1 := resource.MustParse("100Mi")
mem2 := resource.MustParse("200Mi")
cpu1 := resource.MustParse("100m")
cpu2 := resource.MustParse("200m")
zero := resource.MustParse("0")

cases := map[string]struct {
handler Handler
annotations map[string]string
expResources corev1.ResourceRequirements
expErr string
}{
"no defaults, no annotations": {
handler: Handler{},
annotations: nil,
expResources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{},
Requests: corev1.ResourceList{},
},
},
"all defaults, no annotations": {
handler: Handler{
DefaultProxyCPURequest: cpu1,
DefaultProxyCPULimit: cpu2,
DefaultProxyMemoryRequest: mem1,
DefaultProxyMemoryLimit: mem2,
},
annotations: nil,
expResources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: cpu2,
corev1.ResourceMemory: mem2,
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: cpu1,
corev1.ResourceMemory: mem1,
},
},
},
"no defaults, all annotations": {
handler: Handler{},
annotations: map[string]string{
annotationSidecarProxyCPURequest: "100m",
annotationSidecarProxyMemoryRequest: "100Mi",
annotationSidecarProxyCPULimit: "200m",
annotationSidecarProxyMemoryLimit: "200Mi",
},
expResources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: cpu2,
corev1.ResourceMemory: mem2,
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: cpu1,
corev1.ResourceMemory: mem1,
},
},
},
"annotations override defaults": {
handler: Handler{
DefaultProxyCPURequest: zero,
DefaultProxyCPULimit: zero,
DefaultProxyMemoryRequest: zero,
DefaultProxyMemoryLimit: zero,
},
annotations: map[string]string{
annotationSidecarProxyCPURequest: "100m",
annotationSidecarProxyMemoryRequest: "100Mi",
annotationSidecarProxyCPULimit: "200m",
annotationSidecarProxyMemoryLimit: "200Mi",
},
expResources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: cpu2,
corev1.ResourceMemory: mem2,
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: cpu1,
corev1.ResourceMemory: mem1,
},
},
},
"defaults set to zero, no annotations": {
handler: Handler{
DefaultProxyCPURequest: zero,
DefaultProxyCPULimit: zero,
DefaultProxyMemoryRequest: zero,
DefaultProxyMemoryLimit: zero,
},
annotations: nil,
expResources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: zero,
corev1.ResourceMemory: zero,
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: zero,
corev1.ResourceMemory: zero,
},
},
},
"annotations set to 0": {
handler: Handler{},
annotations: map[string]string{
annotationSidecarProxyCPURequest: "0",
annotationSidecarProxyMemoryRequest: "0",
annotationSidecarProxyCPULimit: "0",
annotationSidecarProxyMemoryLimit: "0",
},
expResources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: zero,
corev1.ResourceMemory: zero,
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: zero,
corev1.ResourceMemory: zero,
},
},
},
"invalid cpu request": {
handler: Handler{},
annotations: map[string]string{
annotationSidecarProxyCPURequest: "invalid",
},
expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-cpu-request:\"invalid\": quantities must match the regular expression",
},
"invalid cpu limit": {
handler: Handler{},
annotations: map[string]string{
annotationSidecarProxyCPULimit: "invalid",
},
expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-cpu-limit:\"invalid\": quantities must match the regular expression",
},
"invalid memory request": {
handler: Handler{},
annotations: map[string]string{
annotationSidecarProxyMemoryRequest: "invalid",
},
expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-memory-request:\"invalid\": quantities must match the regular expression",
},
"invalid memory limit": {
handler: Handler{},
annotations: map[string]string{
annotationSidecarProxyMemoryLimit: "invalid",
},
expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-memory-limit:\"invalid\": quantities must match the regular expression",
},
}

for name, c := range cases {
t.Run(name, func(tt *testing.T) {
require := require.New(tt)
pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: c.annotations,
},

Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "web",
},
},
},
}
container, err := c.handler.envoySidecar(pod, k8sNamespace)
if c.expErr != "" {
require.NotNil(err)
require.Contains(err.Error(), c.expErr)
} else {
require.NoError(err)
require.Equal(c.expResources, container.Resources)
}
})
}
}
Loading

0 comments on commit 8267732

Please sign in to comment.