Skip to content

Commit

Permalink
Refactor resource settings
Browse files Browse the repository at this point in the history 
- default resource settings for the envoy sidecar can be set via flags
- these can be overridden by annotations on the injected pods
- by default there are not resource settings
- add resource settings to init container and sidecar container. These
are always set so users who require resource settings don't need to turn
them on. They are set to more than double observed usage so they won't
affect any users. They are not customizable via annotation or flag because they
don't change based on use-case. They could be exposed later.
  • Loading branch information
@lkysow
lkysow committed May 26, 2020
1 parent ab967fa commit b32b753
Show file tree
Hide file tree
Showing 10 changed files with 428 additions and 104 deletions.
38 changes: 19 additions & 19 deletions connect-inject/container_init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
)

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

type initContainerCommandData struct {
ServiceName string
ProxyServiceName string
Expand All @@ -36,10 +43,6 @@ type initContainerCommandData struct {
ConsulCACert string
}

type initContainerResources struct {
Resources corev1.ResourceRequirements
}

type initContainerCommandUpstreamData struct {
Name string
LocalPort int32
Expand Down Expand Up @@ -78,20 +81,6 @@ func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Co
panic("No service found. This should be impossible since we default it.")
}

icr := initContainerResources{}
if h.Resources {
icr.Resources = corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(h.CPULimit),
corev1.ResourceMemory: resource.MustParse(h.MemoryLimit),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(h.CPULimit),
corev1.ResourceMemory: resource.MustParse(h.MemoryLimit),
},
}
}

// If a port is specified, then we determine the value of that port
// and register that port for the host service.
if raw, ok := pod.Annotations[annotationPort]; ok && raw != "" {
Expand Down Expand Up @@ -205,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 @@ -242,7 +242,7 @@ func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Co
Value: fmt.Sprintf("$(POD_NAME)-%s", data.ProxyServiceName),
},
},
Resources: icr.Resources,
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)
}
88 changes: 73 additions & 15 deletions connect-inject/envoy_sidecar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package connectinject

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

Expand All @@ -12,7 +13,6 @@ import (
type sidecarContainerCommandData struct {
AuthMethod string
ConsulNamespace string
Resources corev1.ResourceRequirements
}

func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Container, error) {
Expand All @@ -21,19 +21,6 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
ConsulNamespace: h.consulNamespace(k8sNamespace),
}

if h.Resources {
templateData.Resources = corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(h.CPULimit),
corev1.ResourceMemory: resource.MustParse(h.MemoryLimit),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(h.CPULimit),
corev1.ResourceMemory: resource.MustParse(h.MemoryLimit),
},
}
}

// Render the command
var buf bytes.Buffer
tpl := template.Must(template.New("root").Parse(strings.TrimSpace(
Expand All @@ -43,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 @@ -54,7 +46,7 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
},
},
},
Resources: templateData.Resources,
Resources: resources,
VolumeMounts: []corev1.VolumeMount{
{
Name: volumeName,
Expand Down Expand Up @@ -97,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 %s annotation: %s", annotationSidecarProxyCPULimit, 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 %s annotation: %s", annotationSidecarProxyCPURequest, 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 %s annotation: %s", annotationSidecarProxyMemoryLimit, 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 %s annotation: %s", annotationSidecarProxyMemoryRequest, 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
Loading

0 comments on commit b32b753

Please sign in to comment.