Skip to content

Commit f35cdf1

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #5111 from bryan-cox/4204
Set default values in API rather than in webhooks
2 parents 61c6f3f + a0b707d commit f35cdf1

21 files changed

+96
-326
lines changed

‎api/v1beta1/azuremachine_default.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,6 @@ func (s *AzureMachineSpec) SetDefaultSSHPublicKey() error {
4949
return nil
5050
}
5151

52-
// SetDefaultCachingType sets the default cache type for an AzureMachine.
53-
func (s *AzureMachineSpec) SetDefaultCachingType() {
54-
if s.OSDisk.CachingType == "" {
55-
s.OSDisk.CachingType = "None"
56-
}
57-
}
58-
5952
// SetDataDisksDefaults sets the data disk defaults for an AzureMachine.
6053
func (s *AzureMachineSpec) SetDataDisksDefaults() {
6154
set := make(map[int32]struct{})
@@ -245,7 +238,6 @@ func (m *AzureMachine) SetDefaults(client client.Client) error {
245238
errs = append(errs, errors.Wrapf(err, "failed to fetch subscription ID for AzureMachine %s/%s", m.Namespace, m.Name))
246239
}
247240

248-
m.Spec.SetDefaultCachingType()
249241
m.Spec.SetDataDisksDefaults()
250242
m.Spec.SetIdentityDefaults(subscriptionID)
251243
m.Spec.SetSpotEvictionPolicyDefaults()

‎api/v1beta1/azuremachine_webhook_test.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -988,11 +988,6 @@ func TestAzureMachine_Default(t *testing.T) {
988988
g.Expect(err).NotTo(HaveOccurred())
989989
g.Expect(publicKeyNotExistTest.machine.Spec.SSHPublicKey).To(Not(BeEmpty()))
990990

991-
cacheTypeNotSpecifiedTest := test{machine: &AzureMachine{ObjectMeta: testObjectMeta, Spec: AzureMachineSpec{OSDisk: OSDisk{CachingType: ""}}}}
992-
err = mw.Default(context.Background(), cacheTypeNotSpecifiedTest.machine)
993-
g.Expect(err).NotTo(HaveOccurred())
994-
g.Expect(cacheTypeNotSpecifiedTest.machine.Spec.OSDisk.CachingType).To(Equal("None"))
995-
996991
for _, possibleCachingType := range armcompute.PossibleCachingTypesValues() {
997992
cacheTypeSpecifiedTest := test{machine: &AzureMachine{ObjectMeta: testObjectMeta, Spec: AzureMachineSpec{OSDisk: OSDisk{CachingType: string(possibleCachingType)}}}}
998993
err = mw.Default(context.Background(), cacheTypeSpecifiedTest.machine)

‎api/v1beta1/azuremachinetemplate_webhook.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@ func (r *AzureMachineTemplate) Default(_ context.Context, obj runtime.Object) er
152152
if err := t.Spec.Template.Spec.SetDefaultSSHPublicKey(); err != nil {
153153
ctrl.Log.WithName("SetDefault").Error(err, "SetDefaultSSHPublicKey failed")
154154
}
155-
t.Spec.Template.Spec.SetDefaultCachingType()
156155
t.Spec.Template.Spec.SetDataDisksDefaults()
157156
t.Spec.Template.Spec.SetNetworkInterfacesDefaults()
158157
return nil

‎api/v1beta1/azuremachinetemplate_webhook_test.go‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -346,53 +346,6 @@ func TestAzureMachineTemplate_ValidateUpdate(t *testing.T) {
346346
},
347347
wantErr: false,
348348
},
349-
{
350-
name: "AzureMachineTemplate with default mismatch",
351-
oldTemplate: &AzureMachineTemplate{
352-
Spec: AzureMachineTemplateSpec{
353-
Template: AzureMachineTemplateResource{
354-
Spec: AzureMachineSpec{
355-
VMSize: "size",
356-
FailureDomain: &failureDomain,
357-
OSDisk: OSDisk{
358-
OSType: "type",
359-
DiskSizeGB: ptr.To[int32](11),
360-
CachingType: "",
361-
},
362-
DataDisks: []DataDisk{},
363-
SSHPublicKey: "",
364-
},
365-
},
366-
},
367-
ObjectMeta: metav1.ObjectMeta{
368-
Name: "OldTemplate",
369-
},
370-
},
371-
template: &AzureMachineTemplate{
372-
Spec: AzureMachineTemplateSpec{
373-
Template: AzureMachineTemplateResource{
374-
Spec: AzureMachineSpec{
375-
VMSize: "size",
376-
FailureDomain: &failureDomain,
377-
OSDisk: OSDisk{
378-
OSType: "type",
379-
DiskSizeGB: ptr.To[int32](11),
380-
CachingType: "None",
381-
},
382-
DataDisks: []DataDisk{},
383-
SSHPublicKey: "fake ssh key",
384-
NetworkInterfaces: []NetworkInterface{{
385-
PrivateIPConfigs: 1,
386-
}},
387-
},
388-
},
389-
},
390-
ObjectMeta: metav1.ObjectMeta{
391-
Name: "NewTemplate",
392-
},
393-
},
394-
wantErr: false,
395-
},
396349
{
397350
name: "AzureMachineTemplate ssh key removed",
398351
oldTemplate: &AzureMachineTemplate{

‎api/v1beta1/azuremanagedcontrolplane_default.go‎

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,6 @@ func setDefaultFleetsMember(fleetsMember *FleetsMember, labels map[string]string
106106
if clusterName, ok := labels[clusterv1.ClusterNameLabel]; ok && fleetsMember.Name == "" {
107107
result.Name = clusterName
108108
}
109-
if fleetsMember.Group == "" {
110-
result.Group = "default"
111-
}
112109
}
113110
return result
114111
}
@@ -133,79 +130,10 @@ func setDefaultVersion(version string) string {
133130
return version
134131
}
135132

136-
func setDefaultAutoScalerProfile(autoScalerProfile *AutoScalerProfile) *AutoScalerProfile {
137-
if autoScalerProfile == nil {
138-
return nil
139-
}
140-
141-
result := autoScalerProfile.DeepCopy()
142-
143-
// Default values are from https://learn.microsoft.com/en-us/azure/aks/cluster-autoscaler#using-the-autoscaler-profile
144-
// If any values are set, they all need to be set.
145-
if autoScalerProfile.BalanceSimilarNodeGroups == nil {
146-
result.BalanceSimilarNodeGroups = (*BalanceSimilarNodeGroups)(ptr.To(string(BalanceSimilarNodeGroupsFalse)))
147-
}
148-
if autoScalerProfile.Expander == nil {
149-
result.Expander = (*Expander)(ptr.To(string(ExpanderRandom)))
150-
}
151-
if autoScalerProfile.MaxEmptyBulkDelete == nil {
152-
result.MaxEmptyBulkDelete = ptr.To("10")
153-
}
154-
if autoScalerProfile.MaxGracefulTerminationSec == nil {
155-
result.MaxGracefulTerminationSec = ptr.To("600")
156-
}
157-
if autoScalerProfile.MaxNodeProvisionTime == nil {
158-
result.MaxNodeProvisionTime = ptr.To("15m")
159-
}
160-
if autoScalerProfile.MaxTotalUnreadyPercentage == nil {
161-
result.MaxTotalUnreadyPercentage = ptr.To("45")
162-
}
163-
if autoScalerProfile.NewPodScaleUpDelay == nil {
164-
result.NewPodScaleUpDelay = ptr.To("0s")
165-
}
166-
if autoScalerProfile.OkTotalUnreadyCount == nil {
167-
result.OkTotalUnreadyCount = ptr.To("3")
168-
}
169-
if autoScalerProfile.ScanInterval == nil {
170-
result.ScanInterval = ptr.To("10s")
171-
}
172-
if autoScalerProfile.ScaleDownDelayAfterAdd == nil {
173-
result.ScaleDownDelayAfterAdd = ptr.To("10m")
174-
}
175-
if autoScalerProfile.ScaleDownDelayAfterDelete == nil {
176-
// Default is the same as the ScanInterval so default to that same value if it isn't set
177-
result.ScaleDownDelayAfterDelete = result.ScanInterval
178-
}
179-
if autoScalerProfile.ScaleDownDelayAfterFailure == nil {
180-
result.ScaleDownDelayAfterFailure = ptr.To("3m")
181-
}
182-
if autoScalerProfile.ScaleDownUnneededTime == nil {
183-
result.ScaleDownUnneededTime = ptr.To("10m")
184-
}
185-
if autoScalerProfile.ScaleDownUnreadyTime == nil {
186-
result.ScaleDownUnreadyTime = ptr.To("20m")
187-
}
188-
if autoScalerProfile.ScaleDownUtilizationThreshold == nil {
189-
result.ScaleDownUtilizationThreshold = ptr.To("0.5")
190-
}
191-
if autoScalerProfile.SkipNodesWithLocalStorage == nil {
192-
result.SkipNodesWithLocalStorage = (*SkipNodesWithLocalStorage)(ptr.To(string(SkipNodesWithLocalStorageFalse)))
193-
}
194-
if autoScalerProfile.SkipNodesWithSystemPods == nil {
195-
result.SkipNodesWithSystemPods = (*SkipNodesWithSystemPods)(ptr.To(string(SkipNodesWithSystemPodsTrue)))
196-
}
197-
198-
return result
199-
}
200-
201133
func (m *AzureManagedControlPlane) setDefaultOIDCIssuerProfile() {
202134
if m.Spec.OIDCIssuerProfile == nil {
203135
m.Spec.OIDCIssuerProfile = &OIDCIssuerProfile{}
204136
}
205-
206-
if m.Spec.OIDCIssuerProfile.Enabled == nil {
207-
m.Spec.OIDCIssuerProfile.Enabled = ptr.To(false)
208-
}
209137
}
210138

211139
func (m *AzureManagedControlPlane) setDefaultDNSPrefix() {
@@ -219,8 +147,5 @@ func (m *AzureManagedControlPlane) setDefaultAKSExtensions() {
219147
if extension.Plan != nil && extension.Plan.Name == "" {
220148
extension.Plan.Name = fmt.Sprintf("%s-%s", m.Name, extension.Plan.Product)
221149
}
222-
if extension.AutoUpgradeMinorVersion == nil {
223-
extension.AutoUpgradeMinorVersion = ptr.To(true)
224-
}
225150
}
226151
}

‎api/v1beta1/azuremanagedcontrolplane_default_test.go‎

Lines changed: 0 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"testing"
2121

2222
. "github.com/onsi/gomega"
23-
"k8s.io/utils/ptr"
2423
)
2524

2625
func TestAzureManagedControlPlane_SetDefaultSSHPublicKey(t *testing.T) {
@@ -54,87 +53,3 @@ func hardcodedAzureManagedControlPlaneWithSSHKey(sshPublicKey string) *AzureMana
5453
},
5554
}
5655
}
57-
58-
func TestSetDefaultAutoScalerProfile(t *testing.T) {
59-
g := NewWithT(t)
60-
61-
type test struct {
62-
amcp *AzureManagedControlPlane
63-
}
64-
65-
defaultAMP := &AzureManagedControlPlane{
66-
Spec: AzureManagedControlPlaneSpec{
67-
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
68-
AutoScalerProfile: &AutoScalerProfile{
69-
BalanceSimilarNodeGroups: (*BalanceSimilarNodeGroups)(ptr.To(string(BalanceSimilarNodeGroupsFalse))),
70-
Expander: (*Expander)(ptr.To(string(ExpanderRandom))),
71-
MaxEmptyBulkDelete: ptr.To("10"),
72-
MaxGracefulTerminationSec: ptr.To("600"),
73-
MaxNodeProvisionTime: ptr.To("15m"),
74-
MaxTotalUnreadyPercentage: ptr.To("45"),
75-
NewPodScaleUpDelay: ptr.To("0s"),
76-
OkTotalUnreadyCount: ptr.To("3"),
77-
ScanInterval: ptr.To("10s"),
78-
ScaleDownDelayAfterAdd: ptr.To("10m"),
79-
ScaleDownDelayAfterDelete: ptr.To("10s"),
80-
ScaleDownDelayAfterFailure: ptr.To("3m"),
81-
ScaleDownUnneededTime: ptr.To("10m"),
82-
ScaleDownUnreadyTime: ptr.To("20m"),
83-
ScaleDownUtilizationThreshold: ptr.To("0.5"),
84-
SkipNodesWithLocalStorage: (*SkipNodesWithLocalStorage)(ptr.To(string(SkipNodesWithLocalStorageFalse))),
85-
SkipNodesWithSystemPods: (*SkipNodesWithSystemPods)(ptr.To(string(SkipNodesWithSystemPodsTrue))),
86-
},
87-
},
88-
},
89-
}
90-
91-
allFieldsAreNilTest := test{amcp: &AzureManagedControlPlane{
92-
Spec: AzureManagedControlPlaneSpec{
93-
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
94-
AutoScalerProfile: &AutoScalerProfile{},
95-
},
96-
},
97-
}}
98-
99-
allFieldsAreNilTest.amcp.Spec.AutoScalerProfile = setDefaultAutoScalerProfile(allFieldsAreNilTest.amcp.Spec.AutoScalerProfile)
100-
101-
g.Expect(allFieldsAreNilTest.amcp.Spec.AutoScalerProfile).To(Equal(defaultAMP.Spec.AutoScalerProfile))
102-
103-
expectedNotNil := &AzureManagedControlPlane{
104-
Spec: AzureManagedControlPlaneSpec{
105-
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
106-
AutoScalerProfile: &AutoScalerProfile{
107-
BalanceSimilarNodeGroups: (*BalanceSimilarNodeGroups)(ptr.To(string(BalanceSimilarNodeGroupsTrue))),
108-
Expander: (*Expander)(ptr.To(string(ExpanderLeastWaste))),
109-
MaxEmptyBulkDelete: ptr.To("5"),
110-
MaxGracefulTerminationSec: ptr.To("300"),
111-
MaxNodeProvisionTime: ptr.To("10m"),
112-
MaxTotalUnreadyPercentage: ptr.To("30"),
113-
NewPodScaleUpDelay: ptr.To("30s"),
114-
OkTotalUnreadyCount: ptr.To("5"),
115-
ScanInterval: ptr.To("20s"),
116-
ScaleDownDelayAfterAdd: ptr.To("5m"),
117-
ScaleDownDelayAfterDelete: ptr.To("1m"),
118-
ScaleDownDelayAfterFailure: ptr.To("2m"),
119-
ScaleDownUnneededTime: ptr.To("5m"),
120-
ScaleDownUnreadyTime: ptr.To("10m"),
121-
ScaleDownUtilizationThreshold: ptr.To("0.4"),
122-
SkipNodesWithLocalStorage: (*SkipNodesWithLocalStorage)(ptr.To(string(SkipNodesWithLocalStorageTrue))),
123-
SkipNodesWithSystemPods: (*SkipNodesWithSystemPods)(ptr.To(string(SkipNodesWithSystemPodsFalse))),
124-
},
125-
},
126-
},
127-
}
128-
129-
allFieldsAreNotNilTest := test{amcp: &AzureManagedControlPlane{
130-
Spec: AzureManagedControlPlaneSpec{
131-
AzureManagedControlPlaneClassSpec: AzureManagedControlPlaneClassSpec{
132-
AutoScalerProfile: ptr.To(*expectedNotNil.Spec.AutoScalerProfile),
133-
},
134-
},
135-
}}
136-
137-
allFieldsAreNotNilTest.amcp.Spec.AutoScalerProfile = setDefaultAutoScalerProfile(allFieldsAreNotNilTest.amcp.Spec.AutoScalerProfile)
138-
139-
g.Expect(allFieldsAreNotNilTest.amcp.Spec.AutoScalerProfile).To(Equal(expectedNotNil.Spec.AutoScalerProfile))
140-
}

0 commit comments

Comments
 (0)