Skip to content

Commit fd7c081

Browse files
vishesh92vishesh92
committed
resolve comments
1 parent 7fb9755 commit fd7c081

File tree

2 files changed

+39
-43
lines changed

2 files changed

+39
-43
lines changed

‎pkg/cloud/instance.go

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func verifyDiskoffering(csMachine *infrav1.CloudStackMachine, c *client, diskOff
208208
}
209209

210210
// CheckAccountLimits Checks the account's limit of VM, CPU & Memory
211-
func (c *client) CheckAccountLimits(fd *infrav1.CloudStackFailureDomain, offering cloudstack.ServiceOffering) error {
211+
func (c *client) CheckAccountLimits(fd *infrav1.CloudStackFailureDomain, offering *cloudstack.ServiceOffering) error {
212212
if c.user.Account.CPUAvailable != "Unlimited" {
213213
cpuAvailable, err := strconv.ParseInt(c.user.Account.CPUAvailable, 10, 0)
214214
if err == nil && int64(offering.Cpunumber) > cpuAvailable {
@@ -233,7 +233,7 @@ func (c *client) CheckAccountLimits(fd *infrav1.CloudStackFailureDomain, offerin
233233
}
234234

235235
// CheckDomainLimits Checks the domain's limit of VM, CPU & Memory
236-
func (c *client) CheckDomainLimits(fd *infrav1.CloudStackFailureDomain, offering cloudstack.ServiceOffering) error {
236+
func (c *client) CheckDomainLimits(fd *infrav1.CloudStackFailureDomain, offering *cloudstack.ServiceOffering) error {
237237
if c.user.Account.Domain.CPUAvailable != "Unlimited" {
238238
cpuAvailable, err := strconv.ParseInt(c.user.Account.Domain.CPUAvailable, 10, 0)
239239
if err == nil && int64(offering.Cpunumber) > cpuAvailable {
@@ -257,8 +257,8 @@ func (c *client) CheckDomainLimits(fd *infrav1.CloudStackFailureDomain, offering
257257
return nil
258258
}
259259

260-
// CheckNetworkLimits Checks the available IPs for a shared network
261-
func (c *client) CheckNetworkLimits(fd *infrav1.CloudStackFailureDomain) error {
260+
// CheckSharedNetworkFreeIps Checks the available IPs for a shared network
261+
func (c *client) CheckSharedNetworkFreeIps(fd *infrav1.CloudStackFailureDomain) error {
262262
if fd.Spec.Zone.Network.Type == NetworkTypeShared {
263263

264264
p := c.cs.Address.NewListPublicIpAddressesParams()
@@ -280,43 +280,48 @@ func (c *client) CheckNetworkLimits(fd *infrav1.CloudStackFailureDomain) error {
280280
}
281281
}
282282
if freeIPCount < 1 {
283-
return fmt.Errorf("no public IPs available in the shared network id: %s name: %s", fd.Spec.Zone.ID, fd.Spec.Zone.Name)
283+
return fmt.Errorf("no public IPs available in the shared network. networkid: %s network: %s zone: %s zoneid: %s", fd.Spec.Zone.Network.ID, fd.Spec.Zone.Network.Name, fd.Spec.Zone.ID, fd.Spec.Zone.Name)
284284
}
285285
}
286286
return nil
287287
}
288288

289-
// CheckLimitsAndCreateVM will check the account & domain limits and then create a
290-
// VM instance, and sets the infrastructure machine spec and status accordingly.
291-
func (c *client) CheckLimitsAndCreateVM(
292-
csMachine *infrav1.CloudStackMachine,
293-
capiMachine *clusterv1.Machine,
294-
csCluster *infrav1.CloudStackCluster,
289+
// CheckLimits will check the account & domain limits
290+
func (c *client) CheckLimits(
295291
fd *infrav1.CloudStackFailureDomain,
296-
affinity *infrav1.CloudStackAffinityGroup,
297-
userData string,
292+
offering *cloudstack.ServiceOffering,
298293
) error {
299294

300-
offering, err := c.ResolveServiceOffering(csMachine, fd.Spec.Zone.ID)
295+
err := c.CheckAccountLimits(fd, offering)
301296
if err != nil {
302297
return err
303298
}
304299

305-
templateID, err := c.ResolveTemplate(csCluster, csMachine, fd.Spec.Zone.ID)
306-
if err != nil {
307-
return err
308-
}
309-
diskOfferingID, err := c.ResolveDiskOffering(csMachine, fd.Spec.Zone.ID)
300+
err = c.CheckDomainLimits(fd, offering)
310301
if err != nil {
311302
return err
312303
}
313304

314-
err = c.CheckAccountLimits(fd, offering)
305+
return nil
306+
}
307+
308+
// DeployVM will create a VM instance,
309+
// and sets the infrastructure machine spec and status accordingly.
310+
func (c *client) DeployVM(
311+
csMachine *infrav1.CloudStackMachine,
312+
capiMachine *clusterv1.Machine,
313+
csCluster *infrav1.CloudStackCluster,
314+
fd *infrav1.CloudStackFailureDomain,
315+
affinity *infrav1.CloudStackAffinityGroup,
316+
offering *cloudstack.ServiceOffering,
317+
userData string,
318+
) error {
319+
320+
templateID, err := c.ResolveTemplate(csCluster, csMachine, fd.Spec.Zone.ID)
315321
if err != nil {
316322
return err
317323
}
318-
319-
err = c.CheckDomainLimits(fd, offering)
324+
diskOfferingID, err := c.ResolveDiskOffering(csMachine, fd.Spec.Zone.ID)
320325
if err != nil {
321326
return err
322327
}
@@ -393,12 +398,21 @@ func (c *client) GetOrCreateVMInstance(
393398
return err
394399
}
395400

396-
if err := c.CheckNetworkLimits(fd); err != nil {
401+
if err := c.CheckSharedNetworkFreeIps(fd); err != nil {
402+
return err
403+
}
404+
405+
offering, err := c.ResolveServiceOffering(csMachine, fd.Spec.Zone.ID)
406+
if err != nil {
407+
return err
408+
}
409+
410+
err = c.CheckLimits(fd, &offering)
411+
if err != nil {
397412
return err
398413
}
399414

400-
// Create VM instance.
401-
if err := c.CheckLimitsAndCreateVM(csMachine, capiMachine, csCluster, fd, affinity, userData); err != nil {
415+
if err := c.DeployVM(csMachine, capiMachine, csCluster, fd, affinity, &offering, userData); err != nil {
402416
return err
403417
}
404418

‎pkg/cloud/instance_test.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,6 @@ var _ = Describe("Instance", func() {
293293
Cpunumber: 2,
294294
Memory: 1024,
295295
}, 1, nil)
296-
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
297-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
298-
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
299296
user := &cloud.User{
300297
Account: cloud.Account{
301298
Domain: cloud.Domain{
@@ -325,9 +322,6 @@ var _ = Describe("Instance", func() {
325322
Cpunumber: 2,
326323
Memory: 1024,
327324
}, 1, nil)
328-
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
329-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
330-
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
331325
user := &cloud.User{
332326
Account: cloud.Account{
333327
Domain: cloud.Domain{
@@ -357,9 +351,6 @@ var _ = Describe("Instance", func() {
357351
Cpunumber: 2,
358352
Memory: 1024,
359353
}, 1, nil)
360-
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
361-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
362-
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
363354
user := &cloud.User{
364355
Account: cloud.Account{
365356
Domain: cloud.Domain{
@@ -389,9 +380,6 @@ var _ = Describe("Instance", func() {
389380
Cpunumber: 2,
390381
Memory: 1024,
391382
}, 1, nil)
392-
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
393-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
394-
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
395383
user := &cloud.User{
396384
Account: cloud.Account{
397385
Domain: cloud.Domain{
@@ -421,9 +409,6 @@ var _ = Describe("Instance", func() {
421409
Cpunumber: 2,
422410
Memory: 1024,
423411
}, 1, nil)
424-
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
425-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
426-
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
427412
user := &cloud.User{
428413
Account: cloud.Account{
429414
Domain: cloud.Domain{
@@ -453,9 +438,6 @@ var _ = Describe("Instance", func() {
453438
Cpunumber: 2,
454439
Memory: 1024,
455440
}, 1, nil)
456-
ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil)
457-
dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil)
458-
dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil)
459441
user := &cloud.User{
460442
Account: cloud.Account{
461443
Domain: cloud.Domain{

0 commit comments

Comments
 (0)