Skip to content

Commit cefafbe

Browse files
wanyufewanyufe
authored
Add zoneID parameter when retrieving service/disk offering (#208)
* Add zoneID parameter when retrieving service offering * Add zoneId as additional parameter when retrieving diskOffering by name * Update error message to include zoneID when resolving diskOffering
1 parent 8468d84 commit cefafbe

File tree

2 files changed

+38
-38
lines changed

2 files changed

+38
-38
lines changed

‎pkg/cloud/instance.go

+14-14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2022 The Kubernetes Authors.
2+
Copyright 2023 The Kubernetes Authors.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -84,7 +84,7 @@ func (c *client) ResolveVMInstanceDetails(csMachine *infrav1.CloudStackMachine)
8484
return errors.New("no match found")
8585
}
8686

87-
func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine) (offeringID string, retErr error) {
87+
func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine, zoneID string) (offeringID string, retErr error) {
8888
if len(csMachine.Spec.Offering.ID) > 0 {
8989
csOffering, count, err := c.cs.ServiceOffering.GetServiceOfferingByID(csMachine.Spec.Offering.ID)
9090
if err != nil {
@@ -102,14 +102,14 @@ func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine) (o
102102
}
103103
return csMachine.Spec.Offering.ID, nil
104104
}
105-
offeringID, count, err := c.cs.ServiceOffering.GetServiceOfferingID(csMachine.Spec.Offering.Name)
105+
offeringID, count, err := c.cs.ServiceOffering.GetServiceOfferingID(csMachine.Spec.Offering.Name, cloudstack.WithZone(zoneID))
106106
if err != nil {
107107
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
108108
return "", multierror.Append(retErr, errors.Wrapf(
109-
err, "could not get Service Offering ID from %s", csMachine.Spec.Offering.Name))
109+
err, "could not get Service Offering ID from %s in zone %s", csMachine.Spec.Offering.Name, zoneID))
110110
} else if count != 1 {
111111
return "", multierror.Append(retErr, errors.Errorf(
112-
"expected 1 Service Offering with name %s, but got %d", csMachine.Spec.Offering.Name, count))
112+
"expected 1 Service Offering with name %s in zone %s, but got %d", csMachine.Spec.Offering.Name, zoneID, count))
113113
}
114114
return offeringID, nil
115115
}
@@ -151,25 +151,25 @@ func (c *client) ResolveTemplate(
151151
// ResolveDiskOffering Retrieves diskOffering by using disk offering ID if ID is provided and confirm returned
152152
// disk offering name matches name provided in spec.
153153
// If disk offering ID is not provided, the disk offering name is used to retrieve disk offering ID.
154-
func (c *client) ResolveDiskOffering(csMachine *infrav1.CloudStackMachine) (diskOfferingID string, retErr error) {
154+
func (c *client) ResolveDiskOffering(csMachine *infrav1.CloudStackMachine, zoneID string) (diskOfferingID string, retErr error) {
155155
diskOfferingID = csMachine.Spec.DiskOffering.ID
156156
if len(csMachine.Spec.DiskOffering.Name) > 0 {
157-
diskID, count, err := c.cs.DiskOffering.GetDiskOfferingID(csMachine.Spec.DiskOffering.Name)
157+
diskID, count, err := c.cs.DiskOffering.GetDiskOfferingID(csMachine.Spec.DiskOffering.Name, cloudstack.WithZone(zoneID))
158158
if err != nil {
159159
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
160160
return "", multierror.Append(retErr, errors.Wrapf(
161161
err, "could not get DiskOffering ID from %s", csMachine.Spec.DiskOffering.Name))
162162
} else if count != 1 {
163163
return "", multierror.Append(retErr, errors.Errorf(
164-
"expected 1 DiskOffering with name %s, but got %d", csMachine.Spec.DiskOffering.Name, count))
164+
"expected 1 DiskOffering with name %s in zone %s, but got %d", csMachine.Spec.DiskOffering.Name, zoneID, count))
165165
} else if len(csMachine.Spec.DiskOffering.ID) > 0 && diskID != csMachine.Spec.DiskOffering.ID {
166166
return "", multierror.Append(retErr, errors.Errorf(
167-
"diskOffering ID %s does not match ID %s returned using name %s",
168-
csMachine.Spec.DiskOffering.ID, diskID, csMachine.Spec.DiskOffering.Name))
167+
"diskOffering ID %s does not match ID %s returned using name %s in zone %s",
168+
csMachine.Spec.DiskOffering.ID, diskID, csMachine.Spec.DiskOffering.Name, zoneID))
169169
} else if len(diskID) == 0 {
170170
return "", multierror.Append(retErr, errors.Errorf(
171-
"empty diskOffering ID %s returned using name %s",
172-
diskID, csMachine.Spec.DiskOffering.Name))
171+
"empty diskOffering ID %s returned using name %s in zone %s",
172+
diskID, csMachine.Spec.DiskOffering.Name, zoneID))
173173
}
174174
diskOfferingID = diskID
175175
}
@@ -221,15 +221,15 @@ func (c *client) GetOrCreateVMInstance(
221221
return err
222222
}
223223

