Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi disk support #40

Merged
merged 26 commits into from
Mar 25, 2021
Merged

Multi disk support #40

merged 26 commits into from
Mar 25, 2021

Conversation

X4mp
Copy link
Contributor

@X4mp X4mp commented Mar 15, 2021

Description

This Pull Request introduces a temparary implementation to support multiple disks when creating or updating a virtual server. This is temporary until the backend/API allows to configure multiple disks natively. Until then creating a virtual server with multiple disks requires to create it with one disk and then update it with the rest. This is done automatically if disks is configured with the plan.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAnxCloudVirtualServer'
...

Release Note

Release note for CHANGELOG:

Added `disks` as possible configuration to provide more than one single disk to a `virtual server`. If provided it will always overrule the default `disk` and `disk_type` configurations.

References

None

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@paepke
Copy link
Contributor

paepke commented Mar 15, 2021

/ok-to-test sha=cde6ae1

@X4mp X4mp changed the title Multi disk support v0.2.4 Multi disk support Mar 17, 2021
Copy link
Contributor Author

@X4mp X4mp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review done in cooperation with @paepke

Makefile Outdated
@@ -6,7 +6,7 @@ HOSTNAME=hashicorp.com
NAMESPACE=anexia-it
NAME=anxcloud
BINARY=terraform-provider-${NAME}
VERSION=0.2.3
VERSION=0.2.5
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change the version in a feature branch. It should only be incremented for release.

Description: "Requested disk capacity in GB.",
},
"disk_type": {
Type: schema.TypeString,
Optional: true,
Description: "Requested disk category (limits disk performance, e.g. IOPS). Default as defined by data center.",
},
"disks": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove legacy support for disk as attribute and change the attribute name for the list to disk.

@@ -259,17 +288,20 @@ func resourceVirtualServerRead(ctx context.Context, d *schema.ResourceData, m in
diags = append(diags, diag.FromErr(err)...)
}

if len(info.DiskInfo) != 1 {
return diag.Errorf("unsupported number of disks, currently only 1 disk is allowed, got %d", len(info.DiskInfo))
var disks []vm.Disk
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

known list length. Use capacity and length to pre-create the list object.

ChangeDisks: changeDisks,
}

v := vsphere.NewAPI(c)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use vsphereAPI over just v as variable name.

@@ -244,3 +270,20 @@ func flattenVirtualServerInfo(in *info.Info) []interface{} {

return []interface{}{att}
}

func flattenVirtualServerDisks(in []vm.Disk) []interface{} {
att := []interface{}{}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

known length.

@@ -36,6 +36,32 @@ func expandVirtualServerNetworks(p []interface{}) []vm.Network {
return networks
}

func expandVirtualServerDisks(p []interface{}) []vm.Disk {
var disks []vm.Disk
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

known length.

}
if err = d.Set("disk", info.DiskInfo[0].DiskGB); err != nil {

fDisks := flattenVirtualServerDisks(disks)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a better name for fDisks.

@@ -98,6 +98,65 @@ func TestAccAnxCloudVirtualServer(t *testing.T) {
})
}

func TestAccAnxCloudVirtualServerMultiDiskScaling(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implement all testcases for cases supported by the create/update logic.

@paepke
Copy link
Contributor

paepke commented Mar 24, 2021

/ok-to-test sha=2ea429f

@paepke
Copy link
Contributor

paepke commented Mar 25, 2021

/ok-to-test sha=616d14b

Signed-off-by: Roland Urbano <[email protected]>
@paepke
Copy link
Contributor

paepke commented Mar 25, 2021

/ok-to-test sha=5a11c64

Signed-off-by: Roland Urbano <[email protected]>
@paepke
Copy link
Contributor

paepke commented Mar 25, 2021

/ok-to-test sha=f3bb0b0

@paepke paepke merged commit 9045a80 into anexia-it:main Mar 25, 2021
@X4mp X4mp mentioned this pull request Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants