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

Refactor Resource volume attach and documentation #89

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions ibm/service/power/ibm_pi_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const (
Attr_Pool = "pool"
Attr_PoolName = "pool_name"
Attr_Port = "port"
Attr_PowerVolumeAttachDeleting = "power_volume_attach_deleting"
Attr_PortID = "portid"
Attr_PrimaryRole = "primary_role"
Attr_ProcType = "proctype"
Expand Down Expand Up @@ -242,14 +243,15 @@ const (
Attr_VPCCRNs = "vpc_crns"
Attr_VPCEnabled = "vpc_enabled"
Attr_VirtualCoresAssigned = "virtual_cores_assigned"
Attr_VolumeGroupName = "volume_group_name"
Attr_VolumeGroups = "volume_groups"
Attr_VolumeIDs = "volume_ids"
Attr_VolumePool = "volume_pool"
Attr_VolumeSnapshots = "volume_snapshots"
Attr_Volumes = "volumes"
Attr_WWN = "wwn"
Attr_Workspaces = "workspaces"
//Attr_VolumeAttachStatus = "volume_attach_status"
Attr_VolumeGroupName = "volume_group_name"
Attr_VolumeGroups = "volume_groups"
Attr_VolumeIDs = "volume_ids"
Attr_VolumePool = "volume_pool"
Attr_VolumeSnapshots = "volume_snapshots"
Attr_Volumes = "volumes"
Attr_WWN = "wwn"
Attr_Workspaces = "workspaces"

// TODO: Second Half Cleanup, remove extra variables

Expand Down Expand Up @@ -333,12 +335,15 @@ const (
PIPlacementGroupMembers = "members"

// Volume
PIVolumeIds = "pi_volume_ids"
PIAffinityPolicy = "pi_affinity_policy"
PIAffinityVolume = "pi_affinity_volume"
PIAffinityInstance = "pi_affinity_instance"
PIAntiAffinityInstances = "pi_anti_affinity_instances"
PIAntiAffinityVolumes = "pi_anti_affinity_volumes"
PIAffinityPolicy = "pi_affinity_policy"
PIAffinityVolume = "pi_affinity_volume"
PIAffinityInstance = "pi_affinity_instance"
PIAntiAffinityInstances = "pi_anti_affinity_instances"
PIAntiAffinityVolumes = "pi_anti_affinity_volumes"
PIVolumeIds = "pi_volume_ids"
Attr_VolumeAllowableAttachStatus = "in-use"
Attr_VolumeProvisioning = "creating"
Attr_VolumeProvisioningDone = "available"

// Volume Clone
PIVolumeCloneName = "pi_volume_clone_name"
Expand Down
69 changes: 34 additions & 35 deletions ibm/service/power/resource_ibm_pi_volume_attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import (
"log"
"time"

st "github.com/IBM-Cloud/power-go-client/clients/instance"
"github.com/IBM-Cloud/power-go-client/helpers"
"github.com/IBM-Cloud/power-go-client/clients/instance"
"github.com/IBM-Cloud/power-go-client/power/client/p_cloud_volumes"
"github.com/IBM-Cloud/terraform-provider-ibm/ibm/conns"
"github.com/IBM-Cloud/terraform-provider-ibm/ibm/flex"
Expand All @@ -34,29 +33,29 @@ func ResourceIBMPIVolumeAttach() *schema.Resource {

Schema: map[string]*schema.Schema{

helpers.PICloudInstanceId: {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Arg_CloudInstanceID: {
Description: " Cloud Instance ID - This is the service_instance_id.",
ForceNew: true,
Required: true,
Type: schema.TypeString,
},

helpers.PIVolumeId: {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Arg_VolumeID: {
Description: "Id of the volume to attach. Note these volumes should have been created",
ForceNew: true,
Required: true,
Type: schema.TypeString,
},

helpers.PIInstanceId: {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Arg_PVMInstanceId: {
Description: "PI Instance Id",
ForceNew: true,
Required: true,
Type: schema.TypeString,
},

// Computed Attribute
helpers.PIVolumeAttachStatus: {
Attr_Status: {
Type: schema.TypeString,
Computed: true,
},
Expand All @@ -70,11 +69,11 @@ func resourceIBMPIVolumeAttachCreate(ctx context.Context, d *schema.ResourceData
return diag.FromErr(err)
}

volumeID := d.Get(helpers.PIVolumeId).(string)
pvmInstanceID := d.Get(helpers.PIInstanceId).(string)
cloudInstanceID := d.Get(helpers.PICloudInstanceId).(string)
volumeID := d.Get(Arg_VolumeID).(string)
pvmInstanceID := d.Get(Arg_PVMInstanceId).(string)
cloudInstanceID := d.Get(Arg_CloudInstanceID).(string)

volClient := st.NewIBMPIVolumeClient(ctx, sess, cloudInstanceID)
volClient := instance.NewIBMPIVolumeClient(ctx, sess, cloudInstanceID)
volinfo, err := volClient.Get(volumeID)
if err != nil {
return diag.FromErr(err)
Expand All @@ -89,7 +88,7 @@ func resourceIBMPIVolumeAttachCreate(ctx context.Context, d *schema.ResourceData
log.Printf("Volume State /Status is permitted and hence attaching the volume to the instance")
}

if volinfo.State == helpers.PIVolumeAllowableAttachStatus && !*volinfo.Shareable {
if volinfo.State == Attr_VolumeAllowableAttachStatus && !*volinfo.Shareable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The value of PIVolumeAllowableAttachStatus = "in-use" need to be updated

return diag.Errorf("the volume cannot be attached in the current state. The volume must be in the *available* state. No other states are permissible")
}

Expand Down Expand Up @@ -121,14 +120,14 @@ func resourceIBMPIVolumeAttachRead(ctx context.Context, d *schema.ResourceData,
}
cloudInstanceID, pvmInstanceID, volumeID := ids[0], ids[1], ids[2]

client := st.NewIBMPIVolumeClient(ctx, sess, cloudInstanceID)
client := instance.NewIBMPIVolumeClient(ctx, sess, cloudInstanceID)

vol, err := client.CheckVolumeAttach(pvmInstanceID, volumeID)
if err != nil {
return diag.FromErr(err)
}

d.Set(helpers.PIVolumeAttachStatus, vol.State)
d.Set(Attr_Status, vol.State)
return nil
}

Expand All @@ -143,7 +142,7 @@ func resourceIBMPIVolumeAttachDelete(ctx context.Context, d *schema.ResourceData
return diag.FromErr(err)
}
cloudInstanceID, pvmInstanceID, volumeID := ids[0], ids[1], ids[2]
client := st.NewIBMPIVolumeClient(ctx, sess, cloudInstanceID)
client := instance.NewIBMPIVolumeClient(ctx, sess, cloudInstanceID)

log.Printf("the id of the volume to detach is %s ", volumeID)

Expand All @@ -170,12 +169,12 @@ func resourceIBMPIVolumeAttachDelete(ctx context.Context, d *schema.ResourceData
return nil
}

func isWaitForIBMPIVolumeAttachAvailable(ctx context.Context, client *st.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string, timeout time.Duration) (interface{}, error) {
func isWaitForIBMPIVolumeAttachAvailable(ctx context.Context, client *instance.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string, timeout time.Duration) (interface{}, error) {
log.Printf("Waiting for Volume (%s) to be available for attachment", id)

stateConf := &resource.StateChangeConf{
Pending: []string{"retry", helpers.PIVolumeProvisioning},
Target: []string{helpers.PIVolumeAllowableAttachStatus},
Pending: []string{"retry", Attr_VolumeProvisioning},
Target: []string{Attr_VolumeAllowableAttachStatus},
Comment on lines +176 to +177
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to update the const to be those

	PIVolumeProvisioning     = "creating"
	PIVolumeAllowableAttachStatus     = "in-use"

Copy link
Author

Choose a reason for hiding this comment

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

changes made

Refresh: isIBMPIVolumeAttachRefreshFunc(client, id, cloudInstanceID, pvmInstanceID),
Delay: 10 * time.Second,
MinTimeout: 30 * time.Second,
Expand All @@ -185,27 +184,27 @@ func isWaitForIBMPIVolumeAttachAvailable(ctx context.Context, client *st.IBMPIVo
return stateConf.WaitForStateContext(ctx)
}

func isIBMPIVolumeAttachRefreshFunc(client *st.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string) resource.StateRefreshFunc {
func isIBMPIVolumeAttachRefreshFunc(client *instance.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
vol, err := client.Get(id)
if err != nil {
return nil, "", err
}

if vol.State == "in-use" && flex.StringContains(vol.PvmInstanceIDs, pvmInstanceID) {
return vol, helpers.PIVolumeAllowableAttachStatus, nil
return vol, Attr_VolumeAllowableAttachStatus, nil
}

return vol, helpers.PIVolumeProvisioning, nil
return vol, Attr_VolumeProvisioning, nil
}
}

func isWaitForIBMPIVolumeDetach(ctx context.Context, client *st.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string, timeout time.Duration) (interface{}, error) {
func isWaitForIBMPIVolumeDetach(ctx context.Context, client *instance.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string, timeout time.Duration) (interface{}, error) {
log.Printf("Waiting for Volume (%s) to be available after detachment", id)

stateConf := &resource.StateChangeConf{
Pending: []string{"detaching", helpers.PowerVolumeAttachDeleting},
Target: []string{helpers.PIVolumeProvisioningDone},
Pending: []string{"detaching", Attr_PowerVolumeAttachDeleting},
Target: []string{Attr_VolumeProvisioningDone},
Refresh: isIBMPIVolumeDetachRefreshFunc(client, id, cloudInstanceID, pvmInstanceID),
Delay: 10 * time.Second,
MinTimeout: 30 * time.Second,
Expand All @@ -215,15 +214,15 @@ func isWaitForIBMPIVolumeDetach(ctx context.Context, client *st.IBMPIVolumeClien
return stateConf.WaitForStateContext(ctx)
}

func isIBMPIVolumeDetachRefreshFunc(client *st.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string) resource.StateRefreshFunc {
func isIBMPIVolumeDetachRefreshFunc(client *instance.IBMPIVolumeClient, id, cloudInstanceID, pvmInstanceID string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
vol, err := client.Get(id)
if err != nil {
uErr := errors.Unwrap(err)
switch uErr.(type) {
case *p_cloud_volumes.PcloudCloudinstancesVolumesGetNotFound:
log.Printf("[DEBUG] volume does not exist while detaching %v", err)
return vol, helpers.PIVolumeProvisioningDone, nil
return vol, Attr_VolumeProvisioningDone, nil
}
return nil, "", err
}
Expand All @@ -233,7 +232,7 @@ func isIBMPIVolumeDetachRefreshFunc(client *st.IBMPIVolumeClient, id, cloudInsta
// In case of Sharable Volume it can be `in-use` state
if !flex.StringContains(vol.PvmInstanceIDs, pvmInstanceID) &&
(*vol.Shareable || (!*vol.Shareable && vol.State == "available")) {
return vol, helpers.PIVolumeProvisioningDone, nil
return vol, Attr_VolumeProvisioningDone, nil
}

return vol, "detaching", nil
Expand Down
6 changes: 2 additions & 4 deletions ibm/service/power/resource_ibm_pi_volume_attach_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ func testAccCheckIBMPIVolumeAttachConfig(name string) string {
pi_cloud_instance_id = "%[1]s"
pi_volume_id = ibm_pi_volume.power_volume.volume_id
pi_instance_id = ibm_pi_instance.power_instance.instance_id
}
`, acc.Pi_cloud_instance_id, name, acc.Pi_image, acc.Pi_network_name)
}`, acc.Pi_cloud_instance_id, name, acc.Pi_image, acc.Pi_network_name)
}

func testAccCheckIBMPIShareableVolumeAttachConfig(name string) string {
Expand Down Expand Up @@ -171,6 +170,5 @@ func testAccCheckIBMPIShareableVolumeAttachConfig(name string) string {
pi_cloud_instance_id = "%[1]s"
pi_volume_id = ibm_pi_volume.power_volume.volume_id
pi_instance_id = ibm_pi_instance.power_instance[1].instance_id
}
`, acc.Pi_cloud_instance_id, name, acc.Pi_image, acc.Pi_network_name)
}`, acc.Pi_cloud_instance_id, name, acc.Pi_image, acc.Pi_network_name)
}
3 changes: 1 addition & 2 deletions website/docs/r/pi_volume_attach.html.markdown
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---

subcategory: "Power Systems"
layout: "ibm"
page_title: "IBM: pi_volume_attach"
Expand All @@ -21,7 +20,7 @@ resource "ibm_pi_volume_attach" "testacc_volume_attach"{
}
```

**Note**
**Notes**
* Please find [supported Regions](https://cloud.ibm.com/apidocs/power-cloud#endpoint) for endpoints.
* If a Power cloud instance is provisioned at `lon04`, The provider level attributes should be as follows:
* `region` - `lon`
Expand Down
Loading