diff --git a/pkg/apis/internal.acorn.io/v1/appinstance.go b/pkg/apis/internal.acorn.io/v1/appinstance.go index 9a49d8157..6481c877c 100644 --- a/pkg/apis/internal.acorn.io/v1/appinstance.go +++ b/pkg/apis/internal.acorn.io/v1/appinstance.go @@ -5,6 +5,8 @@ package v1 import ( "strings" + "golang.org/x/exp/maps" + "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -122,6 +124,19 @@ func (in *AppInstance) GetStopped() bool { return in.Spec.Stop != nil && *in.Spec.Stop && in.DeletionTimestamp.IsZero() } +// GetAllContainerNames returns a string slice containing the name of every container, job, and sidecar defined in Status.AppSpec. +func (in *AppInstance) GetAllContainerNames() []string { + allContainers := append(maps.Keys(in.Status.AppSpec.Containers), maps.Keys(in.Status.AppSpec.Jobs)...) + for _, container := range in.Status.AppSpec.Containers { + allContainers = append(allContainers, maps.Keys(container.Sidecars)...) + } + for _, job := range in.Status.AppSpec.Jobs { + allContainers = append(allContainers, maps.Keys(job.Sidecars)...) + } + slices.Sort[[]string](allContainers) + return allContainers +} + func (in *AppInstanceSpec) GetAutoUpgrade() bool { return in.AutoUpgrade != nil && *in.AutoUpgrade } diff --git a/pkg/controller/defaults/memory.go b/pkg/controller/defaults/memory.go index 240084436..e921155e7 100644 --- a/pkg/controller/defaults/memory.go +++ b/pkg/controller/defaults/memory.go @@ -7,6 +7,7 @@ import ( adminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1" "github.com/acorn-io/runtime/pkg/computeclasses" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/strings/slices" ) // defaultMemory calculates the default that should be used and considers the defaults from the Config, ComputeClass, and @@ -54,6 +55,17 @@ func addDefaultMemory(req router.Request, cfg *apiv1.Config, appInstance *v1.App return err } + // Remove any memory defaults for containers that are no longer defined in the app. + allContainers := appInstance.GetAllContainerNames() + for containerName := range appInstance.Status.Defaults.Memory { + if containerName == "" { + continue + } + if !slices.Contains(allContainers, containerName) { + delete(appInstance.Status.Defaults.Memory, containerName) + } + } + return nil } diff --git a/pkg/controller/defaults/memory_test.go b/pkg/controller/defaults/memory_test.go index 5df464f24..ed07d16b3 100644 --- a/pkg/controller/defaults/memory_test.go +++ b/pkg/controller/defaults/memory_test.go @@ -72,3 +72,7 @@ func TestTwoPCCDefaultsShouldError(t *testing.T) { assert.True(t, resp.NoPrune, "NoPrune should be true when error occurs") } + +func TestRemovedContainer(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/memory/removed-container", Calculate) +} diff --git a/pkg/controller/defaults/testdata/memory/removed-container/existing.yaml b/pkg/controller/defaults/testdata/memory/removed-container/existing.yaml new file mode 100644 index 000000000..68c21dc2e --- /dev/null +++ b/pkg/controller/defaults/testdata/memory/removed-container/existing.yaml @@ -0,0 +1,9 @@ +kind: ProjectInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-namespace +spec: {} +status: + defaultRegion: local + supportedRegions: + - local diff --git a/pkg/controller/defaults/testdata/memory/removed-container/expected.golden b/pkg/controller/defaults/testdata/memory/removed-container/expected.golden new file mode 100644 index 000000000..6ee276044 --- /dev/null +++ b/pkg/controller/defaults/testdata/memory/removed-container/expected.golden @@ -0,0 +1,62 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + oneimage: + build: + context: . + dockerfile: Dockerfile + image: image-name + metrics: {} + ports: + - port: 80 + protocol: http + targetPort: 81 + probes: null + sidecars: + left: + image: foo + metrics: {} + ports: + - port: 90 + protocol: tcp + targetPort: 91 + probes: null + appStatus: {} + columns: {} + conditions: + reason: Success + status: "True" + success: true + type: defaults + defaults: + memory: + "": 0 + left: 0 + oneimage: 0 + region: local + namespace: app-created-namespace + observedGeneration: 1 + resolvedOfferings: {} + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/defaults/testdata/memory/removed-container/input.yaml b/pkg/controller/defaults/testdata/memory/removed-container/input.yaml new file mode 100644 index 000000000..0b3c6840f --- /dev/null +++ b/pkg/controller/defaults/testdata/memory/removed-container/input.yaml @@ -0,0 +1,36 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 # 1Mi +status: + observedGeneration: 1 + namespace: app-created-namespace + appImage: + id: test + appSpec: + containers: + oneimage: + sidecars: + left: + image: "foo" + ports: + - port: 90 + targetPort: 91 + protocol: tcp + ports: + - port: 80 + targetPort: 81 + protocol: http + image: "image-name" + build: + dockerfile: "Dockerfile" + context: "." + defaults: + memory: + twoimage: 0 diff --git a/pkg/controller/resolvedofferings/computeclass.go b/pkg/controller/resolvedofferings/computeclass.go index fdefc6bb6..41be722c8 100644 --- a/pkg/controller/resolvedofferings/computeclass.go +++ b/pkg/controller/resolvedofferings/computeclass.go @@ -7,6 +7,7 @@ import ( adminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1" "github.com/acorn-io/runtime/pkg/computeclasses" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/strings/slices" ) // resolveComputeClasses resolves the compute class information for each container in the AppInstance @@ -69,6 +70,17 @@ func resolveComputeClasses(req router.Request, cfg *apiv1.Config, appInstance *v return err } + // Remove any resolved offerings for containers that are no longer defined in the app. + allContainers := appInstance.GetAllContainerNames() + for containerName := range appInstance.Status.ResolvedOfferings.Containers { + if containerName == "" { + continue + } + if !slices.Contains(allContainers, containerName) { + delete(appInstance.Status.ResolvedOfferings.Containers, containerName) + } + } + return nil } diff --git a/pkg/controller/resolvedofferings/computeclass_test.go b/pkg/controller/resolvedofferings/computeclass_test.go index ef30d298c..da94265a2 100644 --- a/pkg/controller/resolvedofferings/computeclass_test.go +++ b/pkg/controller/resolvedofferings/computeclass_test.go @@ -88,3 +88,7 @@ func TestUserOverrideComputeClass(t *testing.T) { func TestWithAcornfileMemoryAndSpecOverride(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/computeclass/with-acornfile-memory-and-spec-override", Calculate) } + +func TestRemovedContainer(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/computeclass/removed-container", Calculate) +} diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/existing.yaml b/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/existing.yaml new file mode 100644 index 000000000..68c21dc2e --- /dev/null +++ b/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/existing.yaml @@ -0,0 +1,9 @@ +kind: ProjectInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-namespace +spec: {} +status: + defaultRegion: local + supportedRegions: + - local diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/expected.golden new file mode 100644 index 000000000..63a633fd2 --- /dev/null +++ b/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/expected.golden @@ -0,0 +1,65 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + oneimage: + build: + context: . + dockerfile: Dockerfile + image: image-name + metrics: {} + ports: + - port: 80 + protocol: http + targetPort: 81 + probes: null + sidecars: + left: + image: foo + metrics: {} + ports: + - port: 90 + protocol: tcp + targetPort: 91 + probes: null + appStatus: {} + columns: {} + conditions: + reason: Success + status: "True" + success: true + type: resolved-offerings + defaults: {} + namespace: app-created-namespace + observedGeneration: 1 + resolvedOfferings: + containers: + "": + memory: 0 + left: + memory: 0 + oneimage: + memory: 1048576 + region: local + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/input.yaml b/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/input.yaml new file mode 100644 index 000000000..04f65e13d --- /dev/null +++ b/pkg/controller/resolvedofferings/testdata/computeclass/removed-container/input.yaml @@ -0,0 +1,37 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 # 1Mi +status: + observedGeneration: 1 + namespace: app-created-namespace + appImage: + id: test + appSpec: + containers: + oneimage: + sidecars: + left: + image: "foo" + ports: + - port: 90 + targetPort: 91 + protocol: tcp + ports: + - port: 80 + targetPort: 81 + protocol: http + image: "image-name" + build: + dockerfile: "Dockerfile" + context: "." + resolvedOfferings: + containers: + twoimage: + memory: 0 diff --git a/pkg/controller/scheduling/computeclass_test.go b/pkg/controller/scheduling/computeclass_test.go index c55c370ac..71c19a4e3 100644 --- a/pkg/controller/scheduling/computeclass_test.go +++ b/pkg/controller/scheduling/computeclass_test.go @@ -114,3 +114,7 @@ func TestInvalidPriorityClassShouldError(t *testing.T) { assert.True(t, resp.NoPrune, "NoPrune should be true when error occurs") } + +func TestRemovedContainerComputeClass(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/computeclass/removed-container", Calculate) +} diff --git a/pkg/controller/scheduling/memory_test.go b/pkg/controller/scheduling/memory_test.go index 47c01189e..2b99d6b0a 100644 --- a/pkg/controller/scheduling/memory_test.go +++ b/pkg/controller/scheduling/memory_test.go @@ -42,3 +42,7 @@ func TestAllSetOverwrite(t *testing.T) { func TestSameGenerationMemory(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/memory/same-generation", Calculate) } + +func TestRemovedContainerMemory(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/memory/removed-container", Calculate) +} diff --git a/pkg/controller/scheduling/scheduling.go b/pkg/controller/scheduling/scheduling.go index d17f60924..e5bbe4a4c 100644 --- a/pkg/controller/scheduling/scheduling.go +++ b/pkg/controller/scheduling/scheduling.go @@ -13,6 +13,7 @@ import ( nodev1 "k8s.io/api/node/v1" schedulingv1 "k8s.io/api/scheduling/v1" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/utils/strings/slices" ) // Calculate is a handler that sets the scheduling rules for an AppInstance to its @@ -49,6 +50,15 @@ func calculate(req router.Request, appInstance *v1.AppInstance) error { if err := addScheduling(req, appInstance, appInstance.Status.AppSpec.Jobs); err != nil { return err } + + // Remove any scheduling information for containers that are no longer defined in the app. + allContainers := appInstance.GetAllContainerNames() + for containerName := range appInstance.Status.Scheduling { + if !slices.Contains(allContainers, containerName) { + delete(appInstance.Status.Scheduling, containerName) + } + } + return nil } diff --git a/pkg/controller/scheduling/testdata/computeclass/removed-container/existing.yaml b/pkg/controller/scheduling/testdata/computeclass/removed-container/existing.yaml new file mode 100644 index 000000000..a99056da4 --- /dev/null +++ b/pkg/controller/scheduling/testdata/computeclass/removed-container/existing.yaml @@ -0,0 +1,20 @@ +--- +kind: ClusterComputeClassInstance +apiVersion: internal.admin.acorn.io/v1 +metadata: + name: sample-compute-class +description: Simple description for a simple ComputeClass +cpuScaler: 0.25 +memory: + min: 1Mi # 1Mi + max: 2Mi # 2Mi + default: 1Mi # 1Mi +affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: foo + operator: In + values: + - bar \ No newline at end of file diff --git a/pkg/controller/scheduling/testdata/computeclass/removed-container/expected.golden b/pkg/controller/scheduling/testdata/computeclass/removed-container/expected.golden new file mode 100644 index 000000000..27911557d --- /dev/null +++ b/pkg/controller/scheduling/testdata/computeclass/removed-container/expected.golden @@ -0,0 +1,88 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + computeClass: + oneimage: sample-compute-class + image: test +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + oneimage: + build: + context: . + dockerfile: Dockerfile + image: image-name + metrics: {} + ports: + - port: 80 + protocol: http + targetPort: 81 + probes: null + sidecars: + left: + image: foo + metrics: {} + ports: + - port: 90 + protocol: tcp + targetPort: 91 + probes: null + appStatus: {} + columns: {} + conditions: + reason: Success + status: "True" + success: true + type: scheduling + defaults: + memory: + "": 0 + left: 1048576 + oneimage: 1048576 + namespace: app-created-namespace + observedGeneration: 1 + resolvedOfferings: {} + scheduling: + left: + requirements: + limits: + memory: 1Mi + requests: + cpu: 1m + memory: 1Mi + oneimage: + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: foo + operator: In + values: + - bar + requirements: + limits: + memory: 1Mi + requests: + cpu: 1m + memory: 1Mi + tolerations: + - key: taints.acorn.io/workload + operator: Exists + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/scheduling/testdata/computeclass/removed-container/input.yaml b/pkg/controller/scheduling/testdata/computeclass/removed-container/input.yaml new file mode 100644 index 000000000..c47bd7789 --- /dev/null +++ b/pkg/controller/scheduling/testdata/computeclass/removed-container/input.yaml @@ -0,0 +1,42 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + computeClass: + oneimage: sample-compute-class +status: + observedGeneration: 1 + defaults: + memory: + "": 0 + left: 1048576 # 1Mi + oneimage: 1048576 # 1Mi + namespace: app-created-namespace + appImage: + id: test + defaults: + appSpec: + containers: + oneimage: + sidecars: + left: + image: "foo" + ports: + - port: 90 + targetPort: 91 + protocol: tcp + ports: + - port: 80 + targetPort: 81 + protocol: http + image: "image-name" + build: + dockerfile: "Dockerfile" + context: "." + scheduling: + twoimage: + requirements: {} diff --git a/pkg/controller/scheduling/testdata/memory/removed-container/expected.golden b/pkg/controller/scheduling/testdata/memory/removed-container/expected.golden new file mode 100644 index 000000000..9a3f42408 --- /dev/null +++ b/pkg/controller/scheduling/testdata/memory/removed-container/expected.golden @@ -0,0 +1,73 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + oneimage: + build: + context: . + dockerfile: Dockerfile + image: image-name + metrics: {} + ports: + - port: 80 + protocol: http + targetPort: 81 + probes: null + sidecars: + left: + image: foo + metrics: {} + ports: + - port: 90 + protocol: tcp + targetPort: 91 + probes: null + appStatus: {} + columns: {} + conditions: + reason: Success + status: "True" + success: true + type: scheduling + defaults: + memory: + "": 0 + left: 0 + oneimage: 0 + namespace: app-created-namespace + observedGeneration: 1 + resolvedOfferings: {} + scheduling: + left: + requirements: {} + oneimage: + requirements: + limits: + memory: 1Mi + requests: + memory: 1Mi + tolerations: + - key: taints.acorn.io/workload + operator: Exists + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/scheduling/testdata/memory/removed-container/input.yaml b/pkg/controller/scheduling/testdata/memory/removed-container/input.yaml new file mode 100644 index 000000000..eeaa01483 --- /dev/null +++ b/pkg/controller/scheduling/testdata/memory/removed-container/input.yaml @@ -0,0 +1,41 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 # 1Mi +status: + observedGeneration: 1 + defaults: + memory: + "": 0 + left: 0 + oneimage: 0 + namespace: app-created-namespace + appImage: + id: test + appSpec: + containers: + oneimage: + sidecars: + left: + image: "foo" + ports: + - port: 90 + targetPort: 91 + protocol: tcp + ports: + - port: 80 + targetPort: 81 + protocol: http + image: "image-name" + build: + dockerfile: "Dockerfile" + context: "." + scheduling: + twoimage: + requirements: {} diff --git a/pkg/controller/scheduling/testdata/tolerations/removed-container/expected.golden b/pkg/controller/scheduling/testdata/tolerations/removed-container/expected.golden new file mode 100644 index 000000000..78f6af63e --- /dev/null +++ b/pkg/controller/scheduling/testdata/tolerations/removed-container/expected.golden @@ -0,0 +1,63 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + oneimage: + build: + context: . + dockerfile: Dockerfile + image: image-name + metrics: {} + ports: + - port: 80 + protocol: http + targetPort: 81 + probes: null + sidecars: + left: + image: foo + metrics: {} + ports: + - port: 90 + protocol: tcp + targetPort: 91 + probes: null + appStatus: {} + columns: {} + conditions: + reason: Success + status: "True" + success: true + type: scheduling + defaults: {} + namespace: app-created-namespace + observedGeneration: 1 + resolvedOfferings: {} + scheduling: + left: + requirements: {} + oneimage: + requirements: {} + tolerations: + - key: taints.acorn.io/workload + operator: Exists + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/scheduling/testdata/tolerations/removed-container/input.yaml b/pkg/controller/scheduling/testdata/tolerations/removed-container/input.yaml new file mode 100644 index 000000000..3694ed179 --- /dev/null +++ b/pkg/controller/scheduling/testdata/tolerations/removed-container/input.yaml @@ -0,0 +1,35 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test +status: + observedGeneration: 1 + namespace: app-created-namespace + appImage: + id: test + defaults: + appSpec: + containers: + oneimage: + sidecars: + left: + image: "foo" + ports: + - port: 90 + targetPort: 91 + protocol: tcp + ports: + - port: 80 + targetPort: 81 + protocol: http + image: "image-name" + build: + dockerfile: "Dockerfile" + context: "." + scheduling: + twoimage: + tolerations: [] diff --git a/pkg/controller/scheduling/toleration_test.go b/pkg/controller/scheduling/toleration_test.go index 9c5af3442..5677ede03 100644 --- a/pkg/controller/scheduling/toleration_test.go +++ b/pkg/controller/scheduling/toleration_test.go @@ -14,3 +14,7 @@ func TestContainerTolerations(t *testing.T) { func TestJobTolerations(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/tolerations/job", Calculate) } + +func TestRemovedContainerTolerations(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/tolerations/removed-container", Calculate) +}