Skip to content

Commit ff6f736

Browse files
sbueringersbueringer
authored
⚠️ Change type of int32 fields with valid zero value to *int32 (#12424)
* Change type of int32 fields with valid zero value to *int32 Signed-off-by: Stefan Büringer buringerst@vmware.com * Fix review findings --------- Signed-off-by: Stefan Büringer buringerst@vmware.com
1 parent ed4e6b2 commit ff6f736

36 files changed

+538
-250
lines changed

‎api/controlplane/kubeadm/v1beta1/conversion.go‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,17 @@ func (src *KubeadmControlPlane) ConvertTo(dstRaw conversion.Hub) error {
8383
bootstrapv1beta1.RestoreKubeadmConfigSpec(&restored.Spec.KubeadmConfigSpec, &dst.Spec.KubeadmConfigSpec)
8484
}
8585

86+
if src.Spec.RemediationStrategy != nil {
87+
if dst.Spec.RemediationStrategy == nil {
88+
dst.Spec.RemediationStrategy = &controlplanev1.RemediationStrategy{}
89+
}
90+
var restoredRetryPeriodSeconds *int32
91+
if restored.Spec.RemediationStrategy != nil {
92+
restoredRetryPeriodSeconds = restored.Spec.RemediationStrategy.RetryPeriodSeconds
93+
}
94+
clusterv1.Convert_Duration_To_Pointer_int32(src.Spec.RemediationStrategy.RetryPeriod, ok, restoredRetryPeriodSeconds, &dst.Spec.RemediationStrategy.RetryPeriodSeconds)
95+
}
96+
8697
// Override restored data with timeouts values already existing in v1beta1 but in other structs.
8798
src.Spec.KubeadmConfigSpec.ConvertTo(&dst.Spec.KubeadmConfigSpec)
8899
return nil
@@ -133,6 +144,17 @@ func (src *KubeadmControlPlaneTemplate) ConvertTo(dstRaw conversion.Hub) error {
133144
bootstrapv1beta1.RestoreKubeadmConfigSpec(&restored.Spec.Template.Spec.KubeadmConfigSpec, &dst.Spec.Template.Spec.KubeadmConfigSpec)
134145
}
135146

147+
if src.Spec.Template.Spec.RemediationStrategy != nil {
148+
if dst.Spec.Template.Spec.RemediationStrategy == nil {
149+
dst.Spec.Template.Spec.RemediationStrategy = &controlplanev1.RemediationStrategy{}
150+
}
151+
var restoredRetryPeriodSeconds *int32
152+
if restored.Spec.Template.Spec.RemediationStrategy != nil {
153+
restoredRetryPeriodSeconds = restored.Spec.Template.Spec.RemediationStrategy.RetryPeriodSeconds
154+
}
155+
clusterv1.Convert_Duration_To_Pointer_int32(src.Spec.Template.Spec.RemediationStrategy.RetryPeriod, ok, restoredRetryPeriodSeconds, &dst.Spec.Template.Spec.RemediationStrategy.RetryPeriodSeconds)
156+
}
157+
136158
// Override restored data with timeouts values already existing in v1beta1 but in other structs.
137159
src.Spec.Template.Spec.KubeadmConfigSpec.ConvertTo(&dst.Spec.Template.Spec.KubeadmConfigSpec)
138160
return nil
@@ -281,7 +303,7 @@ func Convert_v1beta1_RemediationStrategy_To_v1beta2_RemediationStrategy(in *Reme
281303
return err
282304
}
283305
out.MinHealthyPeriodSeconds = clusterv1.ConvertToSeconds(in.MinHealthyPeriod)
284-
out.RetryPeriodSeconds = ptr.Deref(clusterv1.ConvertToSeconds(&in.RetryPeriod), 0)
306+
out.RetryPeriodSeconds = clusterv1.ConvertToSeconds(&in.RetryPeriod)
285307
return nil
286308
}
287309

@@ -290,7 +312,7 @@ func Convert_v1beta2_RemediationStrategy_To_v1beta1_RemediationStrategy(in *cont
290312
return err
291313
}
292314
out.MinHealthyPeriod = clusterv1.ConvertFromSeconds(in.MinHealthyPeriodSeconds)
293-
out.RetryPeriod = ptr.Deref(clusterv1.ConvertFromSeconds(&in.RetryPeriodSeconds), metav1.Duration{})
315+
out.RetryPeriod = ptr.Deref(clusterv1.ConvertFromSeconds(in.RetryPeriodSeconds), metav1.Duration{})
294316
return nil
295317
}
296318