224-
offeringID, err := c.ResolveServiceOffering(csMachine)
224+
offeringID, err := c.ResolveServiceOffering(csMachine, fd.Spec.Zone.ID)
225225
if err != nil {
226226
return err
227227
}
228228
templateID, err := c.ResolveTemplate(csCluster, csMachine, fd.Spec.Zone.ID)
229229
if err != nil {
230230
return err
231231
}
232-
diskOfferingID, err := c.ResolveDiskOffering(csMachine)
232+
diskOfferingID, err := c.ResolveDiskOffering(csMachine, fd.Spec.Zone.ID)
233233
if err != nil {
234234
return err
235235
}

‎pkg/cloud/instance_test.go

+24-24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2022 The Kubernetes Authors.
2+
Copyright 2023 The Kubernetes Authors.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -143,23 +143,23 @@ var _ = Describe("Instance", func() {
143143

144144
It("returns errors occurring while fetching service offering information", func() {
145145
expectVMNotFound()
146-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).Return("", -1, unknownError)
146+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).Return("", -1, unknownError)
147147
Ω(client.GetOrCreateVMInstance(
148148
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).
149149
ShouldNot(Succeed())
150150
})
151151

152152
It("returns errors if more than one service offering found", func() {
153153
expectVMNotFound()
154-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).Return("", 2, nil)
154+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).Return("", 2, nil)
155155
Ω(client.GetOrCreateVMInstance(
156156
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).
157157
ShouldNot(Succeed())
158158
})
159159

