Skip to content

Commit d1aacae

Browse files
authored
fix: propagate template labels to virtual buffer pods (#3174)
1 parent cc1cd08 commit d1aacae

11 files changed

Lines changed: 184 additions & 73 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module sigs.k8s.io/karpenter
22

3-
go 1.26.5
3+
go 1.26.6
44

55
require (
66
github.com/Pallinder/go-randomdata v1.2.0

pkg/apis/autoscaling/v1beta1/constants.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ const (
3939
FakePodAnnotationKey = "karpenter.sh/capacity-buffer-fake-pod"
4040
FakePodAnnotationValue = "true"
4141

42-
// BufferNameLabel records which CapacityBuffer a virtual pod belongs to.
43-
BufferNameLabel = "karpenter.sh/capacity-buffer-name"
42+
// BufferNameAnnotation records which CapacityBuffer a virtual pod belongs to.
43+
BufferNameAnnotation = "karpenter.sh/capacity-buffer-name"
4444

45-
// BufferNamespaceLabel records the namespace of the CapacityBuffer a virtual pod belongs to.
46-
BufferNamespaceLabel = "karpenter.sh/capacity-buffer-namespace"
45+
// BufferNamespaceAnnotation records the namespace of the CapacityBuffer a virtual pod belongs to.
46+
BufferNamespaceAnnotation = "karpenter.sh/capacity-buffer-namespace"
4747

4848
// VirtualPodPriority is the priority stamped onto virtual buffer pods so that
4949
// future preemption / disruption logic can identify them as low-value.

pkg/controllers/capacitybuffer/controller.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (c *Controller) podTemplateToBuffers(ctx context.Context, obj client.Object
163163
// without re-fetching. Returns (false, nil, nil) for customer-induced errors
164164
// (not found, unsupported kind), or (false, nil, err) for unexpected failures
165165
// that should be retried.
166-
func (c *Controller) resolveAndUpdateStatus(ctx context.Context, cb *autoscalingv1beta1.CapacityBuffer) (bool, *v1.PodSpec, error) {
166+
func (c *Controller) resolveAndUpdateStatus(ctx context.Context, cb *autoscalingv1beta1.CapacityBuffer) (bool, *v1.PodTemplateSpec, error) {
167167
if cb.Spec.PodTemplateRef == nil && cb.Spec.ScalableRef == nil {
168168
cb.SetCondition(autoscalingv1beta1.ReadyForProvisioningCondition, metav1.ConditionFalse, ReasonResolutionFailed, "Neither podTemplateRef nor scalableRef is set")
169169
return false, nil, nil
@@ -191,11 +191,10 @@ func (c *Controller) resolveAndUpdateStatus(ctx context.Context, cb *autoscaling
191191
}
192192

193193
// Compute replicas from all applicable constraints.
194-
podSpec := &result.PodSpec
195-
replicas := computeReplicas(cb, podSpec, candidates)
194+
replicas := computeReplicas(cb, &result.PodTemplateSpec.Spec, candidates)
196195
cb.SetCondition(autoscalingv1beta1.ReadyForProvisioningCondition, metav1.ConditionTrue, ReasonResolved, "Pod template resolved successfully")
197196
cb.Status.Replicas = &replicas
198-
return true, podSpec, nil
197+
return true, &result.PodTemplateSpec, nil
199198
}
200199

201200
// computeReplicas derives the desired buffer replica count from the configured

pkg/controllers/provisioning/buffers.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ func bufferKeyOf(pod *corev1.Pod) string {
7171
if !IsVirtualPod(pod) {
7272
return ""
7373
}
74-
ns := pod.Labels[autoscalingv1beta1.BufferNamespaceLabel]
75-
name := pod.Labels[autoscalingv1beta1.BufferNameLabel]
74+
ns := pod.Annotations[autoscalingv1beta1.BufferNamespaceAnnotation]
75+
name := pod.Annotations[autoscalingv1beta1.BufferNameAnnotation]
7676
if ns == "" || name == "" {
7777
return ""
7878
}

pkg/controllers/provisioning/buffers_test.go

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,29 +42,25 @@ var _ = Describe("bufferKeyOf", func() {
4242
Expect(bufferKeyOf(pod)).To(Equal(""))
4343
})
4444

45-
It("should return empty string when namespace label is missing", func() {
45+
It("should return empty string when namespace annotation is missing", func() {
4646
pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
4747
Name: "virt",
4848
Namespace: "default",
4949
Annotations: map[string]string{
5050
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
51-
},
52-
Labels: map[string]string{
53-
autoscalingv1beta1.BufferNameLabel: "my-buffer",
51+
autoscalingv1beta1.BufferNameAnnotation: "my-buffer",
5452
},
5553
}}
5654
Expect(bufferKeyOf(pod)).To(Equal(""))
5755
})
5856

59-
It("should return empty string when name label is missing", func() {
57+
It("should return empty string when name annotation is missing", func() {
6058
pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
6159
Name: "virt",
6260
Namespace: "default",
6361
Annotations: map[string]string{
64-
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
65-
},
66-
Labels: map[string]string{
67-
autoscalingv1beta1.BufferNamespaceLabel: "default",
62+
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
63+
autoscalingv1beta1.BufferNamespaceAnnotation: "default",
6864
},
6965
}}
7066
Expect(bufferKeyOf(pod)).To(Equal(""))
@@ -75,11 +71,9 @@ var _ = Describe("bufferKeyOf", func() {
7571
Name: "virt",
7672
Namespace: "default",
7773
Annotations: map[string]string{
78-
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
79-
},
80-
Labels: map[string]string{
81-
autoscalingv1beta1.BufferNameLabel: "my-buffer",
82-
autoscalingv1beta1.BufferNamespaceLabel: "default",
74+
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
75+
autoscalingv1beta1.BufferNameAnnotation: "my-buffer",
76+
autoscalingv1beta1.BufferNamespaceAnnotation: "default",
8377
},
8478
}}
8579
Expect(bufferKeyOf(pod)).To(Equal("default/my-buffer"))
@@ -116,7 +110,7 @@ var _ = Describe("classifyBufferPods", func() {
116110
cbA := test.ReadyBuffer("a", 3)
117111
cbB := test.ReadyBuffer("b", 2)
118112
buffers := map[string]*autoscalingv1beta1.CapacityBuffer{"default/a": cbA, "default/b": cbB}
119-
spec := corev1.PodSpec{}
113+
spec := corev1.PodTemplateSpec{}
120114

121115
aPods := virtualpods.BuildVirtualPods(cbA, spec)
122116
bPods := virtualpods.BuildVirtualPods(cbB, spec)
@@ -151,7 +145,7 @@ var _ = Describe("classifyBufferPods", func() {
151145
"ns-a/buffer": cbA,
152146
"ns-b/buffer": cbB,
153147
}
154-
spec := corev1.PodSpec{}
148+
spec := corev1.PodTemplateSpec{}
155149

156150
aPods := virtualpods.BuildVirtualPods(cbA, spec)
157151
bPods := virtualpods.BuildVirtualPods(cbB, spec)
@@ -217,7 +211,7 @@ var _ = Describe("bufferPodCountsFromResults", func() {
217211
It("should count virtual pods per providerID on existing nodes", func() {
218212
cbA := test.ReadyBuffer("a", 3)
219213
cbB := test.ReadyBuffer("b", 2)
220-
spec := corev1.PodSpec{}
214+
spec := corev1.PodTemplateSpec{}
221215

222216
aPods := virtualpods.BuildVirtualPods(cbA, spec)
223217
bPods := virtualpods.BuildVirtualPods(cbB, spec)
@@ -279,7 +273,7 @@ var _ = Describe("computeProvisioningCondition with scalableRef buffer", func()
279273
var _ = Describe("filterVirtualPodErrors", func() {
280274
It("should remove virtual pods from error map", func() {
281275
cb := test.ReadyBuffer("web", 2)
282-
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodSpec{})
276+
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodTemplateSpec{})
283277
realPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "real", Namespace: "default"}}
284278

285279
input := map[*corev1.Pod]error{
@@ -295,7 +289,7 @@ var _ = Describe("filterVirtualPodErrors", func() {
295289

296290
It("should return empty map when all pods are virtual", func() {
297291
cb := test.ReadyBuffer("web", 2)
298-
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodSpec{})
292+
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodTemplateSpec{})
299293

300294
input := map[*corev1.Pod]error{
301295
virtualPods[0]: fmt.Errorf("err"),
@@ -328,7 +322,7 @@ var _ = Describe("filterVirtualPodErrors", func() {
328322
var _ = Describe("filterVirtualPodMapping", func() {
329323
It("should remove virtual pods from pod slices", func() {
330324
cb := test.ReadyBuffer("web", 2)
331-
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodSpec{})
325+
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodTemplateSpec{})
332326
realPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "real", Namespace: "default"}}
333327

334328
input := map[string][]*corev1.Pod{
@@ -343,7 +337,7 @@ var _ = Describe("filterVirtualPodMapping", func() {
343337

344338
It("should drop keys entirely when only virtual pods remain", func() {
345339
cb := test.ReadyBuffer("web", 2)
346-
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodSpec{})
340+
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodTemplateSpec{})
347341
realPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "real", Namespace: "default"}}
348342

349343
input := map[string][]*corev1.Pod{
@@ -359,7 +353,7 @@ var _ = Describe("filterVirtualPodMapping", func() {
359353

360354
It("should return empty map when all pods are virtual", func() {
361355
cb := test.ReadyBuffer("web", 2)
362-
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodSpec{})
356+
virtualPods := virtualpods.BuildVirtualPods(cb, corev1.PodTemplateSpec{})
363357

364358
input := map[string][]*corev1.Pod{
365359
"nodepool-a": {virtualPods[0], virtualPods[1]},

pkg/state/virtualpods/utilities.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,25 +65,27 @@ func isBufferReadyForProvisioning(cb *autoscalingv1beta1.CapacityBuffer) bool {
6565
// resolveVirtualPodSpec fetches the pod spec for a buffer using the shared
6666
// workload resolution utilities. Reads from spec (not status) to avoid stale
6767
// references when users switch between podTemplateRef and scalableRef.
68-
func resolveVirtualPodSpec(ctx context.Context, kubeClient client.Client, cb *autoscalingv1beta1.CapacityBuffer) (corev1.PodSpec, error) {
68+
func resolveVirtualPodSpec(ctx context.Context, kubeClient client.Client, cb *autoscalingv1beta1.CapacityBuffer) (corev1.PodTemplateSpec, error) {
6969
result, err := apps.ResolveCapacityBuffer(ctx, kubeClient, cb)
7070
if err != nil {
71-
return corev1.PodSpec{}, err
71+
return corev1.PodTemplateSpec{}, err
7272
}
73-
return result.PodSpec, nil
73+
return result.PodTemplateSpec, nil
7474
}
7575

7676
// BuildVirtualPods materializes N identical placeholder pods for a buffer using
77-
// the given pod spec. Deterministic names and UIDs let downstream components
78-
// associate results back to the owning buffer without additional bookkeeping.
79-
func BuildVirtualPods(cb *autoscalingv1beta1.CapacityBuffer, spec corev1.PodSpec) []*corev1.Pod {
77+
// the given pod template spec. Template labels are propagated to virtual pods so that
78+
// topology spread constraints and affinity rules are evaluated correctly.
79+
// Deterministic names and UIDs let downstream components associate results back
80+
// to the owning buffer without additional bookkeeping.
81+
func BuildVirtualPods(cb *autoscalingv1beta1.CapacityBuffer, spec corev1.PodTemplateSpec) []*corev1.Pod {
8082
if cb.Status.Replicas == nil || *cb.Status.Replicas <= 0 {
8183
return nil
8284
}
8385
count := int(*cb.Status.Replicas)
8486
out := make([]*corev1.Pod, 0, count)
8587
// Strip anything that would make the scheduler call the API server.
86-
strippedSpec := sanitizeVirtualPodSpec(spec)
88+
strippedSpec := sanitizeVirtualPodSpec(spec.Spec)
8789
strippedSpec.Priority = lo.ToPtr(autoscalingv1beta1.VirtualPodPriority)
8890

8991
for i := 1; i <= count; i++ {
@@ -92,12 +94,12 @@ func BuildVirtualPods(cb *autoscalingv1beta1.CapacityBuffer, spec corev1.PodSpec
9294
Name: fmt.Sprintf("capacity-buffer-%s-%d", cb.Name, i),
9395
Namespace: cb.Namespace,
9496
UID: types.UID(fmt.Sprintf("%s-%d", cb.UID, i)),
97+
Labels: spec.Labels,
98+
9599
Annotations: map[string]string{
96-
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
97-
},
98-
Labels: map[string]string{
99-
autoscalingv1beta1.BufferNameLabel: cb.Name,
100-
autoscalingv1beta1.BufferNamespaceLabel: cb.Namespace,
100+
autoscalingv1beta1.FakePodAnnotationKey: autoscalingv1beta1.FakePodAnnotationValue,
101+
autoscalingv1beta1.BufferNameAnnotation: cb.Name,
102+
autoscalingv1beta1.BufferNamespaceAnnotation: cb.Namespace,
101103
},
102104
CreationTimestamp: metav1.NewTime(time.Now()),
103105
},

pkg/state/virtualpods/utilities_test.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ import (
3232
var _ = Describe("buildVirtualPods", func() {
3333
It("should create the correct number of pods with expected metadata", func() {
3434
cb := test.ReadyBuffer("web", 3)
35-
spec := corev1.PodSpec{
36-
Containers: []corev1.Container{{Name: "app", Image: "pause:v1"}},
35+
spec := corev1.PodTemplateSpec{
36+
ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "web", "env": "prod"}},
37+
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app", Image: "pause:v1"}}},
3738
}
3839

3940
pods := BuildVirtualPods(cb, spec)
@@ -45,8 +46,10 @@ var _ = Describe("buildVirtualPods", func() {
4546
Expect(p.Namespace).To(Equal("default"))
4647
Expect(string(p.UID)).To(Equal("uid-web-" + strconv.Itoa(idx)))
4748
Expect(p.Annotations[autoscalingv1beta1.FakePodAnnotationKey]).To(Equal("true"))
48-
Expect(p.Labels[autoscalingv1beta1.BufferNameLabel]).To(Equal("web"))
49-
Expect(p.Labels[autoscalingv1beta1.BufferNamespaceLabel]).To(Equal("default"))
49+
Expect(p.Annotations[autoscalingv1beta1.BufferNameAnnotation]).To(Equal("web"))
50+
Expect(p.Annotations[autoscalingv1beta1.BufferNamespaceAnnotation]).To(Equal("default"))
51+
Expect(p.Labels).To(HaveKeyWithValue("app", "web"))
52+
Expect(p.Labels).To(HaveKeyWithValue("env", "prod"))
5053
Expect(p.Spec.Priority).ToNot(BeNil())
5154
Expect(*p.Spec.Priority).To(Equal(autoscalingv1beta1.VirtualPodPriority))
5255
Expect(p.Spec.NodeName).To(BeEmpty())
@@ -63,18 +66,23 @@ var _ = Describe("buildVirtualPods", func() {
6366

6467
It("should produce deterministic UIDs across calls", func() {
6568
cb := test.ReadyBuffer("web", 2)
66-
spec := corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}
69+
spec := corev1.PodTemplateSpec{
70+
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}},
71+
}
6772
a := BuildVirtualPods(cb, spec)
6873
b := BuildVirtualPods(cb, spec)
6974
Expect(a).To(HaveLen(len(b)))
7075
for i := range a {
7176
Expect(a[i].UID).To(Equal(b[i].UID))
77+
Expect(a[i].Labels).To(BeNil())
7278
}
7379
})
7480

7581
It("should return nil for zero replicas", func() {
7682
cb := test.ReadyBuffer("web", 0)
77-
spec := corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}
83+
spec := corev1.PodTemplateSpec{
84+
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}},
85+
}
7886
Expect(BuildVirtualPods(cb, spec)).To(BeNil())
7987
})
8088
})
@@ -251,8 +259,9 @@ var _ = Describe("listBuffersReadyForProvisioning", func() {
251259
var _ = Describe("buildVirtualPods with scalableRef buffer", func() {
252260
It("should create pods with expected metadata for scalableRef buffers", func() {
253261
cb := test.ReadyScalableRefBuffer("scalable-app", 2)
254-
spec := corev1.PodSpec{
255-
Containers: []corev1.Container{{Name: "app", Image: "nginx:latest"}},
262+
spec := corev1.PodTemplateSpec{
263+
ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "scalable"}},
264+
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app", Image: "nginx:latest"}}},
256265
}
257266

258267
pods := BuildVirtualPods(cb, spec)
@@ -263,8 +272,9 @@ var _ = Describe("buildVirtualPods with scalableRef buffer", func() {
263272
Expect(p.Name).To(Equal("capacity-buffer-scalable-app-" + strconv.Itoa(idx)))
264273
Expect(p.Namespace).To(Equal("default"))
265274
Expect(p.Annotations[autoscalingv1beta1.FakePodAnnotationKey]).To(Equal("true"))
266-
Expect(p.Labels[autoscalingv1beta1.BufferNameLabel]).To(Equal("scalable-app"))
267-
Expect(p.Labels[autoscalingv1beta1.BufferNamespaceLabel]).To(Equal("default"))
275+
Expect(p.Annotations[autoscalingv1beta1.BufferNameAnnotation]).To(Equal("scalable-app"))
276+
Expect(p.Annotations[autoscalingv1beta1.BufferNamespaceAnnotation]).To(Equal("default"))
277+
Expect(p.Labels).To(HaveKeyWithValue("app", "scalable"))
268278
Expect(*p.Spec.Priority).To(Equal(autoscalingv1beta1.VirtualPodPriority))
269279
}
270280
})

pkg/state/virtualpods/virtualpods.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func NewVirtualPodCache(kubeClient client.Client) *Cache {
5252
// already-resolved pod spec. The caller resolves
5353
// the spec once to compute replicas and status, then passes it here so the cache
5454
// doesn't re-fetch the same PodTemplate/workload
55-
func (v *Cache) UpdateEntry(cb *autoscalingv1beta1.CapacityBuffer, spec corev1.PodSpec) {
55+
func (v *Cache) UpdateEntry(cb *autoscalingv1beta1.CapacityBuffer, spec corev1.PodTemplateSpec) {
5656
if !isBufferReadyForProvisioning(cb) {
5757
v.RemoveEntry(client.ObjectKeyFromObject(cb))
5858
return

pkg/state/virtualpods/virtualpods_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ var _ = Describe("VirtualPodCache", func() {
119119
pods := cache.GetAll(ctx)
120120
Expect(pods).To(HaveLen(3))
121121
for _, p := range pods {
122-
Expect(p.Labels[autoscalingv1beta1.BufferNameLabel]).To(Equal("web"))
122+
Expect(p.Annotations[autoscalingv1beta1.BufferNameAnnotation]).To(Equal("web"))
123123
}
124124
})
125125

@@ -146,7 +146,7 @@ var _ = Describe("VirtualPodCache", func() {
146146
// Flip the buffer to not-ready and update. UpdateEntry drops the entry
147147
// regardless of the spec it is handed.
148148
cb.Status.Conditions[0].Status = metav1.ConditionFalse
149-
cache.UpdateEntry(cb, corev1.PodSpec{})
149+
cache.UpdateEntry(cb, corev1.PodTemplateSpec{})
150150
Expect(cache.GetAll(ctx)).To(BeEmpty())
151151
})
152152

@@ -202,7 +202,7 @@ var _ = Describe("VirtualPodCache", func() {
202202
pods := cache.GetAll(ctx)
203203
Expect(pods).To(HaveLen(3))
204204
for _, p := range pods {
205-
Expect(p.Labels[autoscalingv1beta1.BufferNameLabel]).To(Equal("web"))
205+
Expect(p.Annotations[autoscalingv1beta1.BufferNameAnnotation]).To(Equal("web"))
206206
}
207207
})
208208

@@ -232,7 +232,7 @@ var _ = Describe("VirtualPodCache", func() {
232232
pods := cache.GetAll(ctx)
233233
Expect(pods).To(HaveLen(2))
234234
for _, p := range pods {
235-
Expect(p.Labels[autoscalingv1beta1.BufferNameLabel]).To(Equal("api"))
235+
Expect(p.Annotations[autoscalingv1beta1.BufferNameAnnotation]).To(Equal("api"))
236236
}
237237
})
238238

0 commit comments

Comments
 (0)