Skip to content

Commit b506fc3

Browse files
wanyufewanyufe
committed
Update template and offering to explicitly specify name or id
1 parent 648ea6b commit b506fc3

10 files changed

+121
-61
lines changed

‎api/v1beta1/cloudstackmachine_types.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ type CloudStackMachineSpec struct {
3838
InstanceID *string `json:"instanceID,omitempty"`
3939

4040
// CloudStack compute offering.
41-
Offering string `json:"offering"`
41+
Offering CloudStackResourceIdentifier `json:"offering"`
4242

4343
// CloudStack template to use.
44-
Template string `json:"template"`
44+
Template CloudStackResourceIdentifier `json:"template"`
4545

4646
// CloudStack ssh key to use.
4747
// +optional
@@ -69,6 +69,16 @@ type CloudStackMachineSpec struct {
6969
IdentityRef *CloudStackIdentityReference `json:"identityRef,omitempty"`
7070
}
7171

72+
type CloudStackResourceIdentifier struct {
73+
// Cloudstack resource ID.
74+
// +optional
75+
ID string `json:"id,omitempty"`
76+
77+
// Cloudstack resource Name
78+
// +optional
79+
Name string `json:"name,omitempty"`
80+
}
81+
7282
// TODO: Review the use of this field/type.
7383
type InstanceState string
7484

‎api/v1beta1/cloudstackmachine_webhook.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ func (r *CloudStackMachine) ValidateCreate() error {
6363
errorList = append(errorList, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
6464
}
6565

66-
errorList = webhookutil.EnsureFieldExists(r.Spec.Offering, "Offering", errorList)
67-
errorList = webhookutil.EnsureFieldExists(r.Spec.Template, "Template", errorList)
66+
errorList = webhookutil.EnsureAtLeastOneFieldExists(r.Spec.Offering.ID, r.Spec.Offering.Name, "Offering", errorList)
67+
errorList = webhookutil.EnsureAtLeastOneFieldExists(r.Spec.Template.ID, r.Spec.Template.Name, "Template", errorList)
6868

6969
return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
7070
}
@@ -81,9 +81,9 @@ func (r *CloudStackMachine) ValidateUpdate(old runtime.Object) error {
8181
}
8282
oldSpec := oldMachine.Spec
8383

84-
errorList = webhookutil.EnsureStringFieldsAreEqual(r.Spec.Offering, oldSpec.Offering, "offering", errorList)
84+
errorList = webhookutil.EnsureBothFieldsAreEqual(r.Spec.Offering.ID, r.Spec.Offering.Name, oldSpec.Offering.ID, oldSpec.Offering.Name, "offering", errorList)
8585
errorList = webhookutil.EnsureStringFieldsAreEqual(r.Spec.SSHKey, oldSpec.SSHKey, "sshkey", errorList)
86-
errorList = webhookutil.EnsureStringFieldsAreEqual(r.Spec.Template, oldSpec.Template, "template", errorList)
86+
errorList = webhookutil.EnsureBothFieldsAreEqual(r.Spec.Template.ID, r.Spec.Template.Name, oldSpec.Template.ID, oldSpec.Template.Name, "template", errorList)
8787
errorList = webhookutil.EnsureStringStringMapFieldsAreEqual(&r.Spec.Details, &oldSpec.Details, "details", errorList)
8888
if r.Spec.IdentityRef != nil && oldSpec.IdentityRef != nil {
8989
errorList = webhookutil.EnsureStringFieldsAreEqual(

‎api/v1beta1/cloudstackmachine_webhook_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package v1beta1_test
1818

1919
import (
2020
"context"
21+
"github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
2122

2223
"github.com/aws/cluster-api-provider-cloudstack/test/dummies"
2324
. "github.com/onsi/ginkgo"
@@ -41,13 +42,13 @@ var _ = Describe("CloudStackMachine webhook", func() {
4142
})
4243

4344
It("should reject a CloudStackMachine with missing Offering attribute", func() {
44-
dummies.CSMachine1.Spec.Offering = ""
45+
dummies.CSMachine1.Spec.Offering = v1beta1.CloudStackResourceIdentifier{ID: "", Name: ""}
4546
Expect(k8sClient.Create(ctx, dummies.CSMachine1)).
4647
Should(MatchError(MatchRegexp(requiredRegex, "Offering")))
4748
})
4849

4950
It("should reject a CloudStackMachine with missing Template attribute", func() {
50-
dummies.CSMachine1.Spec.Template = ""
51+
dummies.CSMachine1.Spec.Template = v1beta1.CloudStackResourceIdentifier{ID: "", Name: ""}
5152
Expect(k8sClient.Create(ctx, dummies.CSMachine1)).
5253
Should(MatchError(MatchRegexp(requiredRegex, "Template")))
5354
})
@@ -65,13 +66,13 @@ var _ = Describe("CloudStackMachine webhook", func() {
6566
})
6667

6768
It("should reject VM offering updates to the CloudStackMachine", func() {
68-
dummies.CSMachine1.Spec.Offering = "ArbitraryUpdateOffering"
69+
dummies.CSMachine1.Spec.Offering = v1beta1.CloudStackResourceIdentifier{Name: "ArbitraryUpdateOffering"}
6970
Ω(k8sClient.Update(ctx, dummies.CSMachine1)).
7071
Should(MatchError(MatchRegexp(forbiddenRegex, "offering")))
7172
})
7273

7374
It("should reject VM template updates to the CloudStackMachine", func() {
74-
dummies.CSMachine1.Spec.Template = "ArbitraryUpdateTemplate"
75+
dummies.CSMachine1.Spec.Template = v1beta1.CloudStackResourceIdentifier{Name: "ArbitraryUpdateTemplate"}
7576
Ω(k8sClient.Update(ctx, dummies.CSMachine1)).
7677
Should(MatchError(MatchRegexp(forbiddenRegex, "template")))
7778
})

‎api/v1beta1/cloudstackmachinetemplate_webhook.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ func (r *CloudStackMachineTemplate) ValidateCreate() error {
7676
"AffinityGroupIDs cannot be specified when Affinity is specified as anything but `no`"))
7777
}
7878

79-
errorList = webhookutil.EnsureFieldExists(spec.Offering, "Offering", errorList)
80-
errorList = webhookutil.EnsureFieldExists(spec.Template, "Template", errorList)
79+
errorList = webhookutil.EnsureAtLeastOneFieldExists(spec.Offering.ID, spec.Offering.Name, "Offering", errorList)
80+
errorList = webhookutil.EnsureAtLeastOneFieldExists(spec.Template.ID, spec.Template.Name, "Template", errorList)
8181

8282
return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
8383
}
@@ -96,9 +96,9 @@ func (r *CloudStackMachineTemplate) ValidateUpdate(old runtime.Object) error {
9696
oldSpec := oldMachineTemplate.Spec.Spec.Spec
9797

9898
errorList := field.ErrorList(nil)
99-
errorList = webhookutil.EnsureStringFieldsAreEqual(spec.Offering, oldSpec.Offering, "offering", errorList)
99+
errorList = webhookutil.EnsureBothFieldsAreEqual(spec.Offering.ID, spec.Offering.Name, oldSpec.Offering.ID, oldSpec.Offering.Name, "offering", errorList)
100100
errorList = webhookutil.EnsureStringFieldsAreEqual(spec.SSHKey, oldSpec.SSHKey, "sshkey", errorList)
101-
errorList = webhookutil.EnsureStringFieldsAreEqual(spec.Template, oldSpec.Template, "template", errorList)
101+
errorList = webhookutil.EnsureBothFieldsAreEqual(spec.Template.ID, spec.Template.Name, oldSpec.Template.ID, oldSpec.Template.Name, "template", errorList)
102102
errorList = webhookutil.EnsureStringStringMapFieldsAreEqual(&spec.Details, &oldSpec.Details, "details", errorList)
103103
errorList = webhookutil.EnsureStringFieldsAreEqual(spec.Affinity, oldSpec.Affinity, "affinity", errorList)
104104

‎api/v1beta1/cloudstackmachinetemplate_webhook_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package v1beta1_test
1818

1919
import (
2020
"context"
21+
"github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
2122

2223
"github.com/aws/cluster-api-provider-cloudstack/test/dummies"
2324
. "github.com/onsi/ginkgo"
@@ -41,13 +42,13 @@ var _ = Describe("CloudStackMachineTemplate webhook", func() {
4142
})
4243

4344
It("Should reject a CloudStackMachineTemplate when missing the VM Offering attribute", func() {
44-
dummies.CSMachineTemplate1.Spec.Spec.Spec.Offering = ""
45+
dummies.CSMachineTemplate1.Spec.Spec.Spec.Offering = v1beta1.CloudStackResourceIdentifier{Name: "", ID: ""}
4546
Expect(k8sClient.Create(ctx, dummies.CSMachineTemplate1)).
4647
Should(MatchError(MatchRegexp(requiredRegex, "Offering")))
4748
})
4849

4950
It("Should reject a CloudStackMachineTemplate when missing the VM Template attribute", func() {
50-
dummies.CSMachineTemplate1.Spec.Spec.Spec.Template = ""
51+
dummies.CSMachineTemplate1.Spec.Spec.Spec.Template = v1beta1.CloudStackResourceIdentifier{Name: "", ID: ""}
5152
Expect(k8sClient.Create(ctx, dummies.CSMachineTemplate1)).
5253
Should(MatchError(MatchRegexp(requiredRegex, "Template")))
5354
})
@@ -65,13 +66,13 @@ var _ = Describe("CloudStackMachineTemplate webhook", func() {
6566
})
6667

6768
It("should reject VM template updates to the CloudStackMachineTemplate", func() {
68-
dummies.CSMachineTemplate1.Spec.Spec.Spec.Template = "ArbitraryUpdateTemplate"
69+
dummies.CSMachineTemplate1.Spec.Spec.Spec.Template = v1beta1.CloudStackResourceIdentifier{Name: "ArbitraryUpdateTemplate"}
6970
Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
7071
Should(MatchError(MatchRegexp(forbiddenRegex, "template")))
7172
})
7273

7374
It("should reject VM offering updates to the CloudStackMachineTemplate", func() {
74-
dummies.CSMachineTemplate1.Spec.Spec.Spec.Offering = "Offering2"
75+
dummies.CSMachineTemplate1.Spec.Spec.Spec.Offering = v1beta1.CloudStackResourceIdentifier{Name: "Offering2"}
7576
Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
7677
Should(MatchError(MatchRegexp(forbiddenRegex, "offering")))
7778
})

‎config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachines.yaml

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,14 @@ spec:
9898
type: string
9999
offering:
100100
description: CloudStack compute offering.
101-
type: string
101+
properties:
102+
id:
103+
description: Cloudstack resource ID.
104+
type: string
105+
name:
106+
description: Cloudstack resource Name
107+
type: string
108+
type: object
102109
providerID:
103110
description: 'The CS specific unique identifier. Of the form: fmt.Sprintf("cloudstack:///%s",
104111
CS Machine ID)'
@@ -108,7 +115,14 @@ spec:
108115
type: string
109116
template:
110117
description: CloudStack template to use.
111-
type: string
118+
properties:
119+
id:
120+
description: Cloudstack resource ID.
121+
type: string
122+
name:
123+
description: Cloudstack resource Name
124+
type: string
125+
type: object
112126
required:
113127
- offering
114128
- template

‎config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachinetemplates.yaml

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,14 @@ spec:
8686
type: string
8787
offering:
8888
description: CloudStack compute offering.
89-
type: string
89+
properties:
90+
id:
91+
description: Cloudstack resource ID.
92+
type: string
93+
name:
94+
description: Cloudstack resource Name
95+
type: string
96+
type: object
9097
providerID:
9198
description: 'The CS specific unique identifier. Of the form:
9299
fmt.Sprintf("cloudstack:///%s", CS Machine ID)'
@@ -96,7 +103,14 @@ spec:
96103
type: string
97104
template:
98105
description: CloudStack template to use.
99-
type: string
106+
properties:
107+
id:
108+
description: Cloudstack resource ID.
109+
type: string
110+
name:
111+
description: Cloudstack resource Name
112+
type: string
113+
type: object
100114
required:
101115
- offering
102116
- template

‎pkg/cloud/instance.go

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -81,57 +81,55 @@ func (c *client) ResolveVMInstanceDetails(csMachine *infrav1.CloudStackMachine)
8181
}
8282

8383
func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine) (offeringID string, retErr error) {
84-
offeringID, count, err := c.cs.ServiceOffering.GetServiceOfferingID(csMachine.Spec.Offering)
85-
if err != nil {
86-
retErr = multierror.Append(retErr, errors.Wrapf(
87-
err, "could not get Service Offering ID from %s", csMachine.Spec.Offering))
88-
} else if count != 1 {
89-
retErr = multierror.Append(retErr, errors.Errorf(
90-
"expected 1 Service Offering with name %s, but got %d", csMachine.Spec.Offering, count))
91-
}
92-
93-
if retErr != nil {
94-
if _, count, err := c.cs.ServiceOffering.GetServiceOfferingByID(csMachine.Spec.Offering); err != nil {
84+
if len(csMachine.Spec.Offering.ID) > 0 {
85+
_, count, err := c.cs.ServiceOffering.GetServiceOfferingByID(csMachine.Spec.Offering.ID)
86+
if err != nil {
9587
return "", multierror.Append(retErr, errors.Wrapf(
96-
err, "could not get Service Offering by ID %s", csMachine.Spec.Offering))
88+
err, "could not get Service Offering by ID %s", csMachine.Spec.Offering.ID))
9789
} else if count != 1 {
9890
return "", multierror.Append(retErr, errors.Errorf(
99-
"expected 1 Service Offering with UUID %s, but got %d", csMachine.Spec.Offering, count))
100-
} else {
101-
offeringID = csMachine.Spec.Offering
91+
"expected 1 Service Offering with UUID %s, but got %d", csMachine.Spec.Offering.ID, count))
10292
}
93+
return csMachine.Spec.Offering.ID, nil
94+
} else {
95+
offeringID, count, err := c.cs.ServiceOffering.GetServiceOfferingID(csMachine.Spec.Offering.Name)
96+
if err != nil {
97+
retErr = multierror.Append(retErr, errors.Wrapf(
98+
err, "could not get Service Offering ID from %s", csMachine.Spec.Offering.Name))
99+
} else if count != 1 {
100+
retErr = multierror.Append(retErr, errors.Errorf(
101+
"expected 1 Service Offering with name %s, but got %d", csMachine.Spec.Offering.Name, count))
102+
}
103+
return offeringID, nil
103104
}
104-
105-
return offeringID, nil
106105
}
107106

