allow individual overrides and check for null (#82)

pipeline/builder/builder.go

@@ -21,7 +21,7 @@ import (
 )
 
 const (
-	errParseResourceList = "unable to parse resource limits for container: %s"
+	errOverrideResource = "could not override %s for container: %s"
 )
 
 var (
@@ -333,23 +333,19 @@ func (b *Builder) buildDeployEmbeddedManifestStage(index int, s config.Stage) (*
 			}
 			for ii, specContainer := range d.Spec.Template.Spec.Containers {
 				for _, overrideContainer := range maniStage.ContainerOverrides {
-					if specContainer.Name != overrideContainer.Name {
+					if specContainer.Name != overrideContainer.Name || overrideContainer.Resources == nil {
 						continue
 					}
-					if overrideContainer.Resources.Requests.Memory != "" || overrideContainer.Resources.Requests.CPU != "" {
-						requestsList, err := parseResourceList(overrideContainer.Resources.Requests.Memory, overrideContainer.Resources.Requests.CPU)
-						if err != nil {
-							return nil, errors.Wrapf(err, fmt.Sprintf(errParseResourceList, overrideContainer.Name))
-						}
-						d.Spec.Template.Spec.Containers[ii].Resources.Requests = requestsList
+					requests, err := overrideResource(specContainer.Resources.Requests, overrideContainer.Resources.Requests)
+					if err != nil {
+						return nil, errors.Wrapf(err, errOverrideResource, "requests", overrideContainer.Name)
 					}
-					if overrideContainer.Resources.Limits.Memory != "" || overrideContainer.Resources.Limits.CPU != "" {
-						limitsList, err := parseResourceList(overrideContainer.Resources.Limits.Memory, overrideContainer.Resources.Limits.CPU)
-						if err != nil {
-							return nil, errors.Wrapf(err, fmt.Sprintf(errParseResourceList, overrideContainer.Name))
-						}
-						d.Spec.Template.Spec.Containers[ii].Resources.Limits = limitsList
+					d.Spec.Template.Spec.Containers[ii].Resources.Requests = requests
+					limits, err := overrideResource(specContainer.Resources.Limits, overrideContainer.Resources.Limits)
+					if err != nil {
+						return nil, errors.Wrapf(err, errOverrideResource, "limits", overrideContainer.Name)
 					}
+					d.Spec.Template.Spec.Containers[ii].Resources.Limits = limits
 				}
 			}
 			objs[i] = d.DeepCopyObject()
@@ -451,20 +447,29 @@ func (b *Builder) buildDeleteEmbeddedManifestStage(index int, s config.Stage) (*
 	return stage, nil
 }
 
-// parseResourceList converts memory and cpu string to a resourceList
-func parseResourceList(memory string, cpu string) (corev1.ResourceList, error) {
-	memoryQty, err := resource.ParseQuantity(memory)
-	if err != nil {
-		return nil, errors.Wrapf(err, "could not parse memory")
+func overrideResource(resourceList corev1.ResourceList, override *config.Resource) (corev1.ResourceList, error) {
+	if override == nil {
+		return resourceList, nil
 	}
-	cpuQty, err := resource.ParseQuantity(cpu)
-	if err != nil {
-		return nil, errors.Wrapf(err, "could not parse cpu")
+	result := make(corev1.ResourceList)
+	if resourceList != nil {
+		result = resourceList
+	}
+	if override.Memory != "" {
+		memoryQty, err := resource.ParseQuantity(override.Memory)
+		if err != nil {
+			return nil, errors.Wrapf(err, "could not parse memory")
+		}
+		result[corev1.ResourceMemory] = memoryQty
+	}
+	if override.CPU != "" {
+		cpuQty, err := resource.ParseQuantity(override.CPU)
+		if err != nil {
+			return nil, errors.Wrapf(err, "could not parse memory")
+		}
+		result[corev1.ResourceCPU] = cpuQty
 	}
-	return map[corev1.ResourceName]resource.Quantity{
-		corev1.ResourceMemory: memoryQty,
-		corev1.ResourceCPU:    cpuQty,
-	}, nil
+	return result, nil
 }
 
 func (b *Builder) defaultManifestStage(index int, s config.Stage) *types.ManifestStage {

pipeline/builder/builder_test.go

@@ -320,7 +320,7 @@ func TestBuilderPipelineStages(t *testing.T) {
 			assert.Equal(t, int64(360000), spinnaker.Stages[0].(*types.ManifestStage).StageTimeoutMS)
 			assert.Equal(t, true, spinnaker.Stages[0].(*types.ManifestStage).OverrideTimeout)
 		})
-		t.Run("Overrides container overrides", func(t *testing.T) {
+		t.Run("Overrides container overrides limits and requests", func(t *testing.T) {
 			pipeline := &config.Pipeline{
 				Stages: []config.Stage{
 					{
@@ -353,6 +353,180 @@ func TestBuilderPipelineStages(t *testing.T) {
 			assert.Equal(t, "100", container.Resources.Requests.Memory().String())
 			assert.Equal(t, "200", container.Resources.Requests.Cpu().String())
 		})
+		t.Run("Overrides container overrides only requests", func(t *testing.T) {
+			pipeline := &config.Pipeline{
+				Stages: []config.Stage{
+					{
+						Name: "Test DeployEmbeddedManifests Stage",
+						DeployEmbeddedManifests: &config.DeployEmbeddedManifests{
+							Files: []config.ManifestFile{
+								{
+									File: file,
+								},
+							},
+							ContainerOverrides: []*config.ContainerOverrides{
+								{
+									Name: "test-container",
+									Resources: &config.Resources{
+										Requests: &config.Resource{Memory: "100", CPU: "200"},
+									},
+								},
+							},
+						},
+					},
+				},
+			}
+			builder := builder.New(pipeline)
+			spinnaker, err := builder.Pipeline()
+			require.NoError(t, err, "error generating pipeline json")
+			container := spinnaker.Stages[0].(*types.ManifestStage).Manifests[0].(*v1.Deployment).Spec.Template.Spec.Containers[0]
+			assert.Equal(t, "100", container.Resources.Requests.Memory().String())
+			assert.Equal(t, "200", container.Resources.Requests.Cpu().String())
+		})
+	})
+	t.Run("Overrides container overrides only limits", func(t *testing.T) {
+		pipeline := &config.Pipeline{
+			Stages: []config.Stage{
+				{
+					Name: "Test DeployEmbeddedManifests Stage",
+					DeployEmbeddedManifests: &config.DeployEmbeddedManifests{
+						Files: []config.ManifestFile{
+							{
+								File: file,
+							},
+						},
+						ContainerOverrides: []*config.ContainerOverrides{
+							{
+								Name: "test-container",
+								Resources: &config.Resources{
+									Limits: &config.Resource{Memory: "300Mi", CPU: "400m"},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+		builder := builder.New(pipeline)
+		spinnaker, err := builder.Pipeline()
+		require.NoError(t, err, "error generating pipeline json")
+		container := spinnaker.Stages[0].(*types.ManifestStage).Manifests[0].(*v1.Deployment).Spec.Template.Spec.Containers[0]
+		assert.Equal(t, "300Mi", container.Resources.Limits.Memory().String())
+		assert.Equal(t, "400m", container.Resources.Limits.Cpu().String())
+	})
+	t.Run("Overrides container overrides only cpu with resources set in deployment", func(t *testing.T) {
+		pipeline := &config.Pipeline{
+			Stages: []config.Stage{
+				{
+					Name: "Test DeployEmbeddedManifests Stage",
+					DeployEmbeddedManifests: &config.DeployEmbeddedManifests{
+						Files: []config.ManifestFile{
+							{
+								File: file,
+							},
+						},
+						ContainerOverrides: []*config.ContainerOverrides{
+							{
+								Name: "test-container",
+								Resources: &config.Resources{
+									Requests: &config.Resource{CPU: "400m"},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+		builder := builder.New(pipeline)
+		spinnaker, err := builder.Pipeline()
+		require.NoError(t, err, "error generating pipeline json")
+		container := spinnaker.Stages[0].(*types.ManifestStage).Manifests[0].(*v1.Deployment).Spec.Template.Spec.Containers[0]
+		assert.Equal(t, "4", container.Resources.Requests.Memory().String())
+		assert.Equal(t, "400m", container.Resources.Requests.Cpu().String())
+	})
+	t.Run("Overrides container overrides only cpu with resources not set in deployment", func(t *testing.T) {
+		pipeline := &config.Pipeline{
+			Stages: []config.Stage{
+				{
+					Name: "Test DeployEmbeddedManifests Stage",
+					DeployEmbeddedManifests: &config.DeployEmbeddedManifests{
+						Files: []config.ManifestFile{
+							{
+								File: file,
+							},
+						},
+						ContainerOverrides: []*config.ContainerOverrides{
+							{
+								Name: "test-container-no-resources",
+								Resources: &config.Resources{
+									Requests: &config.Resource{CPU: "400m"},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+		builder := builder.New(pipeline)
+		spinnaker, err := builder.Pipeline()
+		require.NoError(t, err, "error generating pipeline json")
+		container := spinnaker.Stages[0].(*types.ManifestStage).Manifests[0].(*v1.Deployment).Spec.Template.Spec.Containers[1]
+		assert.Equal(t, "0", container.Resources.Requests.Memory().String())
+		assert.Equal(t, "400m", container.Resources.Requests.Cpu().String())
+	})
+	t.Run("fails to override requests", func(t *testing.T) {
+		pipeline := &config.Pipeline{
+			Stages: []config.Stage{
+				{
+					Name: "Test DeployEmbeddedManifests Stage",
+					DeployEmbeddedManifests: &config.DeployEmbeddedManifests{
+						Files: []config.ManifestFile{
+							{
+								File: file,
+							},
+						},
+						ContainerOverrides: []*config.ContainerOverrides{
+							{
+								Name: "test-container",
+								Resources: &config.Resources{
+									Requests: &config.Resource{Memory: "bad-memory", CPU: "bad-cpu"},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+		builder := builder.New(pipeline)
+		_, err := builder.Pipeline()
+		assert.Error(t, err, "Failed to buildDeployEmbeddedManifestStage with error: could not override requests for container: test-container: could not parse memory: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$'")
+	})
+	t.Run("fails to override limits", func(t *testing.T) {
+		pipeline := &config.Pipeline{
+			Stages: []config.Stage{
+				{
+					Name: "Test DeployEmbeddedManifests Stage",
+					DeployEmbeddedManifests: &config.DeployEmbeddedManifests{
+						Files: []config.ManifestFile{
+							{
+								File: file,
+							},
+						},
+						ContainerOverrides: []*config.ContainerOverrides{
+							{
+								Name: "test-container",
+								Resources: &config.Resources{
+									Limits: &config.Resource{Memory: "bad-memory", CPU: "bad-cpu"},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+		builder := builder.New(pipeline)
+		_, err := builder.Pipeline()
+		assert.Error(t, err, "Failed to buildDeployEmbeddedManifestStage with error: could not override limits for container: test-container: could not parse memory: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$'")
 	})
 
 	t.Run("Deploy stage is parsed correctly", func(t *testing.T) {

pipeline/builder/kubernetes_test.go

@@ -60,7 +60,7 @@ func TestContainersFromManifests(t *testing.T) {
 
 		require.NoError(t, err, "error on retrieving the deployment manifests")
 
-		assert.Len(t, group.Containers, 1)
+		assert.Len(t, group.Containers, 2)
 		assert.Len(t, group.InitContainers, 1)
 		assert.Len(t, group.Annotations, 2)
 		assert.Equal(t, "fake-namespace", group.Namespace)

pipeline/builder/testdata/deployment.full.yml

@@ -45,11 +45,14 @@ spec:
             readOnly: true
         resources:
           limits:
-            cpu: "1"
-            memory: "2"
+            cpu: 1
+            memory: 2
           requests:
             cpu: "3"
             memory: "4"
+      - name: test-container-no-resources
+        command:
+          - echo
       volumes:
         - name: configmap-volume
           configMap:

test-pipeline.yml

@@ -39,6 +39,11 @@ stages:
       - file: test-configurator2.yml
         env: superOps
       - file: test-configurator.yml
+    containerOverrides:
+      - name: nginx
+        resources:
+          requests:
+            memory: 512Mi
 - account: ops-k8s
   name: "Scale Up"
   scaleManifest: