Fix deleting instances with replicant attribute set (#5176)

* Fix replicant vms not being deleted

* Update documentation

* Use flex function instead of strings

* Change placement group to be a computed value

* Update resource instance tests

* Add replicant instance test

* Fix compilation error

* Separate command from checking

* Add replicant warning

ibm/service/power/data_source_ibm_pi_instance_ip.go

@@ -35,8 +35,8 @@ func DataSourceIBMPIInstanceIP() *schema.Resource {
 			},
 			Arg_NetworkName: {
 				Description:  "The subnet that the instance belongs to.",
-				Type:         schema.TypeString,
 				Required:     true,
+				Type:         schema.TypeString,
 				ValidateFunc: validation.NoZeroValues,
 			},
 

ibm/service/power/resource_ibm_pi_instance.go

@@ -183,6 +183,7 @@ func ResourceIBMPIInstance() *schema.Resource {
 			},
 			helpers.PIPlacementGroupID: {
 				Type:        schema.TypeString,
+				Computed:    true,
 				Optional:    true,
 				Description: "Placement group ID",
 			},
@@ -404,7 +405,13 @@ func resourceIBMPIInstanceCreate(ctx context.Context, d *schema.ResourceData, me
 		instanceReadyStatus = r.(string)
 	}
 
-	d.SetId(fmt.Sprintf("%s/%s", cloudInstanceID, *(*pvmList)[0].PvmInstanceID))
+	// id is a combination of the cloud instance id and all of the pvm instance ids
+	id := cloudInstanceID
+	for _, pvm := range *pvmList {
+		id += "/" + *pvm.PvmInstanceID
+	}
+
+	d.SetId(id)
 
 	for _, s := range *pvmList {
 		if dt, ok := d.GetOk(PIInstanceDeploymentType); ok && dt.(string) == "VMNoStorage" {
@@ -418,7 +425,6 @@ func resourceIBMPIInstanceCreate(ctx context.Context, d *schema.ResourceData, me
 				return diag.FromErr(err)
 			}
 		}
-
 	}
 
 	// If Storage Pool Affinity is given as false we need to update the vm instance.
@@ -453,7 +459,6 @@ func resourceIBMPIInstanceCreate(ctx context.Context, d *schema.ResourceData, me
 	}
 
 	return resourceIBMPIInstanceRead(ctx, d, meta)
-
 }
 
 func resourceIBMPIInstanceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
@@ -462,11 +467,14 @@ func resourceIBMPIInstanceRead(ctx context.Context, d *schema.ResourceData, meta
 		return diag.FromErr(err)
 	}
 
