diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go index 6cdaeba6fec3..27327aa70ac6 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -19,6 +19,7 @@ package routines import ( "context" "flag" + "sort" "time" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" @@ -144,13 +145,22 @@ func (r *recommender) UpdateVPAs() { func getCappedRecommendation(vpaID model.VpaID, resources logic.RecommendedPodResources, policy *vpa_types.PodResourcePolicy) *vpa_types.RecommendedPodResources { containerResources := make([]vpa_types.RecommendedContainerResources, 0, len(resources)) - for containerName, res := range resources { + // Sort the container names from the map. This is because maps are an + // unordered data structure, and iterating through the map will return + // a different order on every call. + containerNames := make([]string, 0, len(resources)) + for containerName := range resources { + containerNames = append(containerNames, containerName) + } + sort.Strings(containerNames) + // Create the list of recommendations for each container. + for _, name := range containerNames { containerResources = append(containerResources, vpa_types.RecommendedContainerResources{ - ContainerName: containerName, - Target: model.ResourcesAsResourceList(res.Target), - LowerBound: model.ResourcesAsResourceList(res.LowerBound), - UpperBound: model.ResourcesAsResourceList(res.UpperBound), - UncappedTarget: model.ResourcesAsResourceList(res.Target), + ContainerName: name, + Target: model.ResourcesAsResourceList(resources[name].Target), + LowerBound: model.ResourcesAsResourceList(resources[name].LowerBound), + UpperBound: model.ResourcesAsResourceList(resources[name].UpperBound), + UncappedTarget: model.ResourcesAsResourceList(resources[name].Target), }) } recommendation := &vpa_types.RecommendedPodResources{ diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender_test.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender_test.go new file mode 100644 index 000000000000..9b8dfd1839e0 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender_test.go @@ -0,0 +1,78 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package routines + +import ( + "testing" + "time" + + labels "k8s.io/apimachinery/pkg/labels" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/logic" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model" + + "github.com/stretchr/testify/assert" +) + +func TestSortedRecommendation(t *testing.T) { + cases := []struct { + name string + resources logic.RecommendedPodResources + expectedLast []string + }{ + { + name: "All recommendations sorted", + resources: logic.RecommendedPodResources{ + "a-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + "b-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + "c-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + "d-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + }, + expectedLast: []string{ + "a-container", + "b-container", + "c-container", + "d-container", + }, + }, + { + name: "All recommendations unsorted", + resources: logic.RecommendedPodResources{ + "b-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + "a-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + "d-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + "c-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}}, + }, + expectedLast: []string{ + "a-container", + "b-container", + "c-container", + "d-container", + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + namespace := "test-namespace" + vpa := model.NewVpa(model.VpaID{Namespace: namespace, VpaName: "my-vpa"}, labels.Nothing(), time.Unix(0, 0)) + vpa.UpdateRecommendation(getCappedRecommendation(vpa.ID, tc.resources, nil)) + // Check that the slice is in the correct order. + for i := range vpa.Recommendation.ContainerRecommendations { + assert.Equal(t, tc.expectedLast[i], vpa.Recommendation.ContainerRecommendations[i].ContainerName) + } + }) + } +}