Skip to content

Commit ed2bdb8

Browse files
authored
Merge pull request #9128 from norbertcyran/fix-overriding-buffers-conditions
Fix overriding buffers conditions
2 parents 068565c + e182602 commit ed2bdb8

File tree

12 files changed

+370
-85
lines changed

12 files changed

+370
-85
lines changed

cluster-autoscaler/capacitybuffer/common/common.go

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,42 +23,33 @@ import (
2323

2424
"k8s.io/apimachinery/pkg/api/meta"
2525
v1 "k8s.io/autoscaler/cluster-autoscaler/apis/capacitybuffer/autoscaling.x-k8s.io/v1beta1"
26+
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer"
2627

2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2829
)
2930

30-
// Constants to use in Capacity Buffers objects
31-
const (
32-
ActiveProvisioningStrategy = "buffer.x-k8s.io/active-capacity"
33-
CapacityBufferKind = "CapacityBuffer"
34-
CapacityBufferApiVersion = "autoscaling.x-k8s.io/v1beta1"
35-
ReadyForProvisioningCondition = "ReadyForProvisioning"
36-
ProvisioningCondition = "Provisioning"
37-
LimitedByQuotasCondition = "LimitedByQuotas"
38-
LimitedByQuotasReason = "ResourceQuotasAllocated"
39-
ConditionTrue = "True"
40-
ConditionFalse = "False"
41-
)
42-
4331
// SetBufferAsReadyForProvisioning updates the passed buffer object with the rest of the attributes and sets its condition to ready
4432
func SetBufferAsReadyForProvisioning(buffer *v1.CapacityBuffer, PodTemplateRef *v1.LocalObjectRef, podTemplateGeneration *int64, replicas *int32, provStrategy *string) {
4533
buffer.Status.PodTemplateRef = PodTemplateRef
4634
buffer.Status.Replicas = replicas
4735
buffer.Status.PodTemplateGeneration = podTemplateGeneration
4836
buffer.Status.ProvisioningStrategy = mapEmptyProvStrategyToDefault(provStrategy)
4937
readyCondition := metav1.Condition{
50-
Type: ReadyForProvisioningCondition,
51-
Status: ConditionTrue,
38+
Type: capacitybuffer.ReadyForProvisioningCondition,
39+
Status: metav1.ConditionTrue,
5240
Message: "ready",
53-
Reason: "atrtibutesSetSuccessfully",
41+
Reason: capacitybuffer.AttributesSetSuccessfullyReason,
5442
LastTransitionTime: metav1.Time{Time: time.Now()},
5543
}
56-
buffer.Status.Conditions = []metav1.Condition{readyCondition}
44+
if buffer.Status.Conditions == nil {
45+
buffer.Status.Conditions = make([]metav1.Condition, 0)
46+
}
47+
meta.SetStatusCondition(&buffer.Status.Conditions, readyCondition)
5748
}
5849