-	cloudInstanceID, instanceID, err := splitID(d.Id())
+	idArr, err := flex.IdParts(d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
+	cloudInstanceID := idArr[0]
+	instanceID := idArr[1]
+
 	client := st.NewIBMPIInstanceClient(ctx, sess, cloudInstanceID)
 	powervmdata, err := client.Get(instanceID)
 	if err != nil {
@@ -839,20 +847,25 @@ func resourceIBMPIInstanceDelete(ctx context.Context, d *schema.ResourceData, me
 		return diag.FromErr(err)
 	}
 
-	cloudInstanceID, instanceID, err := splitID(d.Id())
+	idArr, err := flex.IdParts(d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
+	cloudInstanceID := idArr[0]
 	client := st.NewIBMPIInstanceClient(ctx, sess, cloudInstanceID)
-	err = client.Delete(instanceID)
-	if err != nil {
-		return diag.FromErr(err)
+	for _, instanceID := range idArr[1:] {
+		err = client.Delete(instanceID)
+		if err != nil {
+			return diag.FromErr(err)
+		}
 	}
 
-	_, err = isWaitForPIInstanceDeleted(ctx, client, instanceID)
-	if err != nil {
-		return diag.FromErr(err)
+	for _, instanceID := range idArr[1:] {
+		_, err = isWaitForPIInstanceDeleted(ctx, client, instanceID)
+		if err != nil {
+			return diag.FromErr(err)
+		}
 	}
 
 	d.SetId("")

ibm/service/power/resource_ibm_pi_instance_test.go

@@ -12,6 +12,7 @@ import (
 
 	acc "github.com/IBM-Cloud/terraform-provider-ibm/ibm/acctest"
 	"github.com/IBM-Cloud/terraform-provider-ibm/ibm/conns"
+	"github.com/IBM-Cloud/terraform-provider-ibm/ibm/flex"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -85,7 +86,7 @@ func testAccCheckIBMPIInstanceDeploymentTypeConfig(name, instanceHealthStatus, e
 		pi_instance_name      = "%[2]s"
 		pi_proc_type          = "dedicated"
 		pi_image_id           = data.ibm_pi_image.power_image.id
-		pi_key_pair_name      = ibm_pi_key.key.key_id
+		pi_key_pair_name      = ibm_pi_key.key.name
 		pi_sys_type           = "%[7]s"
 		pi_cloud_instance_id  = "%[1]s"
 		pi_storage_type 	  = "%[8]s"
@@ -157,17 +158,17 @@ func testAccIBMPIInstanceNetworkConfig(name, privateNetIP string) string {
 		pi_processors         = "0.25"
 		pi_instance_name      = "%[2]s"
 		pi_proc_type          = "shared"
-		pi_image_id           = "f4501cad-d0f4-4517-9eea-85402309d90d"
-		pi_key_pair_name      = ibm_pi_key.key.key_id
+		pi_image_id           = "%[4]s"
+		pi_key_pair_name      = ibm_pi_key.key.name
 		pi_sys_type           = "e980"
 		pi_storage_type 	  = "tier3"
 		pi_cloud_instance_id  = "%[1]s"
 		pi_network {
-			network_id = resource.ibm_pi_network.power_networks.id
+			network_id = resource.ibm_pi_network.power_networks.network_id
 			ip_address = "%[3]s"
 		}
 	}
-	`, acc.Pi_cloud_instance_id, name, privateNetIP)
+	`, acc.Pi_cloud_instance_id, name, privateNetIP, acc.Pi_image)
 }
 
 func testAccIBMPIInstanceVTLConfig(name string) string {
@@ -191,7 +192,7 @@ func testAccIBMPIInstanceVTLConfig(name string) string {
 		pi_license_repository_capacity = "3"
 		pi_proc_type          = "shared"
 		pi_image_id           = "%[3]s"
-		pi_key_pair_name      = ibm_pi_key.vtl_key.key_id
+		pi_key_pair_name      = ibm_pi_key.vtl_key.name
 		pi_sys_type           = "s922"
 		pi_cloud_instance_id  = "%[1]s"
 		pi_storage_type 	  = "tier1"
@@ -203,6 +204,27 @@ func testAccIBMPIInstanceVTLConfig(name string) string {
 	`, acc.Pi_cloud_instance_id, name, acc.Pi_image)
 }
 
+func testAccCheckIBMPIInstanceReplicantConfig(name string) string {
+	return fmt.Sprintf(`
+	resource "ibm_pi_instance" "power_instance" {
+		pi_cloud_instance_id = "%[1]s"
+		pi_memory            = "2"
+		pi_processors        = "1"
+		pi_instance_name     = "%[2]s"
+		pi_proc_type         = "shared"
+		pi_image_id          = "%[3]s"
+		pi_sys_type          = "s922"
+		pi_volume_ids        = ["%[5]s"]
+		pi_network {
+		  network_id = "%[4]s"
+		}
+		pi_replicants         = 3
+		pi_replication_policy = "affinity"
+		pi_replication_scheme = "suffix"
+	  }
+	`, acc.Pi_cloud_instance_id, name, acc.Pi_image, acc.Pi_network_name, acc.Pi_volume_name)
+}
+
 func testAccCheckIBMPIInstanceDestroy(s *terraform.State) error {
 	sess, err := acc.TestAccProvider.Meta().(conns.ClientSession).IBMPISession()
 	if err != nil {
@@ -213,19 +235,25 @@ func testAccCheckIBMPIInstanceDestroy(s *terraform.State) error {
 		if rs.Type != "ibm_pi_instance" {
 			continue
 		}
-		cloudInstanceID, instanceID, err := splitID(rs.Primary.ID)
-		if err == nil {
+
+		idArr, err := flex.IdParts(rs.Primary.ID)
+		if err != nil {
 			return err
 		}
-		client := st.NewIBMPIInstanceClient(context.Background(), sess, cloudInstanceID)
-		_, err = client.Get(instanceID)
-		if err == nil {
-			return fmt.Errorf("PI Instance still exists: %s", rs.Primary.ID)
+
+		cloudInstanceID := idArr[0]
+		for _, instanceID := range idArr[1:] {
+			client := st.NewIBMPIInstanceClient(context.Background(), sess, cloudInstanceID)
+			_, err = client.Get(instanceID)
+			if err == nil {
+				return fmt.Errorf("PI Instance still exists: %s", rs.Primary.ID)
+			}
 		}
 	}
 
 	return nil
 }
+
 func testAccCheckIBMPIInstanceExists(n string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -241,15 +269,18 @@ func testAccCheckIBMPIInstanceExists(n string) resource.TestCheckFunc {
 			return err
 		}
 
-		cloudInstanceID, instanceID, err := splitID(rs.Primary.ID)
-		if err == nil {
+		idArr, err := flex.IdParts(rs.Primary.ID)
+		if err != nil {
 			return err
 		}
-		client := st.NewIBMPIInstanceClient(context.Background(), sess, cloudInstanceID)
 
-		_, err = client.Get(instanceID)
-		if err != nil {
-			return err
+		cloudInstanceID := idArr[0]
+		for _, instanceID := range idArr[1:] {
+			client := st.NewIBMPIInstanceClient(context.Background(), sess, cloudInstanceID)
+			_, err = client.Get(instanceID)
+			if err != nil {
+				return err
+			}
 		}
 
 		return nil
@@ -328,6 +359,28 @@ func TestAccIBMPIInstanceIBMiLicense(t *testing.T) {
 	})
 }
 
+func TestAccIBMPIInstanceReplicant(t *testing.T) {
+	instanceRes := "ibm_pi_instance.power_instance"
+	name := fmt.Sprintf("tf-pi-instance-%d", acctest.RandIntRange(10, 100))
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { acc.TestAccPreCheck(t) },
+		Providers:    acc.TestAccProviders,
+		CheckDestroy: testAccCheckIBMPIInstanceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCheckIBMPIInstanceReplicantConfig(name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckIBMPIInstanceExists(instanceRes),
+					resource.TestCheckResourceAttr(instanceRes, "pi_replicants", "3"),
+					resource.TestCheckResourceAttr(instanceRes, "pi_replication_policy", "affinity"),
+					resource.TestCheckResourceAttr(instanceRes, "pi_replication_scheme", "suffix"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
 func TestAccIBMPIInstanceNetwork(t *testing.T) {
 	instanceRes := "ibm_pi_instance.power_instance"
 	name := fmt.Sprintf("tf-pi-instance-%d", acctest.RandIntRange(10, 100))
@@ -467,7 +520,7 @@ func testAccIBMPIInstanceMixedStorage(name string) string {
 		pi_instance_name         = "%[2]s"
 		pi_proc_type             = "shared"
 		pi_image_id              = "ca4ea55f-b329-4cf5-bdce-d2f38cfc6da3"
-		pi_key_pair_name         = ibm_pi_key.key.key_id
+		pi_key_pair_name         = ibm_pi_key.key.name
 		pi_sys_type              = "s922"
 		pi_storage_type          = "tier1"
 		pi_storage_pool_affinity = false
@@ -609,7 +662,7 @@ func testAccCheckIBMPIStoppedInstanceConfigUpdate(name, instanceHealthStatus, pr
 			network_id = data.ibm_pi_network.power_networks.id
 		}
 	}
-	resource "ibm_pi_instance_action" "example" {
+	resource "ibm_pi_instance_action" "power_instance_action" {
   		pi_cloud_instance_id = "%[1]s"
   		pi_instance_id       = ibm_pi_instance.power_instance.instance_id
   		pi_action            = "%[8]s"

website/docs/r/pi_instance.html.markdown

@@ -1,5 +1,4 @@
 ---
-
 subcategory: "Power Systems"
 layout: "ibm"
 page_title: "IBM: pi_instance"
@@ -31,14 +30,15 @@ resource "ibm_pi_instance" "test-instance" {
 }
 ```
 
+~> **WARNING:** Updating a ibm_pi_instance resource with `pi_replicants` set does not update replicant vms!
+
 **Note**
 * 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`
   * `zone` - `lon04`
 
-  Example usage:
-
+Example usage:  
   ```terraform
     provider "ibm" {
       region    =   "lon"
@@ -113,7 +113,7 @@ In addition to all argument reference list, you can access the following attribu
 
 - `health_status` - (String) The health status of the VM.
 - `ibmi_rds` - (Boolean) IBM i Rational Dev Studio.
-- `id` - (String) The unique identifier of the instance. The ID is composed of `<power_instance_id>/<instance_id>`.
+- `id` - (String) The unique identifier of the instance. The ID is composed of `<cloud_instance_id>/<instance_id_1>/.../<instance_id_n>`.
 - `instance_id` - (String) The unique identifier of the instance. 
 - `max_processors`- (Float) The maximum number of processors that can be allocated to the instance with shutting down or rebooting the `LPAR`.
 - `max_virtual_cores` - (Integer) The maximum number of virtual cores.
@@ -135,10 +135,10 @@ In addition to all argument reference list, you can access the following attribu
 - `status` - (String) The status of the instance.
 ## Import
 
-The `ibm_pi_instance` can be imported using `power_instance_id` and `instance_id`.
+The `ibm_pi_instance` can be imported using `cloud_instance_id` and `instance_id`.
 
 **Example**
 
 ```
 $ terraform import ibm_pi_instance.example d7bec597-4726-451f-8a63-e62e6f19c32c/cea6651a-bc0a-4438-9f8a-a0770b112ebb
-```
+```