Update based on review comments

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 IMPROVEMENTS:
 
-* Connect: Add resource request and limit flags for the injected init container and lifecycle sidecar.
+* 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-inject/handler.go

@@ -177,7 +177,7 @@ type Handler struct {
 	DefaultProxyMemoryRequest resource.Quantity
 	DefaultProxyMemoryLimit   resource.Quantity
 
-	// Resource settings for Init Container. All of these fields
+	// Resource settings for init container. All of these fields
 	// will be populated by the defaults provided in the initial flags.
 	InitContainerResources corev1.ResourceRequirements
 

subcommand/inject-connect/command.go

@@ -65,7 +65,7 @@ type Command struct {
 	flagLifecycleSidecarMemoryLimit   string
 	flagLifecycleSidecarMemoryRequest string
 
-	// Init copy container resource settings.
+	// Init container resource settings.
 	flagInitContainerCPULimit      string
 	flagInitContainerCPURequest    string
 	flagInitContainerMemoryLimit   string
@@ -125,17 +125,17 @@ 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.
+	// Proxy sidecar resource setting flags.
 	c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.")
 	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 copy container CPU request.")
-	c.flagSet.StringVar(&c.flagInitContainerCPULimit, "init-container-cpu-limit", "50m", "Init copy container CPU limit.")
-	c.flagSet.StringVar(&c.flagInitContainerMemoryRequest, "init-container-memory-request", "25Mi", "Init copy container memory request.")
-	c.flagSet.StringVar(&c.flagInitContainerMemoryLimit, "init-container-memory-limit", "150Mi", "Init copy container memory limit.")
+	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.")
@@ -405,11 +405,13 @@ func (c *Command) parseAndValidateResourceFlags() (corev1.ResourceRequirements,
 	// 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 is invalid: %s", err)
+		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 is invalid: %s", err)
+		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(
@@ -419,11 +421,13 @@ func (c *Command) parseAndValidateResourceFlags() (corev1.ResourceRequirements,
 
 	initContainerMemoryRequest, err = resource.ParseQuantity(c.flagInitContainerMemoryRequest)
 	if err != nil {
-		return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, fmt.Errorf("-init-container-memory-request is invalid: %s", err)
+		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 is invalid: %s", err)
+		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(
@@ -449,11 +453,13 @@ func (c *Command) parseAndValidateResourceFlags() (corev1.ResourceRequirements,
 	// 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 is invalid: %s", err)
+		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 is invalid: %s", err)
+		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(
@@ -463,11 +469,13 @@ func (c *Command) parseAndValidateResourceFlags() (corev1.ResourceRequirements,
 
 	lifecycleSidecarMemoryRequest, err = resource.ParseQuantity(c.flagLifecycleSidecarMemoryRequest)
 	if err != nil {
-		return corev1.ResourceRequirements{}, corev1.ResourceRequirements{}, fmt.Errorf("-lifecycle-sidecar-memory-request is invalid: %s", err)
+		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 is invalid: %s", err)
+		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(

subcommand/inject-connect/command_test.go

@@ -54,19 +54,19 @@ func TestRun_FlagValidation(t *testing.T) {
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-init-container-cpu-limit=unparseable"},
-			expErr: "-init-container-cpu-limit is invalid",
+			expErr: "-init-container-cpu-limit 'unparseable' is invalid",
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-init-container-cpu-request=unparseable"},
-			expErr: "-init-container-cpu-request is invalid",
+			expErr: "-init-container-cpu-request 'unparseable' is invalid",
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-init-container-memory-limit=unparseable"},
-			expErr: "-init-container-memory-limit is invalid",
+			expErr: "-init-container-memory-limit 'unparseable' is invalid",
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-init-container-memory-request=unparseable"},
-			expErr: "-init-container-memory-request is invalid",
+			expErr: "-init-container-memory-request 'unparseable' is invalid",
 		},
 		{
 			flags: []string{"-consul-k8s-image", "foo",
@@ -84,19 +84,19 @@ func TestRun_FlagValidation(t *testing.T) {
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-cpu-limit=unparseable"},
-			expErr: "-lifecycle-sidecar-cpu-limit is invalid",
+			expErr: "-lifecycle-sidecar-cpu-limit 'unparseable' is invalid",
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-cpu-request=unparseable"},
-			expErr: "-lifecycle-sidecar-cpu-request is invalid",
+			expErr: "-lifecycle-sidecar-cpu-request 'unparseable' is invalid",
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-memory-limit=unparseable"},
-			expErr: "-lifecycle-sidecar-memory-limit is invalid",
+			expErr: "-lifecycle-sidecar-memory-limit 'unparseable' is invalid",
 		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-memory-request=unparseable"},
-			expErr: "-lifecycle-sidecar-memory-request is invalid",
+			expErr: "-lifecycle-sidecar-memory-request 'unparseable' is invalid",
 		},
 		{
 			flags: []string{"-consul-k8s-image", "foo",
@@ -128,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")
+}