5950
// SetBufferAsNotReadyForProvisioning updates the passed buffer object with the rest of the attributes and sets its condition to not ready with the passed error
6051
func SetBufferAsNotReadyForProvisioning(buffer *v1.CapacityBuffer, PodTemplateRef *v1.LocalObjectRef, podTemplateGeneration *int64, replicas *int32, provStrategy *string, err error) {
61-
errorMessage := "Buffer not ready for provisioing"
52+
errorMessage := "Buffer not ready for provisioning"
6253
if err != nil {
6354
errorMessage = err.Error()
6455
}
@@ -68,42 +59,53 @@ func SetBufferAsNotReadyForProvisioning(buffer *v1.CapacityBuffer, PodTemplateRe
6859
buffer.Status.PodTemplateGeneration = podTemplateGeneration
6960
buffer.Status.ProvisioningStrategy = mapEmptyProvStrategyToDefault(provStrategy)
7061
notReadyCondition := metav1.Condition{
71-
Type: ReadyForProvisioningCondition,
72-
Status: ConditionFalse,
62+
Type: capacitybuffer.ReadyForProvisioningCondition,
63+
Status: metav1.ConditionFalse,
7364
Message: errorMessage,
7465
Reason: "error",
7566
LastTransitionTime: metav1.Time{Time: time.Now()},
7667
}
77-
buffer.Status.Conditions = []metav1.Condition{notReadyCondition}
68+
if buffer.Status.Conditions == nil {
69+
buffer.Status.Conditions = make([]metav1.Condition, 0)
70+
}
71+
meta.SetStatusCondition(&buffer.Status.Conditions, notReadyCondition)
7872
}
7973

8074
func mapEmptyProvStrategyToDefault(ps *string) *string {
8175
if ps != nil && *ps == "" {
82-
defaultProvStrategy := ActiveProvisioningStrategy
76+
defaultProvStrategy := capacitybuffer.ActiveProvisioningStrategy
8377
ps = &defaultProvStrategy
8478
}
8579
return ps
8680
}
8781

88-
// UpdateBufferStatusToFailedProvisioing updates the status of the passed buffer and set Provisioning to false with the passes reason and message
89-
func UpdateBufferStatusToFailedProvisioing(buffer *v1.CapacityBuffer, reason, errorMessage string) {
90-
buffer.Status.Conditions = []metav1.Condition{{
91-
Type: ProvisioningCondition,
92-
Status: ConditionFalse,
82+
// UpdateBufferStatusToFailedProvisioning updates the status of the passed buffer and set Provisioning to false with the passes reason and message
83+
func UpdateBufferStatusToFailedProvisioning(buffer *v1.CapacityBuffer, reason, errorMessage string) bool {
84+
newCondition := metav1.Condition{
85+
Type: capacitybuffer.ProvisioningCondition,
86+
Status: metav1.ConditionFalse,
9387
Message: errorMessage,
9488
Reason: reason,
9589
LastTransitionTime: metav1.Time{Time: time.Now()},
96-
}}
90+
}
91+
if buffer.Status.Conditions == nil {
92+
buffer.Status.Conditions = make([]metav1.Condition, 0)
93+
}
94+
return meta.SetStatusCondition(&buffer.Status.Conditions, newCondition)
9795
}
9896

99-
// UpdateBufferStatusToSuccessfullyProvisioing updates the status of the passed buffer and set Provisioning to true with the passes reason
100-
func UpdateBufferStatusToSuccessfullyProvisioing(buffer *v1.CapacityBuffer, reason string) {
101-
buffer.Status.Conditions = []metav1.Condition{{
102-
Type: ProvisioningCondition,
103-
Status: ConditionTrue,
97+
// UpdateBufferStatusToSuccessfullyProvisioning updates the status of the passed buffer and set Provisioning to true with the passes reason
98+
func UpdateBufferStatusToSuccessfullyProvisioning(buffer *v1.CapacityBuffer, reason string) {
99+
newCondition := metav1.Condition{
100+
Type: capacitybuffer.ProvisioningCondition,
101+
Status: metav1.ConditionTrue,
104102
Reason: reason,
105103
LastTransitionTime: metav1.Time{Time: time.Now()},
106-
}}
104+
}
105+
if buffer.Status.Conditions == nil {
106+
buffer.Status.Conditions = make([]metav1.Condition, 0)
107+
}
108+
meta.SetStatusCondition(&buffer.Status.Conditions, newCondition)
107109
}
108110

109111
// MarkBufferAsLimitedByQuota adds or updates the LimitedByQuotas condition with True
@@ -118,16 +120,16 @@ func MarkBufferAsLimitedByQuota(buffer *v1.CapacityBuffer, desiredReplicas, allo
118120

119121
// UpdateBufferStatusLimitedByQuotas adds or updates the LimitedByQuotas condition
120122
func UpdateBufferStatusLimitedByQuotas(buffer *v1.CapacityBuffer, isLimited bool, message string) {
121-
status := ConditionFalse
123+
status := metav1.ConditionFalse
122124
if isLimited {
123-
status = ConditionTrue
125+
status = metav1.ConditionTrue
124126
}
125127

126128
newCondition := metav1.Condition{
127-
Type: LimitedByQuotasCondition,
128-
Status: metav1.ConditionStatus(status),
129+
Type: capacitybuffer.LimitedByQuotasCondition,
130+
Status: status,
129131
Message: message,
130-
Reason: LimitedByQuotasReason,
132+
Reason: capacitybuffer.LimitedByQuotasReason,
131133
LastTransitionTime: metav1.Time{Time: time.Now()},
132134
ObservedGeneration: buffer.Generation,
133135
}
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package common
18+
19+
import (
20+
"errors"
21+
"testing"
22+
"time"
23+
24+
"github.com/google/go-cmp/cmp"
25+
"github.com/google/go-cmp/cmp/cmpopts"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
v1 "k8s.io/autoscaler/cluster-autoscaler/apis/capacitybuffer/autoscaling.x-k8s.io/v1beta1"
28+
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer"
29+
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/testutil"
30+
"k8s.io/utils/ptr"
31+
)
32+
33+
func TestConditionUpdates(t *testing.T) {
34+
existingCondition := metav1.Condition{
35+
Type: "ExistingCondition",
36+
Status: metav1.ConditionTrue,
37+
Reason: "Existing",
38+
Message: "Existing message",
39+
LastTransitionTime: metav1.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC),
40+
}
41+
42+
tests := []struct {
43+
name string
44+
initialBuffer *v1.CapacityBuffer
45+
updateFunc func(*v1.CapacityBuffer)
46+
wantConditions []metav1.Condition
47+
}{
48+
{
49+
name: "SetBufferAsReadyForProvisioning preserves existing conditions",
50+
initialBuffer: testutil.NewBuffer(func(b *v1.CapacityBuffer) {
51+
b.Status.Conditions = []metav1.Condition{existingCondition}
52+
}),
53+
updateFunc: func(b *v1.CapacityBuffer) {
54+
SetBufferAsReadyForProvisioning(b, &v1.LocalObjectRef{Name: "pt"}, ptr.To(int64(1)), ptr.To(int32(1)), ptr.To(capacitybuffer.ActiveProvisioningStrategy))
55+
},
56+
wantConditions: []metav1.Condition{
57+
existingCondition,
58+
{
59+
Type: capacitybuffer.ReadyForProvisioningCondition,
60+
Status: metav1.ConditionTrue,
61+
Reason: capacitybuffer.AttributesSetSuccessfullyReason,
62+
Message: "ready",
63+
},
64+
},
65+
},
66+
{
67+
name: "SetBufferAsNotReadyForProvisioning preserves existing conditions",
68+
initialBuffer: testutil.NewBuffer(func(b *v1.CapacityBuffer) {
69+
b.Status.Conditions = []metav1.Condition{existingCondition}
70+
}),
71+
updateFunc: func(b *v1.CapacityBuffer) {
72+
SetBufferAsNotReadyForProvisioning(b, &v1.LocalObjectRef{Name: "pt"}, ptr.To(int64(1)), ptr.To(int32(1)), ptr.To(capacitybuffer.ActiveProvisioningStrategy), errors.New("some error"))
73+
},
74+
wantConditions: []metav1.Condition{
75+
existingCondition,
76+
{
77+
Type: capacitybuffer.ReadyForProvisioningCondition,
78+
Status: metav1.ConditionFalse,
79+
Reason: "error",
80+
Message: "some error",
81+
},
82+
},
83+
},
84+
{
85+
name: "UpdateBufferStatusToFailedProvisioning preserves existing conditions",
86+
initialBuffer: testutil.NewBuffer(func(b *v1.CapacityBuffer) {
87+
b.Status.Conditions = []metav1.Condition{existingCondition}
88+
}),
89+
updateFunc: func(b *v1.CapacityBuffer) {
90+
UpdateBufferStatusToFailedProvisioning(b, "FailedReason", "failed message")
91+
},
92+
wantConditions: []metav1.Condition{
93+
existingCondition,
94+
{
95+
Type: capacitybuffer.ProvisioningCondition,
96+
Status: metav1.ConditionFalse,
97+
Reason: "FailedReason",
98+
Message: "failed message",
99+
},
100+
},
101+
},
102+
{
103+
name: "UpdateBufferStatusToSuccessfullyProvisioning preserves existing conditions",
104+
initialBuffer: testutil.NewBuffer(func(b *v1.CapacityBuffer) {
105+
b.Status.Conditions = []metav1.Condition{existingCondition}
106+
}),
107+
updateFunc: func(b *v1.CapacityBuffer) {
108+
UpdateBufferStatusToSuccessfullyProvisioning(b, "SuccessReason")
109+
},
110+
wantConditions: []metav1.Condition{
111+
existingCondition,
112+
{
113+
Type: capacitybuffer.ProvisioningCondition,
114+
Status: metav1.ConditionTrue,
115+
Reason: "SuccessReason",
116+
Message: "",
117+
},
118+
},
119+
},
120+
{
121+
name: "MarkBufferAsLimitedByQuota preserves existing conditions",
122+
initialBuffer: testutil.NewBuffer(func(b *v1.CapacityBuffer) {
123+
b.Status.Conditions = []metav1.Condition{existingCondition}
124+
}),
125+
updateFunc: func(b *v1.CapacityBuffer) {
126+
MarkBufferAsLimitedByQuota(b, 10, 5, []string{"quota-1", "quota-2"})
127+
},
128+
wantConditions: []metav1.Condition{
129+
existingCondition,
130+
{
131+
Type: capacitybuffer.LimitedByQuotasCondition,
132+
Status: metav1.ConditionTrue,
133+
Reason: capacitybuffer.LimitedByQuotasReason,
134+
Message: "Buffer replicas limited from 10 to 5 due to quotas: quota-1, quota-2",
135+
},
136+
},
137+
},
138+
{
139+
name: "UpdateBufferStatusLimitedByQuotas preserves existing conditions",
140+
initialBuffer: testutil.NewBuffer(func(b *v1.CapacityBuffer) {
141+
b.Status.Conditions = []metav1.Condition{existingCondition}
142+
}),
143+
updateFunc: func(b *v1.CapacityBuffer) {
144+
UpdateBufferStatusLimitedByQuotas(b, false, "")
145+
},
146+
wantConditions: []metav1.Condition{
147+
existingCondition,
148+
{
149+
Type: capacitybuffer.LimitedByQuotasCondition,
150+
Status: metav1.ConditionFalse,
151+
Reason: capacitybuffer.LimitedByQuotasReason,
152+
},
153+
},
154+
},
155+
}
156+
157+
for _, tc := range tests {
158+
t.Run(tc.name, func(t *testing.T) {
159+
tc.updateFunc(tc.initialBuffer)
160+
161+
sortOpt := cmpopts.SortSlices(func(a, b metav1.Condition) bool {
162+
return a.Type < b.Type
163+
})
164+
ignoreTime := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")
165+
166+
if diff := cmp.Diff(tc.wantConditions, tc.initialBuffer.Status.Conditions, sortOpt, ignoreTime); diff != "" {
167+
t.Errorf("Conditions mismatch (-want +got):\n%s", diff)
168+
}
169+
})
170+
}
171+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package capacitybuffer
18+
19+
// Constants to use in Capacity Buffers objects
20+
const (
21+
ActiveProvisioningStrategy = "buffer.x-k8s.io/active-capacity"
22+
CapacityBufferKind = "CapacityBuffer"
23+
CapacityBufferApiVersion = "autoscaling.x-k8s.io/v1beta1"
24+
ReadyForProvisioningCondition = "ReadyForProvisioning"
25+
ProvisioningCondition = "Provisioning"
26+
LimitedByQuotasCondition = "LimitedByQuotas"
27+
LimitedByQuotasReason = "ResourceQuotasAllocated"
28+
AttributesSetSuccessfullyReason = "AttributesSetSuccessfully"
29+
)

cluster-autoscaler/capacitybuffer/controller/controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ import (
3131
"k8s.io/klog/v2"
3232

3333
v1 "k8s.io/autoscaler/cluster-autoscaler/apis/capacitybuffer/autoscaling.x-k8s.io/v1beta1"
34+
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer"
3435
cbclient "k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/client"
35-
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/common"
3636
filters "k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/filters"
3737
translators "k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/translators"
3838
updater "k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/updater"
@@ -81,7 +81,7 @@ func NewDefaultBufferController(
8181
bc := &bufferController{
8282
client: client,
8383
// Accepting empty string as it represents nil value for ProvisioningStrategy
84-
strategyFilter: filters.NewStrategyFilter([]string{common.ActiveProvisioningStrategy, ""}),
84+
strategyFilter: filters.NewStrategyFilter([]string{capacitybuffer.ActiveProvisioningStrategy, ""}),
8585
translator: translators.NewCombinedTranslator(
8686
[]translators.Translator{
8787
translators.NewPodTemplateBufferTranslator(client),

cluster-autoscaler/capacitybuffer/controller/controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ import (
3232
"k8s.io/apimachinery/pkg/util/wait"
3333
v1 "k8s.io/autoscaler/cluster-autoscaler/apis/capacitybuffer/autoscaling.x-k8s.io/v1beta1"
3434
fakebuffers "k8s.io/autoscaler/cluster-autoscaler/apis/capacitybuffer/client/clientset/versioned/fake"
35+
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer"
3536
cbclient "k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/client"
36-
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/common"
3737
"k8s.io/autoscaler/cluster-autoscaler/capacitybuffer/testutil"
3838
fakek8s "k8s.io/client-go/kubernetes/fake"
3939
k8stesting "k8s.io/client-go/testing"
@@ -113,7 +113,7 @@ func TestControllerIntegration_ResourceQuotas(t *testing.T) {
113113
if checkLimited {
114114
hasLimit := false
115115
for _, c := range b.Status.Conditions {
116-
if c.Type == common.LimitedByQuotasCondition && c.Status == common.ConditionTrue {
116+
if c.Type == capacitybuffer.LimitedByQuotasCondition && c.Status == metav1.ConditionTrue {
117117
hasLimit = true
118118
}
119119
}
@@ -128,7 +128,7 @@ func TestControllerIntegration_ResourceQuotas(t *testing.T) {
128128
b, _ := buffersClient.AutoscalingV1beta1().CapacityBuffers("default").Get(ctx, name, metav1.GetOptions{})
129129
var gotLimited bool
130130
for _, c := range b.Status.Conditions {
131-
if c.Type == common.LimitedByQuotasCondition && c.Status == common.ConditionTrue {
131+
if c.Type == capacitybuffer.LimitedByQuotasCondition && c.Status == metav1.ConditionTrue {
132132
gotLimited = true
133133
}
134134
}

0 commit comments

Comments
 (0)