108107
func (c *client) ResolveTemplate(
109108
csCluster *infrav1.CloudStackCluster,
110109
csMachine *infrav1.CloudStackMachine,
111110
zoneID string,
112111
) (templateID string, retErr error) {
113-
templateID, count, err := c.cs.Template.GetTemplateID(csMachine.Spec.Template, "all", zoneID)
114-
if err != nil {
115-
retErr = multierror.Append(retErr, errors.Wrapf(
116-
err, "could not get Template ID from %s", csMachine.Spec.Template))
117-
} else if count != 1 {
118-
retErr = multierror.Append(retErr, errors.Errorf(
119-
"expected 1 Template with name %s, but got %d", csMachine.Spec.Template, count))
120-
}
121-
122-
if retErr != nil {
123-
if _, count, err := c.cs.Template.GetTemplateByID(csMachine.Spec.Template, "all"); err != nil {
112+
if len(csMachine.Spec.Template.ID) > 0 {
113+
_, count, err := c.cs.Template.GetTemplateByID(csMachine.Spec.Template.ID, "all")
114+
if err != nil {
124115
return "", multierror.Append(retErr, errors.Wrapf(
125-
err, "could not get Template by ID %s", csMachine.Spec.Template))
116+
err, "could not get Template by ID %s", csMachine.Spec.Template.ID))
126117
} else if count != 1 {
127118
return "", multierror.Append(retErr, errors.Errorf(
128-
"expected 1 Template with UUID %s, but got %d", csMachine.Spec.Template, count))
129-
} else {
130-
templateID = csMachine.Spec.Template
119+
"expected 1 Template with UUID %s, but got %d", csMachine.Spec.Template.ID, count))
131120
}
121+
return csMachine.Spec.Template.ID, nil
122+
} else {
123+
templateID, count, err := c.cs.Template.GetTemplateID(csMachine.Spec.Template.Name, "all", zoneID)
124+
if err != nil {
125+
retErr = multierror.Append(retErr, errors.Wrapf(
126+
err, "could not get Template ID from %s", csMachine.Spec.Template.Name))
127+
} else if count != 1 {
128+
retErr = multierror.Append(retErr, errors.Errorf(
129+
"expected 1 Template with name %s, but got %d", csMachine.Spec.Template.Name, count))
130+
}
131+
return templateID, nil
132132
}
133-
134-
return templateID, nil
135133
}
136134

