Merge pull request #2478 from tylerslaton/fix-all-cc

Author: tylerslaton

pkg/controller/quota/quota.go

@@ -132,28 +132,32 @@ func addContainers(containers map[string]v1.Container, quotaRequest *adminv1.Quo
 func addCompute(containers map[string]v1.Container, appInstance *v1.AppInstance, quotaRequest *adminv1.QuotaRequestInstance) {
 	// For each workload, add their memory/cpu requests to the quota request
 	for name, container := range containers {
-		var cpu, memory resource.Quantity
+		var (
+			cpu, memory  resource.Quantity
+			computeClass string
+		)
+
 		if specific, ok := appInstance.Status.ResolvedOfferings.Containers[name]; ok {
 			memory = *resource.NewQuantity(z.Dereference(specific.Memory), resource.BinarySI)
 			cpu = *resource.NewMilliQuantity(z.Dereference(specific.CPU), resource.DecimalSI)
+			computeClass = specific.Class
 		} else if all, ok := appInstance.Status.ResolvedOfferings.Containers[""]; ok {
 			cpu = *resource.NewMilliQuantity(z.Dereference(all.CPU), resource.DecimalSI)
 			memory = *resource.NewQuantity(z.Dereference(all.Memory), resource.BinarySI)
+			computeClass = all.Class
 		}
 
 		// Multiply the memory/cpu requests by the scale of the container
 		cpu.Mul(replicas(container.Scale))
 		memory.Mul(replicas(container.Scale))
 
 		// Add the compute resources to the quota request
-		computeClass := appInstance.Status.ResolvedOfferings.Containers[name].Class
 		quotaRequest.Spec.Resources.Add(adminv1.QuotaRequestResources{BaseResources: adminv1.BaseResources{ComputeClasses: adminv1.ComputeClassResources{
 			computeClass: {
 				Memory: memory,
 				CPU:    cpu,
 			},
-		},
-		}})
+		}}})
 
 		// Recurse over any sidecars. Since sidecars can't have sidecars, this is safe.
 		addCompute(container.Sidecars, appInstance, quotaRequest)

pkg/controller/quota/quota_test.go

@@ -11,6 +11,12 @@ func TestBasic(t *testing.T) {
 	tester.DefaultTest(t, scheme.Scheme, "testdata/basic", EnsureQuotaRequest)
 }
 
+// TestAllSet simulates a secenario where the resolvedOfferings field does not have
+// any usage set for containers, just all.
+func TestOnlyAllSet(t *testing.T) {
+	tester.DefaultTest(t, scheme.Scheme, "testdata/only-all-set", EnsureQuotaRequest)
+}
+
 func TestNotEnforced(t *testing.T) {
 	tester.DefaultTest(t, scheme.Scheme, "testdata/not-enforced", EnsureQuotaRequest)
 }

pkg/controller/quota/testdata/only-all-set/existing.yaml

@@ -0,0 +1,11 @@
+kind: ProjectInstance
+apiVersion: internal.acorn.io/v1
+metadata:
+  name: app-namespace
+  annotations:
+    acorn.io/enforced-quota: "true"
+spec: {}
+status:
+  defaultRegion: local
+  supportedRegions:
+    - local

pkg/controller/quota/testdata/only-all-set/expected.golden

@@ -0,0 +1,34 @@
+`apiVersion: internal.admin.acorn.io/v1
+kind: QuotaRequestInstance
+metadata:
+  annotations:
+    acorn.io/app-generation: "0"
+  creationTimestamp: null
+  name: app-name
+  namespace: app-namespace
+spec:
+  resources:
+    apps: 0
+    computeClasses:
+      default-compute-class:
+        cpu: 250m
+        memory: 1Gi
+    containers: 1
+    images: 0
+    jobs: 1
+    secrets: 1
+    volumeClasses:
+      default-volume-class:
+        volumeStorage: 10G
+    volumes: 1
+status:
+  allocatedResources:
+    apps: 0
+    computeClasses: null
+    containers: 0
+    images: 0
+    jobs: 0
+    secrets: 0
+    volumeClasses: null
+    volumes: 0
+`

pkg/controller/quota/testdata/only-all-set/input.yaml

@@ -0,0 +1,80 @@
+kind: AppInstance
+apiVersion: internal.acorn.io/v1
+metadata:
+  name: app-name
+  namespace: app-namespace
+  uid: 1234567890abcdef
+spec:
+  image: image-name
+  computeClass:
+    "": default-compute-class
+status:
+  observedGeneration: 1
+  namespace: app-created-namespace
+  appImage:
+    id: test
+  appSpec:
+    containers:
+      container-name:
+        sidecars:
+          sidecar-name:
+            image: "image-name"
+            ports:
+              - port: 90
+                targetPort: 91
+                protocol: tcp
+        ports:
+        - port: 80
+          targetPort: 81
+          protocol: http
+        image: "image-name"
+        build:
+          dockerfile: "Dockerfile"
+          context: "."
+    jobs:
+      job-name:
+        ports:
+        - port: 80
+          targetPort: 81
+          protocol: http
+        image: "image-name"
+        build:
+          dockerfile: "Dockerfile"
+          context: "."
+    secrets:
+      test:
+        params:
+          characters: bcdfghjklmnpqrstvwxz2456789
+          length: 54
+        type: token
+    volumes:
+      test:
+        accessModes:
+        - readWriteOnce
+  resolvedOfferings:
+    containers:
+      "":
+        class: default-compute-class
+        cpu: 125
+        memory: 536870912
+      # simulating a bug where the container and sidecar do not have
+      # a resolved offering
+    volumes:
+      test:
+        class: default-volume-class
+        size: 536870912
+  scheduling:
+    container-name:
+      requirements:
+        limits:
+          memory: 512Mi
+        requests:
+          cpu: 75m
+          memory: 128Mi # simulate requestScaler
+    sidecar-name:
+      requirements:
+        limits:
+          memory: 512Mi
+        requests:
+          cpu: 75m
+          memory: 128Mi # simulate requestScaler