‎api/controlplane/kubeadm/v1beta2/kubeadm_control_plane_types.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ type RemediationStrategy struct {
583583
// If not set, a retry will happen immediately.
584584
// +optional
585585
// +kubebuilder:validation:Minimum=0
586-
RetryPeriodSeconds int32 `json:"retryPeriodSeconds,omitempty"`
586+
RetryPeriodSeconds *int32 `json:"retryPeriodSeconds,omitempty"`
587587

588588
// minHealthyPeriodSeconds defines the duration after which KCP will consider any failure to a machine unrelated
589589
// from the previous one. In this case the remediation is not considered a retry anymore, and thus the retry
@@ -740,14 +740,14 @@ type KubeadmControlPlaneV1Beta1DeprecatedStatus struct {
740740
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
741741
//
742742
// +optional
743-
UpdatedReplicas int32 `json:"updatedReplicas"`
743+
UpdatedReplicas int32 `json:"updatedReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
744744

745745
// readyReplicas is the total number of fully running and ready control plane machines.
746746
//
747747
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
748748
//
749749
// +optional
750-
ReadyReplicas int32 `json:"readyReplicas"`
750+
ReadyReplicas int32 `json:"readyReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
751751

752752
// unavailableReplicas is the total number of unavailable machines targeted by this control plane.
753753
// This is the total number of machines that are still required for
@@ -758,7 +758,7 @@ type KubeadmControlPlaneV1Beta1DeprecatedStatus struct {
758758
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
759759
//
760760
// +optional
761-
UnavailableReplicas int32 `json:"unavailableReplicas"`
761+
UnavailableReplicas int32 `json:"unavailableReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
762762
}
763763

764764
// LastRemediationStatus stores info about last remediation performed.

‎api/controlplane/kubeadm/v1beta2/zz_generated.deepcopy.go‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/core/v1beta1/conversion.go‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,22 @@ func (dst *MachineDeployment) ConvertFrom(srcRaw conversion.Hub) error {
499499
func (src *MachineHealthCheck) ConvertTo(dstRaw conversion.Hub) error {
500500
dst := dstRaw.(*clusterv1.MachineHealthCheck)
501501

502-
return Convert_v1beta1_MachineHealthCheck_To_v1beta2_MachineHealthCheck(src, dst, nil)
502+
if err := Convert_v1beta1_MachineHealthCheck_To_v1beta2_MachineHealthCheck(src, dst, nil); err != nil {
503+
return err
504+
}
505+
506+
// Manually restore data.
507+
restored := &clusterv1.MachineHealthCheck{}
508+
ok, err := utilconversion.UnmarshalData(src, restored)
509+
if err != nil {
510+
return err
511+
}
512+
513+
clusterv1.Convert_int32_To_Pointer_int32(src.Status.ExpectedMachines, ok, restored.Status.ExpectedMachines, &dst.Status.ExpectedMachines)
514+
clusterv1.Convert_int32_To_Pointer_int32(src.Status.CurrentHealthy, ok, restored.Status.CurrentHealthy, &dst.Status.CurrentHealthy)
515+
clusterv1.Convert_int32_To_Pointer_int32(src.Status.RemediationsAllowed, ok, restored.Status.RemediationsAllowed, &dst.Status.RemediationsAllowed)
516+
517+
return nil
503518
}
504519

505520
func (dst *MachineHealthCheck) ConvertFrom(srcRaw conversion.Hub) error {
@@ -513,7 +528,8 @@ func (dst *MachineHealthCheck) ConvertFrom(srcRaw conversion.Hub) error {
513528
}
514529

515530
dropEmptyStringsMachineHealthCheck(dst)
516-
return nil
531+
532+
return utilconversion.MarshalData(src, dst)
517533
}
518534

519535
func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error {

‎api/core/v1beta1/zz_generated.conversion.go‎

Lines changed: 18 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/core/v1beta2/conversion.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,35 @@ func Convert_bool_To_Pointer_bool(in bool, hasRestored bool, restoredIn *bool, o
7272
// Otherwise, if the value is true, convert to *true.
7373
*out = ptr.To(true)
7474
}
75+
76+
func Convert_int32_To_Pointer_int32(in int32, hasRestored bool, restoredIn *int32, out **int32) {
77+
// If the value is 0, convert to *0 only if the value was *0 before (we know it was intentionally set to 0).
78+
// In all the other cases we do not know if the value was intentionally set to 0, so convert to nil.
79+
if in == 0 {
80+
if hasRestored && restoredIn != nil && *restoredIn == 0 {
81+
*out = ptr.To[int32](0)
82+
return
83+
}
84+
*out = nil
85+
return
86+
}
87+
88+
// Otherwise, if the value is not 0, convert to *value.
89+
*out = ptr.To(in)
90+
}
91+
92+
func Convert_Duration_To_Pointer_int32(in metav1.Duration, hasRestored bool, restoredIn *int32, out **int32) {
93+
// If the value is 0s, convert to *0 only if the value was *0 before (we know it was intentionally set to 0).
94+
// In all the other cases we do not know if the value was intentionally set to 0, so convert to nil.
95+
if in.Nanoseconds() == 0 {
96+
if hasRestored && restoredIn != nil && *restoredIn == 0 {
97+
*out = ptr.To[int32](0)
98+
return
99+
}
100+
*out = nil
101+
return
102+
}
103+
104+
// Otherwise, if the value is not 0, convert to *value.
105+
*out = ConvertToSeconds(&in)
106+
}

‎api/core/v1beta2/conversion_test.go‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,147 @@ func TestConvert_bool_To_Pointer_bool(t *testing.T) {
113113
})
114114
}
115115
}
116+
117+
func TestConvert_int32_To_Pointer_int32(t *testing.T) {
118+
testCases := []struct {
119+
name string
120+
in int32
121+
hasRestored bool
122+
restored *int32
123+
wantOut *int32
124+
}{
125+
{
126+
name: "when applying v1beta1, 0 should be converted to nil",
127+
in: 0,
128+
wantOut: nil,
129+
},
130+
{
131+
name: "when applying v1beta1, value!=0 should be converted to *value",
132+
in: 1,
133+
wantOut: ptr.To[int32](1),
134+
},
135+
{
136+
name: "when doing round trip, 0 should be converted to nil if not previously explicitly set to 0 (previously set to nil)",
137+
in: 0,
138+
hasRestored: true,
139+
restored: nil,
140+
wantOut: nil,
141+
},
142+
{
143+
name: "when doing round trip, 0 should be converted to nil if not previously explicitly set to 0 (previously set to another value)",
144+
in: 0,
145+
hasRestored: true,
146+
restored: ptr.To[int32](1),
147+
wantOut: nil,
148+
},
149+
{
150+
name: "when doing round trip, 0 should be converted to 0 if previously explicitly set to 0",
151+
in: 0,
152+
hasRestored: true,
153+
restored: ptr.To[int32](0),
154+
wantOut: ptr.To[int32](0),
155+
},
156+
{
157+
name: "when doing round trip, value should be converted to *value (no matter of restored value is nil)",
158+
in: 1,
159+
hasRestored: true,
160+
restored: nil,
161+
wantOut: ptr.To[int32](1),
162+
},
163+
{
164+
name: "when doing round trip, value should be converted to *value (no matter of restored value is not 0)",
165+
in: 1,
166+
hasRestored: true,
167+
restored: ptr.To[int32](2),
168+
wantOut: ptr.To[int32](1),
169+
},
170+
{
171+
name: "when doing round trip, value should be converted to *value (no matter of restored value is 0)",
172+
in: 1,
173+
hasRestored: true,
174+
restored: ptr.To[int32](0),
175+
wantOut: ptr.To[int32](1),
176+
},
177+
}
178+
for _, tt := range testCases {
179+
t.Run(tt.name, func(t *testing.T) {
180+
g := NewWithT(t)
181+
182+
var out *int32
183+
Convert_int32_To_Pointer_int32(tt.in, tt.hasRestored, tt.restored, &out)
184+
g.Expect(out).To(Equal(tt.wantOut))
185+
})
186+
}
187+
}
188+
189+
func TestConvert_Duration_To_Pointer_int32(t *testing.T) {
190+
testCases := []struct {
191+
name string
192+
in metav1.Duration
193+
hasRestored bool
194+
restored *int32
195+
wantOut *int32
196+
}{
197+
{
198+
name: "when applying v1beta1, 0s should be converted to nil",
199+
in: metav1.Duration{},
200+
wantOut: nil,
201+
},
202+
{
203+
name: "when applying v1beta1, value!=0s should be converted to *value",
204+
in: metav1.Duration{Duration: 1 * time.Second},
205+
wantOut: ptr.To[int32](1),
206+
},
207+
{
208+
name: "when doing round trip, 0s should be converted to nil if not previously explicitly set to 0s (previously set to nil)",
209+
in: metav1.Duration{},
210+
hasRestored: true,
211+
restored: nil,
212+
wantOut: nil,
213+
},
214+
{
215+
name: "when doing round trip, 0s should be converted to nil if not previously explicitly set to 0s (previously set to another value)",
216+
in: metav1.Duration{},
217+
hasRestored: true,
218+
restored: ptr.To[int32](1),
219+
wantOut: nil,
220+
},
221+
{
222+
name: "when doing round trip, 0s should be converted to 0s if previously explicitly set to 0s",
223+
in: metav1.Duration{},
224+
hasRestored: true,
225+
restored: ptr.To[int32](0),
226+
wantOut: ptr.To[int32](0),
227+
},
228+
{
229+
name: "when doing round trip, value should be converted to *value (no matter of restored value is nil)",
230+
in: metav1.Duration{Duration: 1 * time.Second},
231+
hasRestored: true,
232+
restored: nil,
233+
wantOut: ptr.To[int32](1),
234+
},
235+
{
236+
name: "when doing round trip, value should be converted to *value (no matter of restored value is not 0s)",
237+
in: metav1.Duration{Duration: 1 * time.Second},
238+
hasRestored: true,
239+
restored: ptr.To[int32](2),
240+
wantOut: ptr.To[int32](1),
241+
},
242+
{
243+
name: "when doing round trip, value should be converted to *value (no matter of restored value is 0s)",
244+
in: metav1.Duration{Duration: 1 * time.Second},
245+
hasRestored: true,
246+
restored: ptr.To[int32](0),
247+
wantOut: ptr.To[int32](1),
248+
},
249+
}
250+
for _, tt := range testCases {
251+
t.Run(tt.name, func(t *testing.T) {
252+
g := NewWithT(t)
253+
254+
var out *int32
255+
Convert_Duration_To_Pointer_int32(tt.in, tt.hasRestored, tt.restored, &out)
256+
g.Expect(out).To(Equal(tt.wantOut))
257+
})
258+
}
259+
}

‎api/core/v1beta2/machinedeployment_types.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,22 +492,22 @@ type MachineDeploymentV1Beta1DeprecatedStatus struct {
492492
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
493493
//
494494
// +optional
495-
UpdatedReplicas int32 `json:"updatedReplicas"`
495+
UpdatedReplicas int32 `json:"updatedReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
496496

497497
// readyReplicas is the total number of ready machines targeted by this deployment.
498498
//
499499
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
500500
//
501501
// +optional
502-
ReadyReplicas int32 `json:"readyReplicas"`
502+
ReadyReplicas int32 `json:"readyReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
503503

504504
// availableReplicas is the total number of available machines (ready for at least minReadySeconds)
505505
// targeted by this deployment.
506506
//
507507
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
508508
//
509509
// +optional
510-
AvailableReplicas int32 `json:"availableReplicas"`
510+
AvailableReplicas int32 `json:"availableReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
511511

512512
// unavailableReplicas is the total number of unavailable machines targeted by this deployment.
513513
// This is the total number of machines that are still required for
@@ -518,7 +518,7 @@ type MachineDeploymentV1Beta1DeprecatedStatus struct {
518518
// Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details.
519519
//
520520
// +optional
521-
UnavailableReplicas int32 `json:"unavailableReplicas"`
521+
UnavailableReplicas int32 `json:"unavailableReplicas"` //nolint:kubeapilinter // field will be removed when v1beta1 is removed
522522
}
523523

524524
// ANCHOR_END: MachineDeploymentStatus

0 commit comments

Comments
 (0)