137135
// GetOrCreateVMInstance CreateVMInstance will fetch or create a VM instance, and

‎pkg/webhookutil/webhook_validators.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,27 @@ func EnsureFieldExists(value string, name string, allErrs field.ErrorList) field
3131
return allErrs
3232
}
3333

34+
func EnsureAtLeastOneFieldExists(value1 string, value2 string, name string, allErrs field.ErrorList) field.ErrorList {
35+
if value1 == "" && value2 == "" {
36+
allErrs = append(allErrs, field.Required(field.NewPath("spec", name), name))
37+
}
38+
return allErrs
39+
}
40+
3441
func EnsureStringFieldsAreEqual(new string, old string, name string, allErrs field.ErrorList) field.ErrorList {
3542
if new != old {
3643
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", name), name))
3744
}
3845
return allErrs
3946
}
4047

48+
func EnsureBothFieldsAreEqual(new1 string, new2 string, old1 string, old2 string, name string, allErrs field.ErrorList) field.ErrorList {
49+
if new1 != old1 || new2 != old2 {
50+
allErrs = append(allErrs, field.Required(field.NewPath("spec", name), name))
51+
}
52+
return allErrs
53+
}
54+
4155
func EnsureStringStringMapFieldsAreEqual(new *map[string]string, old *map[string]string, name string, allErrs field.ErrorList) field.ErrorList {
4256
if old == nil && new == nil {
4357
return allErrs

‎test/dummies/vars.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,12 @@ func SetDummyCSMachineTemplateVars() {
109109
Kind: "Secret",
110110
Name: "IdentitySecret",
111111
},
112-
Template: "Template",
113-
Offering: "Offering",
112+
Template: capcv1.CloudStackResourceIdentifier{
113+
Name: "Template",
114+
},
115+
Offering: capcv1.CloudStackResourceIdentifier{
116+
Name: "Offering",
117+
},
114118
Details: map[string]string{
115119
"memoryOvercommitRatio": "1.2",
116120
},
@@ -136,9 +140,13 @@ func SetDummyCSMachineVars() {
136140
Kind: "Secret",
137141
Name: "IdentitySecret",
138142
},
139-
InstanceID: pointer.String("Instance1"),
140-
Template: "Template",
141-
Offering: "Offering",
143+
InstanceID: pointer.String("Instance1"),
144+
Template: capcv1.CloudStackResourceIdentifier{
145+
Name: "Template",
146+
},
147+
Offering: capcv1.CloudStackResourceIdentifier{
148+
Name: "Offering",
149+
},
142150
AffinityGroupIDs: []string{"41eeb6e4-946f-4a18-b543-b2184815f1e4"},
143151
Details: map[string]string{
144152
"memoryOvercommitRatio": "1.2",

0 commit comments

Comments
 (0)