Skip to content

Commit 27a9a16

Browse files
authored
Merge pull request #8052 from plkokanov/fix/nil-pointer
VPA: Fix nil ptr when loading metrics and `memory-saver=true`
2 parents 46fb73a + 260c306 commit 27a9a16

File tree

2 files changed

+33
-6
lines changed

2 files changed

+33
-6
lines changed

vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -499,15 +499,17 @@ func (feeder *clusterStateFeeder) LoadRealTimeMetrics(ctx context.Context) {
499499
sampleCount := 0
500500
droppedSampleCount := 0
501501
for _, containerMetrics := range containersMetrics {
502-
podInitContainers := feeder.clusterState.Pods()[containerMetrics.ID.PodID].InitContainers
503-
if slices.Contains(podInitContainers, containerMetrics.ID.ContainerName) {
504-
klog.V(3).InfoS("Skipping metric samples for init container", "pod", klog.KRef(containerMetrics.ID.PodID.Namespace, containerMetrics.ID.PodID.PodName), "container", containerMetrics.ID.ContainerName)
505-
droppedSampleCount += len(containerMetrics.Usage)
506-
continue
502+
// Container metrics are fetched for all pods, however, not all pod states are tracked in memory saver mode.
503+
if pod, exists := feeder.clusterState.Pods()[containerMetrics.ID.PodID]; exists && pod != nil {
504+
if slices.Contains(pod.InitContainers, containerMetrics.ID.ContainerName) {
505+
klog.V(3).InfoS("Skipping metric samples for init container", "pod", klog.KRef(containerMetrics.ID.PodID.Namespace, containerMetrics.ID.PodID.PodName), "container", containerMetrics.ID.ContainerName)
506+
droppedSampleCount += len(containerMetrics.Usage)
507+
continue
508+
}
507509
}
508510
for _, sample := range newContainerUsageSamplesWithKey(containerMetrics) {
509511
if err := feeder.clusterState.AddSample(sample); err != nil {
510-
// Not all pod states are tracked in memory saver mode
512+
// Not all pod states are tracked in memory saver mode.
511513
if _, isKeyError := err.(model.KeyError); isKeyError && feeder.memorySaveMode {
512514
continue
513515
}

vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ type fakeClusterState struct {
9797
}
9898

9999
func (cs *fakeClusterState) AddSample(sample *model.ContainerUsageSampleWithKey) error {
100+
_, podExists := cs.stubbedPods[sample.Container.PodID]
101+
if !podExists {
102+
return model.NewKeyError(sample.Container.PodID)
103+
}
100104
samplesForContainer := cs.addedSamples[sample.Container]
101105
cs.addedSamples[sample.Container] = append(samplesForContainer, sample)
102106
return nil
@@ -651,6 +655,27 @@ func TestClusterStateFeeder_LoadRealTimeMetrics(t *testing.T) {
651655
samplesForContainer2 := clusterState.addedSamples[regularContainer2]
652656
assert.Contains(t, samplesForContainer2, regularContainer2UsageSamples[0])
653657
assert.Contains(t, samplesForContainer2, regularContainer2UsageSamples[1])
658+
659+
// Add extra container metrics for which there are no added pods to the state to simulate memory-saver=true
660+
extraPodID := model.PodID{Namespace: namespaceName, PodName: "ExtraPod"}
661+
extraContainer := model.ContainerID{PodID: extraPodID, ContainerName: "ExtraContainer"}
662+
extraContainerMetricsSnapshot, _ := newContainerMetricsSnapshot(extraContainer, 200, 2048)
663+
containerMetricsSnapshots = append(containerMetricsSnapshots, extraContainerMetricsSnapshot)
664+
665+
clusterState = NewFakeClusterState(nil, pods)
666+
667+
feeder = clusterStateFeeder{
668+
memorySaveMode: true,
669+
clusterState: clusterState,
670+
metricsClient: fakeMetricsClient{snapshots: containerMetricsSnapshots},
671+
}
672+
673+
feeder.LoadRealTimeMetrics(tctx)
674+
675+
assert.Equal(t, 2, len(clusterState.addedSamples))
676+
677+
_, samplesForExtraContainerExist := clusterState.addedSamples[extraContainer]
678+
assert.False(t, samplesForExtraContainerExist)
654679
}
655680

656681
type fakeHistoryProvider struct {

0 commit comments

Comments
 (0)