Skip to content

Commit 0f0b378

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3087 from nikParasyr/v1beta2_flavor
⚠️ Standardize flavor in OpenStackMachine
2 parents b3b1b35 + 3e049fc commit 0f0b378

File tree

47 files changed

+1454
-303
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+1454
-303
lines changed

‎api/v1beta1/conversion.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"fmt"
2021
"sort"
2122
"strings"
2223
"unsafe"
@@ -31,6 +32,7 @@ import (
3132
ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion"
3233

3334
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta2"
35+
"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional"
3436
)
3537

3638
// ConvertTo converts this OpenStackCluster to the Hub version (v1beta2).
@@ -289,6 +291,79 @@ func Convert_v1_Condition_To_v1beta1_Condition(in *metav1.Condition, out *cluste
289291
return nil
290292
}
291293

294+
// Convert_v1beta1_OpenStackMachineSpec_To_v1beta2_OpenStackMachineSpec
295+
// handles manual conversion for Flavor/FlavorID -> FlavorParam.
296+
func Convert_v1beta1_OpenStackMachineSpec_To_v1beta2_OpenStackMachineSpec(
297+
in *OpenStackMachineSpec,
298+
out *infrav1.OpenStackMachineSpec,
299+
s apiconversion.Scope,
300+
) error {
301+
// First copy all identical fields
302+
if err := autoConvert_v1beta1_OpenStackMachineSpec_To_v1beta2_OpenStackMachineSpec(in, out, s); err != nil {
303+
return err
304+
}
305+
306+
switch {
307+
case in.FlavorID != nil:
308+
var id optional.String
309+
_ = optional.Convert_string_To_optional_String(in.FlavorID, &id, s)
310+
311+
out.Flavor = infrav1.FlavorParam{
312+
ID: id,
313+
}
314+
315+
case in.Flavor != nil:
316+
var name optional.String
317+
_ = optional.Convert_string_To_optional_String(in.Flavor, &name, s)
318+
319+
out.Flavor = infrav1.FlavorParam{
320+
Filter: &infrav1.FlavorFilter{
321+
Name: name,
322+
},
323+
}
324+
325+
default:
326+
// This should not be reached due to api validations
327+
return fmt.Errorf("cannot convert OpenStackMachineSpec to v1beta2: " +
328+
"neither Flavor nor FlavorID is set in v1beta1 object")
329+
}
330+
331+
return nil
332+
}
333+
334+
// Convert_v1beta2_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec
335+
// handles manual conversion for FlavorParam -> Flavor/FlavorID.
336+
func Convert_v1beta2_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(
337+
in *infrav1.OpenStackMachineSpec,
338+
out *OpenStackMachineSpec,
339+
s apiconversion.Scope,
340+
) error {
341+
if err := autoConvert_v1beta2_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in, out, s); err != nil {
342+
return err
343+
}
344+
345+
switch {
346+
case in.Flavor.ID != nil && *in.Flavor.ID != "":
347+
id := *in.Flavor.ID
348+
out.FlavorID = &id
349+
out.Flavor = nil
350+
351+
case in.Flavor.Filter != nil &&
352+
in.Flavor.Filter.Name != nil &&
353+
*in.Flavor.Filter.Name != "":
354+
name := *in.Flavor.Filter.Name
355+
out.Flavor = &name
356+
out.FlavorID = nil
357+
358+
default:
359+
// This should not be reached due to api validations
360+
return fmt.Errorf("cannot convert OpenStackMachineSpec to v1beta1: " +
361+
"neither Flavor nor FlavorID is set in v1beta1 object")
362+
}
363+
364+
return nil
365+
}
366+
292367
// LegacyCalicoSecurityGroupRules returns a list of security group rules for calico
293368
// that need to be applied to the control plane and worker security groups when
294369
// managed security groups are enabled and upgrading to v1beta1.

‎api/v1beta1/conversion_test.go‎

Lines changed: 155 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
2727

2828
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta2"
29+
"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional"
2930
)
3031

