From 9d1cdb8d2ba7079ddb1bbadbea0d6538c840e7b3 Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Mon, 6 Jul 2020 17:11:13 -0500 Subject: [PATCH] Add resource setting flags for lifecycle sidecar and connect inject init container Adds default flag values so this isn't a breaking change. Moves the flag parsing and validation into a separate function which includes the translation into a ResourceRequirement. --- CHANGELOG.md | 4 + connect-inject/container_init.go | 24 +-- connect-inject/container_init_test.go | 21 ++- connect-inject/handler.go | 12 +- connect-inject/lifecycle_sidecar.go | 23 +-- connect-inject/lifecycle_sidecar_test.go | 50 +++--- subcommand/inject-connect/command.go | 179 +++++++++++++++++++--- subcommand/inject-connect/command_test.go | 77 ++++++++++ 8 files changed, 292 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 116eca2644..5246897355 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## UNRELEASED +IMPROVEMENTS: + +* Connect: Add resource request and limit flags for the injected init and lifecycle sidecar containers [[GH-298](https://github.com/hashicorp/consul-k8s/pull/298)]. + BUG FIXES: * Connect: Respect allow/deny list flags when namespaces are disabled. [[GH-296](https://github.com/hashicorp/consul-k8s/issues/296)] diff --git a/connect-inject/container_init.go b/connect-inject/container_init.go index 80de5cd43a..244efb9278 100644 --- a/connect-inject/container_init.go +++ b/connect-inject/container_init.go @@ -8,17 +8,6 @@ import ( "text/template" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) - -// The init container is bound in memory usage by the size of the consul binary -// as it issues a cpu of the binary to a shared volume. The limit is set to be -// slightly larger than the binary to ensure we don't get OOM killed during the cp. -const ( - initContainerCPULimit = "50m" - initContainerCPURequest = "50m" - initContainerMemoryLimit = "150Mi" - initContainerMemoryRequest = "25Mi" ) type initContainerCommandData struct { @@ -203,17 +192,6 @@ 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, @@ -251,7 +229,7 @@ func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Co Value: fmt.Sprintf("$(POD_NAME)-%s", data.ProxyServiceName), }, }, - Resources: resources, + Resources: h.InitContainerResources, VolumeMounts: volMounts, Command: []string{"/bin/sh", "-ec", buf.String()}, }, nil diff --git a/connect-inject/container_init_test.go b/connect-inject/container_init_test.go index 1115439a3e..a9a823ecea 100644 --- a/connect-inject/container_init_test.go +++ b/connect-inject/container_init_test.go @@ -1488,7 +1488,18 @@ export CONSUL_GRPC_ADDR="${HOST_IP}:8502"`) func TestHandlerContainerInit_Resources(t *testing.T) { require := require.New(t) - h := Handler{} + h := Handler{ + InitContainerResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("10Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("20m"), + corev1.ResourceMemory: resource.MustParse("25Mi"), + }, + }, + } pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -1508,12 +1519,12 @@ func TestHandlerContainerInit_Resources(t *testing.T) { require.NoError(err) require.Equal(corev1.ResourceRequirements{ Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(initContainerCPULimit), - corev1.ResourceMemory: resource.MustParse(initContainerMemoryLimit), + corev1.ResourceCPU: resource.MustParse("20m"), + corev1.ResourceMemory: resource.MustParse("25Mi"), }, Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(initContainerCPURequest), - corev1.ResourceMemory: resource.MustParse(initContainerMemoryRequest), + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("10Mi"), }, }, container.Resources) } diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 631e1f299f..ccc1c162df 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -78,6 +78,7 @@ const ( // service is synced (i.e. re-registered) with the local agent. annotationSyncPeriod = "consul.hashicorp.com/connect-sync-period" + // annotations for sidecar proxy resource limits annotationSidecarProxyCPULimit = "consul.hashicorp.com/sidecar-proxy-cpu-limit" annotationSidecarProxyCPURequest = "consul.hashicorp.com/sidecar-proxy-cpu-request" annotationSidecarProxyMemoryLimit = "consul.hashicorp.com/sidecar-proxy-memory-limit" @@ -169,12 +170,21 @@ type Handler struct { // Only necessary if ACLs are enabled. CrossNamespaceACLPolicy string - // Default resource settings for sidecar proxies. + // Default resource settings for sidecar proxies. Some of these + // fields may be empty. DefaultProxyCPURequest resource.Quantity DefaultProxyCPULimit resource.Quantity DefaultProxyMemoryRequest resource.Quantity DefaultProxyMemoryLimit resource.Quantity + // Resource settings for init container. All of these fields + // will be populated by the defaults provided in the initial flags. + InitContainerResources corev1.ResourceRequirements + + // Resource settings for lifecycle sidecar. All of these fields + // will be populated by the defaults provided in the initial flags. + LifecycleSidecarResources corev1.ResourceRequirements + // Log Log hclog.Logger } diff --git a/connect-inject/lifecycle_sidecar.go b/connect-inject/lifecycle_sidecar.go index cc3f509b31..ba71bea04b 100644 --- a/connect-inject/lifecycle_sidecar.go +++ b/connect-inject/lifecycle_sidecar.go @@ -1,17 +1,9 @@ package connectinject import ( - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - "strings" -) -const ( - lifecycleContainerCPULimit = "20m" - lifecycleContainerCPURequest = "20m" - lifecycleContainerMemoryLimit = "25Mi" - lifecycleContainerMemoryRequest = "25Mi" + corev1 "k8s.io/api/core/v1" ) func (h *Handler) lifecycleSidecar(pod *corev1.Pod) corev1.Container { @@ -61,17 +53,6 @@ func (h *Handler) lifecycleSidecar(pod *corev1.Pod) corev1.Container { }) } - resources := corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(lifecycleContainerCPULimit), - corev1.ResourceMemory: resource.MustParse(lifecycleContainerMemoryLimit), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(lifecycleContainerCPURequest), - corev1.ResourceMemory: resource.MustParse(lifecycleContainerMemoryRequest), - }, - } - return corev1.Container{ Name: "consul-connect-lifecycle-sidecar", Image: h.ImageConsulK8S, @@ -83,6 +64,6 @@ func (h *Handler) lifecycleSidecar(pod *corev1.Pod) corev1.Container { }, }, Command: command, - Resources: resources, + Resources: h.LifecycleSidecarResources, } } diff --git a/connect-inject/lifecycle_sidecar_test.go b/connect-inject/lifecycle_sidecar_test.go index a7a8162d62..b18db75094 100644 --- a/connect-inject/lifecycle_sidecar_test.go +++ b/connect-inject/lifecycle_sidecar_test.go @@ -1,12 +1,26 @@ package connectinject import ( + "testing" + "github.com/hashicorp/go-hclog" "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" - "testing" +) + +var ( + lifecycleResources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("25Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("20m"), + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + } ) // NOTE: This is tested here rather than in handler_test because doing it there @@ -16,8 +30,9 @@ import ( // Test that the lifecycle sidecar is as expected. func TestLifecycleSidecar_Default(t *testing.T) { handler := Handler{ - Log: hclog.Default().Named("handler"), - ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", + Log: hclog.Default().Named("handler"), + ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", + LifecycleSidecarResources: lifecycleResources, } container := handler.lifecycleSidecar(&corev1.Pod{ Spec: corev1.PodSpec{ @@ -54,16 +69,7 @@ func TestLifecycleSidecar_Default(t *testing.T) { "-service-config", "/consul/connect-inject/service.hcl", "-consul-binary", "/consul/connect-inject/consul", }, - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(lifecycleContainerCPULimit), - corev1.ResourceMemory: resource.MustParse(lifecycleContainerMemoryLimit), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(lifecycleContainerCPURequest), - corev1.ResourceMemory: resource.MustParse(lifecycleContainerMemoryRequest), - }, - }, + Resources: lifecycleResources, }, container) } @@ -128,9 +134,10 @@ func TestLifecycleSidecar_SyncPeriodAnnotation(t *testing.T) { // and that the CA is provided func TestLifecycleSidecar_TLS(t *testing.T) { handler := Handler{ - Log: hclog.Default().Named("handler"), - ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", - ConsulCACert: "consul-ca-cert", + Log: hclog.Default().Named("handler"), + ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", + ConsulCACert: "consul-ca-cert", + LifecycleSidecarResources: lifecycleResources, } container := handler.lifecycleSidecar(&corev1.Pod{ Spec: corev1.PodSpec{ @@ -171,15 +178,6 @@ func TestLifecycleSidecar_TLS(t *testing.T) { "-service-config", "/consul/connect-inject/service.hcl", "-consul-binary", "/consul/connect-inject/consul", }, - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(lifecycleContainerCPULimit), - corev1.ResourceMemory: resource.MustParse(lifecycleContainerMemoryLimit), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(lifecycleContainerCPURequest), - corev1.ResourceMemory: resource.MustParse(lifecycleContainerMemoryRequest), - }, - }, + Resources: lifecycleResources, }, container) } diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index ed1b40f525..114fc38109 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/go-hclog" "github.com/mitchellh/cli" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" @@ -58,6 +59,18 @@ type Command struct { flagDefaultSidecarProxyMemoryLimit string flagDefaultSidecarProxyMemoryRequest string + // Lifecycle sidecar resource settings. + flagLifecycleSidecarCPULimit string + flagLifecycleSidecarCPURequest string + flagLifecycleSidecarMemoryLimit string + flagLifecycleSidecarMemoryRequest string + + // Init container resource settings. + flagInitContainerCPULimit string + flagInitContainerCPURequest string + flagInitContainerMemoryLimit string + flagInitContainerMemoryRequest string + flagSet *flag.FlagSet http *flags.HTTPFlags @@ -112,11 +125,23 @@ func (c *Command) init() { "[Enterprise Only] Name of the ACL policy to attach to all created Consul namespaces to allow service "+ "discovery across Consul namespaces. Only necessary if ACLs are enabled.") - // Resource setting flags. - c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPULimit, "default-sidecar-proxy-cpu-limit", "", "Default sidecar proxy CPU limit.") + // Proxy sidecar resource setting flags. c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.") - c.flagSet.StringVar(&c.flagDefaultSidecarProxyMemoryLimit, "default-sidecar-proxy-memory-limit", "", "Default sidecar proxy memory limit.") + c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPULimit, "default-sidecar-proxy-cpu-limit", "", "Default sidecar proxy CPU limit.") c.flagSet.StringVar(&c.flagDefaultSidecarProxyMemoryRequest, "default-sidecar-proxy-memory-request", "", "Default sidecar proxy memory request.") + c.flagSet.StringVar(&c.flagDefaultSidecarProxyMemoryLimit, "default-sidecar-proxy-memory-limit", "", "Default sidecar proxy memory limit.") + + // Init container resource setting flags. + c.flagSet.StringVar(&c.flagInitContainerCPURequest, "init-container-cpu-request", "50m", "Init container CPU request.") + c.flagSet.StringVar(&c.flagInitContainerCPULimit, "init-container-cpu-limit", "50m", "Init container CPU limit.") + c.flagSet.StringVar(&c.flagInitContainerMemoryRequest, "init-container-memory-request", "25Mi", "Init container memory request.") + c.flagSet.StringVar(&c.flagInitContainerMemoryLimit, "init-container-memory-limit", "150Mi", "Init container memory limit.") + + // Lifecycle sidecar resource setting flags. + c.flagSet.StringVar(&c.flagLifecycleSidecarCPURequest, "lifecycle-sidecar-cpu-request", "20m", "Lifecycle sidecar CPU request.") + c.flagSet.StringVar(&c.flagLifecycleSidecarCPULimit, "lifecycle-sidecar-cpu-limit", "20m", "Lifecycle sidecar CPU limit.") + c.flagSet.StringVar(&c.flagLifecycleSidecarMemoryRequest, "lifecycle-sidecar-memory-request", "25Mi", "Lifecycle sidecar memory request.") + c.flagSet.StringVar(&c.flagLifecycleSidecarMemoryLimit, "lifecycle-sidecar-memory-limit", "25Mi", "Lifecycle sidecar memory limit.") c.http = &flags.HTTPFlags{} flags.Merge(c.flagSet, c.http.Flags()) @@ -135,50 +160,58 @@ func (c *Command) Run(args []string) int { return 1 } - var cpuLimit, cpuRequest, memoryLimit, memoryRequest resource.Quantity + // Proxy resources + var sidecarProxyCPULimit, sidecarProxyCPURequest, sidecarProxyMemoryLimit, sidecarProxyMemoryRequest resource.Quantity var err error - if c.flagDefaultSidecarProxyCPULimit != "" { - cpuLimit, err = resource.ParseQuantity(c.flagDefaultSidecarProxyCPULimit) + if c.flagDefaultSidecarProxyCPURequest != "" { + sidecarProxyCPURequest, err = resource.ParseQuantity(c.flagDefaultSidecarProxyCPURequest) if err != nil { - c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-cpu-limit is invalid: %s", err)) + c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-cpu-request is invalid: %s", err)) return 1 } } - if c.flagDefaultSidecarProxyCPURequest != "" { - cpuRequest, err = resource.ParseQuantity(c.flagDefaultSidecarProxyCPURequest) + if c.flagDefaultSidecarProxyCPULimit != "" { + sidecarProxyCPULimit, err = resource.ParseQuantity(c.flagDefaultSidecarProxyCPULimit) if err != nil { - c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-cpu-request is invalid: %s", err)) + c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-cpu-limit is invalid: %s", err)) return 1 } } - if cpuLimit.Value() != 0 && cpuRequest.Cmp(cpuLimit) > 0 { + if sidecarProxyCPULimit.Value() != 0 && sidecarProxyCPURequest.Cmp(sidecarProxyCPULimit) > 0 { c.UI.Error(fmt.Sprintf( "request must be <= limit: -default-sidecar-proxy-cpu-request value of %q is greater than the -default-sidecar-proxy-cpu-limit value of %q", c.flagDefaultSidecarProxyCPURequest, c.flagDefaultSidecarProxyCPULimit)) return 1 } - if c.flagDefaultSidecarProxyMemoryLimit != "" { - memoryLimit, err = resource.ParseQuantity(c.flagDefaultSidecarProxyMemoryLimit) + if c.flagDefaultSidecarProxyMemoryRequest != "" { + sidecarProxyMemoryRequest, err = resource.ParseQuantity(c.flagDefaultSidecarProxyMemoryRequest) if err != nil { - c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-memory-limit is invalid: %s", err)) + c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-memory-request is invalid: %s", err)) return 1 } } - if c.flagDefaultSidecarProxyMemoryRequest != "" { - memoryRequest, err = resource.ParseQuantity(c.flagDefaultSidecarProxyMemoryRequest) + if c.flagDefaultSidecarProxyMemoryLimit != "" { + sidecarProxyMemoryLimit, err = resource.ParseQuantity(c.flagDefaultSidecarProxyMemoryLimit) if err != nil { - c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-memory-request is invalid: %s", err)) + c.UI.Error(fmt.Sprintf("-default-sidecar-proxy-memory-limit is invalid: %s", err)) return 1 } } - if memoryLimit.Value() != 0 && memoryRequest.Cmp(memoryLimit) > 0 { + if sidecarProxyMemoryLimit.Value() != 0 && sidecarProxyMemoryRequest.Cmp(sidecarProxyMemoryLimit) > 0 { c.UI.Error(fmt.Sprintf( "request must be <= limit: -default-sidecar-proxy-memory-request value of %q is greater than the -default-sidecar-proxy-memory-limit value of %q", c.flagDefaultSidecarProxyMemoryRequest, c.flagDefaultSidecarProxyMemoryLimit)) return 1 } + // Validate resource request/limit flags and parse into corev1.ResourceRequirements + initResources, lifecycleResources, err := c.parseAndValidateResourceFlags() + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + // We must have an in-cluster K8S client if c.clientset == nil { config, err := rest.InClusterConfig() @@ -264,10 +297,12 @@ func (c *Command) Run(args []string) int { WriteServiceDefaults: c.flagWriteServiceDefaults, DefaultProtocol: c.flagDefaultProtocol, ConsulCACert: string(consulCACert), - DefaultProxyCPULimit: cpuLimit, - DefaultProxyCPURequest: cpuRequest, - DefaultProxyMemoryLimit: memoryLimit, - DefaultProxyMemoryRequest: memoryRequest, + DefaultProxyCPURequest: sidecarProxyCPURequest, + DefaultProxyCPULimit: sidecarProxyCPULimit, + DefaultProxyMemoryRequest: sidecarProxyMemoryRequest, + DefaultProxyMemoryLimit: sidecarProxyMemoryLimit, + InitContainerResources: initResources, + LifecycleSidecarResources: lifecycleResources, EnableNamespaces: c.flagEnableNamespaces, AllowK8sNamespacesSet: allowSet, DenyK8sNamespacesSet: denySet, @@ -363,6 +398,106 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client } } +func (c *Command) parseAndValidateResourceFlags() (corev1.ResourceRequirements, corev1.ResourceRequirements, error) { + // Init container + var initContainerCPULimit, initContainerCPURequest, initContainerMemoryLimit, initContainerMemoryRequest resource.Quantity + + // Parse and validate the initContainer resources. + initContainerCPURequest, err := resource.ParseQuantity(c.flagInitContainerCPURequest) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-init-container-cpu-request '%s' is invalid: %s", c.flagInitContainerCPURequest, err) + } + initContainerCPULimit, err = resource.ParseQuantity(c.flagInitContainerCPULimit) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-init-container-cpu-limit '%s' is invalid: %s", c.flagInitContainerCPULimit, err) + } + if initContainerCPULimit.Value() != 0 && initContainerCPURequest.Cmp(initContainerCPULimit) > 0 { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, fmt.Errorf( + "request must be <= limit: -init-container-cpu-request value of %q is greater than the -init-container-cpu-limit value of %q", + c.flagInitContainerCPURequest, c.flagInitContainerCPULimit) + } + + initContainerMemoryRequest, err = resource.ParseQuantity(c.flagInitContainerMemoryRequest) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-init-container-memory-request '%s' is invalid: %s", c.flagInitContainerMemoryRequest, err) + } + initContainerMemoryLimit, err = resource.ParseQuantity(c.flagInitContainerMemoryLimit) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-init-container-memory-limit '%s' is invalid: %s", c.flagInitContainerMemoryLimit, err) + } + if initContainerMemoryLimit.Value() != 0 && initContainerMemoryRequest.Cmp(initContainerMemoryLimit) > 0 { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, fmt.Errorf( + "request must be <= limit: -init-container-memory-request value of %q is greater than the -init-container-memory-limit value of %q", + c.flagInitContainerMemoryRequest, c.flagInitContainerMemoryLimit) + } + + // Put into corev1.ResourceRequirements form + initResources := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: initContainerCPURequest, + corev1.ResourceMemory: initContainerMemoryRequest, + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: initContainerCPULimit, + corev1.ResourceMemory: initContainerMemoryLimit, + }, + } + + // Lifecycle sidecar + var lifecycleSidecarCPULimit, lifecycleSidecarCPURequest, lifecycleSidecarMemoryLimit, lifecycleSidecarMemoryRequest resource.Quantity + + // Parse and validate the lifecycle sidecar resources + lifecycleSidecarCPURequest, err = resource.ParseQuantity(c.flagLifecycleSidecarCPURequest) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-lifecycle-sidecar-cpu-request '%s' is invalid: %s", c.flagLifecycleSidecarCPURequest, err) + } + lifecycleSidecarCPULimit, err = resource.ParseQuantity(c.flagLifecycleSidecarCPULimit) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-lifecycle-sidecar-cpu-limit '%s' is invalid: %s", c.flagLifecycleSidecarCPULimit, err) + } + if lifecycleSidecarCPULimit.Value() != 0 && lifecycleSidecarCPURequest.Cmp(lifecycleSidecarCPULimit) > 0 { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, fmt.Errorf( + "request must be <= limit: -lifecycle-sidecar-cpu-request value of %q is greater than the -lifecycle-sidecar-cpu-limit value of %q", + c.flagLifecycleSidecarCPURequest, c.flagLifecycleSidecarCPULimit) + } + + lifecycleSidecarMemoryRequest, err = resource.ParseQuantity(c.flagLifecycleSidecarMemoryRequest) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-lifecycle-sidecar-memory-request '%s' is invalid: %s", c.flagLifecycleSidecarMemoryRequest, err) + } + lifecycleSidecarMemoryLimit, err = resource.ParseQuantity(c.flagLifecycleSidecarMemoryLimit) + if err != nil { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, + fmt.Errorf("-lifecycle-sidecar-memory-limit '%s' is invalid: %s", c.flagLifecycleSidecarMemoryLimit, err) + } + if lifecycleSidecarMemoryLimit.Value() != 0 && lifecycleSidecarMemoryRequest.Cmp(lifecycleSidecarMemoryLimit) > 0 { + return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, fmt.Errorf( + "request must be <= limit: -lifecycle-sidecar-memory-request value of %q is greater than the -lifecycle-sidecar-memory-limit value of %q", + c.flagLifecycleSidecarMemoryRequest, c.flagLifecycleSidecarMemoryLimit) + } + + // Put into corev1.ResourceRequirements form + lifecycleResources := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: lifecycleSidecarCPURequest, + corev1.ResourceMemory: lifecycleSidecarMemoryRequest, + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: lifecycleSidecarCPULimit, + corev1.ResourceMemory: lifecycleSidecarMemoryLimit, + }, + } + + return initResources, lifecycleResources, nil +} + func (c *Command) Synopsis() string { return synopsis } func (c *Command) Help() string { c.once.Do(c.init) diff --git a/subcommand/inject-connect/command_test.go b/subcommand/inject-connect/command_test.go index 9c763f37d0..bad2dca338 100644 --- a/subcommand/inject-connect/command_test.go +++ b/subcommand/inject-connect/command_test.go @@ -52,6 +52,66 @@ func TestRun_FlagValidation(t *testing.T) { }, expErr: "request must be <= limit: -default-sidecar-proxy-cpu-request value of \"50m\" is greater than the -default-sidecar-proxy-cpu-limit value of \"25m\"", }, + { + flags: []string{"-consul-k8s-image", "foo", "-init-container-cpu-limit=unparseable"}, + expErr: "-init-container-cpu-limit 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-init-container-cpu-request=unparseable"}, + expErr: "-init-container-cpu-request 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-init-container-memory-limit=unparseable"}, + expErr: "-init-container-memory-limit 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-init-container-memory-request=unparseable"}, + expErr: "-init-container-memory-request 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", + "-init-container-memory-request=50Mi", + "-init-container-memory-limit=25Mi", + }, + expErr: "request must be <= limit: -init-container-memory-request value of \"50Mi\" is greater than the -init-container-memory-limit value of \"25Mi\"", + }, + { + flags: []string{"-consul-k8s-image", "foo", + "-init-container-cpu-request=50m", + "-init-container-cpu-limit=25m", + }, + expErr: "request must be <= limit: -init-container-cpu-request value of \"50m\" is greater than the -init-container-cpu-limit value of \"25m\"", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-cpu-limit=unparseable"}, + expErr: "-lifecycle-sidecar-cpu-limit 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-cpu-request=unparseable"}, + expErr: "-lifecycle-sidecar-cpu-request 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-memory-limit=unparseable"}, + expErr: "-lifecycle-sidecar-memory-limit 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-memory-request=unparseable"}, + expErr: "-lifecycle-sidecar-memory-request 'unparseable' is invalid", + }, + { + flags: []string{"-consul-k8s-image", "foo", + "-lifecycle-sidecar-memory-request=50Mi", + "-lifecycle-sidecar-memory-limit=25Mi", + }, + expErr: "request must be <= limit: -lifecycle-sidecar-memory-request value of \"50Mi\" is greater than the -lifecycle-sidecar-memory-limit value of \"25Mi\"", + }, + { + flags: []string{"-consul-k8s-image", "foo", + "-lifecycle-sidecar-cpu-request=50m", + "-lifecycle-sidecar-cpu-limit=25m", + }, + expErr: "request must be <= limit: -lifecycle-sidecar-cpu-request value of \"50m\" is greater than the -lifecycle-sidecar-cpu-limit value of \"25m\"", + }, } for _, c := range cases { @@ -68,3 +128,20 @@ func TestRun_FlagValidation(t *testing.T) { }) } } + +func TestRun_ResourceLimitDefaults(t *testing.T) { + cmd := Command{} + cmd.init() + + // Init container defaults + require.Equal(t, cmd.flagInitContainerCPURequest, "50m") + require.Equal(t, cmd.flagInitContainerCPULimit, "50m") + require.Equal(t, cmd.flagInitContainerMemoryRequest, "25Mi") + require.Equal(t, cmd.flagInitContainerMemoryLimit, "150Mi") + + // Lifecycle sidecar container defaults + require.Equal(t, cmd.flagLifecycleSidecarCPURequest, "20m") + require.Equal(t, cmd.flagLifecycleSidecarCPULimit, "20m") + require.Equal(t, cmd.flagLifecycleSidecarMemoryRequest, "25Mi") + require.Equal(t, cmd.flagLifecycleSidecarMemoryLimit, "25Mi") +}