160160
It("returns errors while fetching template", func() {
161161
expectVMNotFound()
162-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
162+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
163163
Return(dummies.CSMachine1.Spec.Offering.ID, 1, nil)
164164
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).
165165
Return("", -1, unknownError)
@@ -170,7 +170,7 @@ var _ = Describe("Instance", func() {
170170

171171
It("returns errors when more than one template found", func() {
172172
expectVMNotFound()
173-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
173+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
174174
Return(dummies.CSMachine1.Spec.Offering.ID, 1, nil)
175175
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return("", 2, nil)
176176
Ω(client.GetOrCreateVMInstance(
@@ -180,21 +180,21 @@ var _ = Describe("Instance", func() {
180180

181181
It("returns errors when more than one diskoffering found", func() {
182182
expectVMNotFound()
183-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
183+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
184184
Return(dummies.CSMachine1.Spec.Offering.ID, 1, nil)
185185
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
186-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 2, nil)
186+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 2, nil)
187187
Ω(client.GetOrCreateVMInstance(
188188
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).
189189
ShouldNot(Succeed())
190190
})
191191

192192
It("returns errors when fetching diskoffering", func() {
193193
expectVMNotFound()
194-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
194+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
195195
Return(dummies.CSMachine1.Spec.Offering.ID, 1, nil)
196196
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
197-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
197+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
198198
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, unknownError)
199199
Ω(client.GetOrCreateVMInstance(
200200
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).
@@ -204,10 +204,10 @@ var _ = Describe("Instance", func() {
204204
It("returns errors when disk size not zero for non-customized disk offering", func() {
205205
expectVMNotFound()
206206
dummies.CSMachine1.Spec.DiskOffering.CustomSize = 1
207-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
207+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
208208
Return(dummies.CSMachine1.Spec.Offering.ID, 1, nil)
209209
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
210-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
210+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
211211
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
212212
Ω(client.GetOrCreateVMInstance(
213213
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).
@@ -217,10 +217,10 @@ var _ = Describe("Instance", func() {
217217
It("returns errors when disk size zero for customized disk offering", func() {
218218
expectVMNotFound()
219219
dummies.CSMachine1.Spec.DiskOffering.CustomSize = 0
220-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
220+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
221221
Return(dummies.CSMachine1.Spec.Offering.ID, 1, nil)
222222
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
223-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
223+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
224224
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: true}, 1, nil)
225225
Ω(client.GetOrCreateVMInstance(
226226
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).
@@ -229,11 +229,11 @@ var _ = Describe("Instance", func() {
229229

230230
It("handles deployment errors", func() {
231231
expectVMNotFound()
232-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).
232+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).
233233
Return(offeringFakeID, 1, nil)
234234
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).
235235
Return(templateFakeID, 1, nil)
236-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).
236+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).
237237
Return(diskOfferingFakeID, 1, nil)
238238
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).
239239
Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
@@ -282,8 +282,8 @@ var _ = Describe("Instance", func() {
282282
dummies.CSMachine1.Spec.Offering.Name = "offering"
283283
dummies.CSMachine1.Spec.Template.Name = "template"
284284

285-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).Return(offeringFakeID, 1, nil)
286-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
285+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).Return(offeringFakeID, 1, nil)
286+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
287287
dos.EXPECT().GetDiskOfferingByID(dummies.CSMachine1.Spec.DiskOffering.ID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
288288
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).
289289
Return(templateFakeID, 1, nil)
@@ -298,7 +298,7 @@ var _ = Describe("Instance", func() {
298298
dummies.CSMachine1.Spec.Template.Name = "template"
299299
dummies.CSMachine1.Spec.DiskOffering = infrav1.CloudStackResourceDiskOffering{}
300300

301-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).Return(offeringFakeID, 1, nil)
301+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).Return(offeringFakeID, 1, nil)
302302
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).
303303
Return(templateFakeID, 1, nil)
304304

@@ -315,7 +315,7 @@ var _ = Describe("Instance", func() {
315315
sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: ""}, 1, nil)
316316
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).
317317
Return(templateFakeID, 1, nil)
318-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
318+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
319319
dos.EXPECT().GetDiskOfferingByID(dummies.CSMachine1.Spec.DiskOffering.ID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
320320

321321
ActionAndAssert()
@@ -328,9 +328,9 @@ var _ = Describe("Instance", func() {
328328
dummies.CSMachine1.Spec.Offering.Name = "offering"
329329
dummies.CSMachine1.Spec.Template.Name = ""
330330

331-
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).Return(offeringFakeID, 1, nil)
331+
sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).Return(offeringFakeID, 1, nil)
332332
ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, executableFilter).Return(&cloudstack.Template{Name: ""}, 1, nil)
333-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
333+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
334334
dos.EXPECT().GetDiskOfferingByID(dummies.CSMachine1.Spec.DiskOffering.ID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
335335

336336
ActionAndAssert()
@@ -345,7 +345,7 @@ var _ = Describe("Instance", func() {
345345

346346
sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).
347347
Return(&cloudstack.ServiceOffering{Name: "offering"}, 1, nil)
348-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).
348+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).
349349
Return(diskOfferingFakeID, 1, nil)
350350
dos.EXPECT().GetDiskOfferingByID(dummies.CSMachine1.Spec.DiskOffering.ID).
351351
Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
@@ -364,7 +364,7 @@ var _ = Describe("Instance", func() {
364364

365365
sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering"}, 1, nil)
366366
ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, executableFilter).Return(&cloudstack.Template{Name: "template"}, 1, nil)
367-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID, 1, nil)
367+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
368368
dos.EXPECT().GetDiskOfferingByID(dummies.CSMachine1.Spec.DiskOffering.ID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
369369

370370
ActionAndAssert()
@@ -416,7 +416,7 @@ var _ = Describe("Instance", func() {
416416

417417
sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering"}, 1, nil)
418418
ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, executableFilter).Return(&cloudstack.Template{Name: "template"}, 1, nil)
419-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name).Return(diskOfferingFakeID+"-not-match", 1, nil)
419+
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID+"-not-match", 1, nil)
420420
requiredRegexp := "diskOffering ID %s does not match ID %s returned using name %s"
421421
Ω(client.GetOrCreateVMInstance(
422422
dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).

0 commit comments

Comments
 (0)