3132
func TestOpenStackClusterConversion(t *testing.T) {
@@ -48,6 +49,12 @@ func TestOpenStackClusterConversion(t *testing.T) {
4849
},
4950
},
5051
ManagedSecurityGroups: &ManagedSecurityGroups{},
52+
Bastion: &Bastion{
53+
Enabled: ptr.To(true),
54+
Spec: &OpenStackMachineSpec{
55+
Flavor: ptr.To("m1.small"),
56+
},
57+
},
5158
},
5259
Status: OpenStackClusterStatus{
5360
Ready: true,
@@ -92,6 +99,12 @@ func TestOpenStackClusterConversion(t *testing.T) {
9299
g.Expect(dst.Spec.IdentityRef.Name).To(Equal("cloud-config"))
93100
g.Expect(dst.Spec.ManagedSubnets).To(HaveLen(1))
94101

102+
// Verify flavor mapping (name -> FlavorParam.Filter.Name)
103+
g.Expect(dst.Spec.Bastion.Spec.Flavor.ID).To(BeNil())
104+
g.Expect(dst.Spec.Bastion.Spec.Flavor.Filter).NotTo(BeNil())
105+
g.Expect(dst.Spec.Bastion.Spec.Flavor.Filter.Name).NotTo(BeNil())
106+
g.Expect(*dst.Spec.Bastion.Spec.Flavor.Filter.Name).To(Equal("m1.small"))
107+
95108
// Verify FailureDomains converted from map to slice
96109
g.Expect(dst.Status.FailureDomains).To(HaveLen(2))
97110
for _, fd := range dst.Status.FailureDomains {
@@ -138,7 +151,80 @@ func TestOpenStackClusterConversion(t *testing.T) {
138151
g.Expect(restored.Status.FailureDomains["az-2"].Attributes).To(HaveKeyWithValue("region", "us-west-1"))
139152
}
140153

141-
func TestOpenStackMachineConversion(t *testing.T) {
154+
// TestOpenStackMachineConversion_FlavorIDTakesPrecedence verifies that when
155+
// both Flavor (name) and FlavorID are set on a v1beta1 object, FlavorID wins
156+
// on upgrade to v1beta2.
157+
//
158+
// On the round-trip back to v1beta1, CAPI's restore annotation mechanism
159+
// preserves the original Flavor name alongside the restored FlavorID, so both
160+
// fields are non-nil after the round-trip.
161+
func TestOpenStackMachineConversion_FlavorIDTakesPrecedence(t *testing.T) {
162+
g := NewWithT(t)
163+
164+
src := &OpenStackMachine{
165+
ObjectMeta: metav1.ObjectMeta{
166+
Name: "test-machine",
167+
},
168+
Spec: OpenStackMachineSpec{
169+
// Both set — FlavorID should win on upgrade.
170+
Flavor: ptr.To("m1.small"),
171+
FlavorID: ptr.To("uuid-456"),
172+
Image: ImageParam{
173+
Filter: &ImageFilter{
174+
Name: ptr.To("ubuntu-22.04"),
175+
},
176+
},
177+
},
178+
}
179+
180+
dst := &infrav1.OpenStackMachine{}
181+
g.Expect(src.ConvertTo(dst)).To(Succeed())
182+
183+
// FlavorID takes precedence: ID must be set, Filter must be nil.
184+
g.Expect(dst.Spec.Flavor.ID).NotTo(BeNil())
185+
g.Expect(*dst.Spec.Flavor.ID).To(Equal("uuid-456"))
186+
g.Expect(dst.Spec.Flavor.Filter).To(BeNil())
187+
188+
// Round-trip back: FlavorID is restored from the hub value.
189+
// The restore annotation also brings back the original Flavor name, so
190+
// both fields will be non-nil — this is expected CAPI behaviour.
191+
restored := &OpenStackMachine{}
192+
g.Expect(restored.ConvertFrom(dst)).To(Succeed())
193+
194+
g.Expect(restored.Spec.FlavorID).To(Equal(ptr.To("uuid-456")))
195+
// Flavor (name) is restored via annotation — it is NOT lost.
196+
g.Expect(restored.Spec.Flavor).To(Equal(ptr.To("m1.small")))
197+
}
198+
199+
// TestOpenStackMachineConversion_NeitherFlavorNorFlavorID verifies that
200+
// a v1beta1 object with neither Flavor nor FlavorID set is rejected during
201+
// conversion rather than producing an invalid FlavorParam{} in v1beta2.
202+
func TestOpenStackMachineConversion_NeitherFlavorNorFlavorID(t *testing.T) {
203+
g := NewWithT(t)
204+
205+
src := &OpenStackMachine{
206+
ObjectMeta: metav1.ObjectMeta{
207+
Name: "test-machine",
208+
},
209+
Spec: OpenStackMachineSpec{
210+
Image: ImageParam{
211+
Filter: &ImageFilter{
212+
Name: ptr.To("ubuntu-22.04"),
213+
},
214+
},
215+
},
216+
}
217+
218+
dst := &infrav1.OpenStackMachine{}
219+
err := src.ConvertTo(dst)
220+
221+
// Neither Flavor nor FlavorID is set: conversion must fail rather than
222+
// produce FlavorParam{} which would violate MinProperties=1.
223+
g.Expect(err).To(HaveOccurred())
224+
g.Expect(err.Error()).To(ContainSubstring("neither Flavor nor FlavorID is set"))
225+
}
226+
227+
func TestOpenStackMachineConversion_FlavorName(t *testing.T) {
142228
g := NewWithT(t)
143229

144230
src := &OpenStackMachine{
@@ -180,10 +266,15 @@ func TestOpenStackMachineConversion(t *testing.T) {
180266

181267
// Verify basic fields
182268
g.Expect(dst.Name).To(Equal("test-machine"))
183-
g.Expect(dst.Spec.Flavor).To(Equal(ptr.To("m1.small")))
184269
g.Expect(dst.Spec.SSHKeyName).To(Equal("test-key"))
185270
g.Expect(ptr.Deref((*string)(dst.Spec.Image.Filter.Name), "")).To(Equal("ubuntu-22.04"))
186271

272+
// Verify flavor mapping (name -> FlavorParam.Filter.Name)
273+
g.Expect(dst.Spec.Flavor.ID).To(BeNil())
274+
g.Expect(dst.Spec.Flavor.Filter).NotTo(BeNil())
275+
g.Expect(dst.Spec.Flavor.Filter.Name).NotTo(BeNil())
276+
g.Expect(*dst.Spec.Flavor.Filter.Name).To(Equal("m1.small"))
277+
187278
// Verify status fields including Initialization and InstanceID
188279
g.Expect(dst.Status.Initialization).NotTo(BeNil())
189280
g.Expect(dst.Status.Initialization.Provisioned).To(BeTrue())
@@ -201,13 +292,48 @@ func TestOpenStackMachineConversion(t *testing.T) {
201292
// Verify round-trip
202293
g.Expect(restored.Name).To(Equal(src.Name))
203294
g.Expect(restored.Spec.Flavor).To(Equal(src.Spec.Flavor))
295+
g.Expect(restored.Spec.FlavorID).To(BeNil())
204296
g.Expect(restored.Spec.SSHKeyName).To(Equal("test-key"))
205297
g.Expect(restored.Status.Ready).To(BeTrue())
206298
g.Expect(restored.Status.Initialization).NotTo(BeNil())
207299
g.Expect(restored.Status.Initialization.Provisioned).To(BeTrue())
208300
g.Expect(*restored.Status.InstanceID).To(Equal("instance-12345"))
209301
}
210302

303+
func TestOpenStackMachineConversion_FlavorID(t *testing.T) {
304+
g := NewWithT(t)
305+
306+
src := &OpenStackMachine{
307+
ObjectMeta: metav1.ObjectMeta{
308+
Name: "test-machine",
309+
},
310+
Spec: OpenStackMachineSpec{
311+
FlavorID: ptr.To("uuid-123"),
312+
SSHKeyName: "test-key",
313+
Image: ImageParam{
314+
Filter: &ImageFilter{
315+
Name: ptr.To("ubuntu-22.04"),
316+
},
317+
},
318+
},
319+
}
320+
321+
dst := &infrav1.OpenStackMachine{}
322+
g.Expect(src.ConvertTo(dst)).To(Succeed())
323+
324+
// Expect ID chosen, Filter nil
325+
g.Expect(dst.Spec.Flavor.ID).NotTo(BeNil())
326+
g.Expect(*dst.Spec.Flavor.ID).To(Equal("uuid-123"))
327+
g.Expect(dst.Spec.Flavor.Filter).To(BeNil())
328+
329+
// Round-trip back: expect FlavorID set, Flavor nil
330+
restored := &OpenStackMachine{}
331+
g.Expect(restored.ConvertFrom(dst)).To(Succeed())
332+
333+
g.Expect(restored.Spec.FlavorID).To(Equal(src.Spec.FlavorID))
334+
g.Expect(restored.Spec.Flavor).To(BeNil())
335+
}
336+
211337
func TestOpenStackClusterTemplateConversion(t *testing.T) {
212338
g := NewWithT(t)
213339

@@ -228,6 +354,12 @@ func TestOpenStackClusterTemplateConversion(t *testing.T) {
228354
CIDR: "10.0.0.0/16",
229355
},
230356
},
357+
Bastion: &Bastion{
358+
Enabled: ptr.To(true),
359+
Spec: &OpenStackMachineSpec{
360+
Flavor: ptr.To("m1.small"),
361+
},
362+
},
231363
},
232364
},
233365
},
@@ -242,6 +374,12 @@ func TestOpenStackClusterTemplateConversion(t *testing.T) {
242374
g.Expect(dst.Spec.Template.Spec.IdentityRef.Name).To(Equal("cloud-config"))
243375
g.Expect(dst.Spec.Template.Spec.ManagedSubnets).To(HaveLen(1))
244376

377+
// Verify flavor mapping (name -> FlavorParam.Filter.Name)
378+
g.Expect(dst.Spec.Template.Spec.Bastion.Spec.Flavor.ID).To(BeNil())
379+
g.Expect(dst.Spec.Template.Spec.Bastion.Spec.Flavor.Filter).NotTo(BeNil())
380+
g.Expect(dst.Spec.Template.Spec.Bastion.Spec.Flavor.Filter.Name).NotTo(BeNil())
381+
g.Expect(*dst.Spec.Template.Spec.Bastion.Spec.Flavor.Filter.Name).To(Equal("m1.small"))
382+
245383
// Convert back
246384
restored := &OpenStackClusterTemplate{}
247385
g.Expect(restored.ConvertFrom(dst)).To(Succeed())
@@ -262,7 +400,7 @@ func TestOpenStackMachineTemplateConversion(t *testing.T) {
262400
Spec: OpenStackMachineTemplateSpec{
263401
Template: OpenStackMachineTemplateResource{
264402
Spec: OpenStackMachineSpec{
265-
Flavor: ptr.To("m1.large"),
403+
Flavor: ptr.To("m1.small"),
266404
Image: ImageParam{
267405
Filter: &ImageFilter{
268406
Name: ptr.To("ubuntu-22.04"),
@@ -279,7 +417,12 @@ func TestOpenStackMachineTemplateConversion(t *testing.T) {
279417

280418
// Verify template spec
281419
g.Expect(dst.Name).To(Equal("test-machine-template"))
282-
g.Expect(dst.Spec.Template.Spec.Flavor).To(Equal(ptr.To("m1.large")))
420+
421+
// Verify flavor mapping (name -> FlavorParam.Filter.Name)
422+
g.Expect(dst.Spec.Template.Spec.Flavor.ID).To(BeNil())
423+
g.Expect(dst.Spec.Template.Spec.Flavor.Filter).NotTo(BeNil())
424+
g.Expect(dst.Spec.Template.Spec.Flavor.Filter.Name).NotTo(BeNil())
425+
g.Expect(*dst.Spec.Template.Spec.Flavor.Filter.Name).To(Equal("m1.small"))
283426

284427
// Convert back
285428
restored := &OpenStackMachineTemplate{}
@@ -513,9 +656,9 @@ func TestOpenStackMachineListConversion(t *testing.T) {
513656

514657
g.Expect(dst.Items).To(HaveLen(2))
515658
g.Expect(dst.Items[0].Name).To(Equal("machine-1"))
516-
g.Expect(dst.Items[0].Spec.Flavor).To(Equal(ptr.To("m1.small")))
659+
g.Expect(dst.Items[0].Spec.Flavor.Filter.Name).To(Equal(optional.String(ptr.To("m1.small"))))
517660
g.Expect(dst.Items[1].Name).To(Equal("machine-2"))
518-
g.Expect(dst.Items[1].Spec.Flavor).To(Equal(ptr.To("m1.large")))
661+
g.Expect(dst.Items[1].Spec.Flavor.Filter.Name).To(Equal(optional.String(ptr.To("m1.large"))))
519662

520663
// Convert back
521664
restored := &OpenStackMachineList{}
@@ -599,8 +742,7 @@ func TestOpenStackMachineTemplateListConversion(t *testing.T) {
599742

600743
g.Expect(dst.Items).To(HaveLen(1))
601744
g.Expect(dst.Items[0].Name).To(Equal("mt-1"))
602-
g.Expect(dst.Items[0].Spec.Template.Spec.Flavor).To(Equal(ptr.To("m1.xlarge")))
603-
745+
g.Expect(dst.Items[0].Spec.Template.Spec.Flavor.Filter.Name).To(Equal(optional.String(ptr.To("m1.xlarge"))))
604746
// Convert back
605747
restored := &OpenStackMachineTemplateList{}
606748
g.Expect(restored.ConvertFrom(dst)).To(Succeed())
@@ -764,7 +906,11 @@ func TestReadyFlagFromConditions(t *testing.T) {
764906
Namespace: "default",
765907
},
766908
Spec: infrav1.OpenStackMachineSpec{
767-
Flavor: ptr.To("m1.small"),
909+
Flavor: infrav1.FlavorParam{
910+
Filter: &infrav1.FlavorFilter{
911+
Name: ptr.To("m1.small"),
912+
},
913+
},
768914
Image: infrav1.ImageParam{
769915
Filter: &infrav1.ImageFilter{
770916
Name: ptr.To("ubuntu"),

0 commit comments

Comments
 (0)