fix(instance): remove volume name when using image (#1390)

changed image to optional as it should not be used when using a volume id

Author: Codelax

docs/resources/instance_server.md

@@ -137,8 +137,8 @@ The following arguments are supported:
 You find all the available types on the [pricing page](https://www.scaleway.com/en/pricing/).
 Updates to this field will recreate a new resource.
 
-- `image` - (Required) The UUID or the label of the base image used by the server. You can use [this endpoint](https://api-marketplace.scaleway.com/images?page=1&per_page=100)
-to find either the right `label` or the right local image `ID` for a given `type`.
+- `image` - (Optional) The UUID or the label of the base image used by the server. You can use [this endpoint](https://api-marketplace.scaleway.com/images?page=1&per_page=100)
+to find either the right `label` or the right local image `ID` for a given `type`. Optional when creating an instance with an existing root volume.
 
 You can check the available labels with our [CLI](https://www.scaleway.com/en/docs/compute/instances/api-cli/creating-managing-instances-with-cliv2/). ```scw marketplace image list```
 
@@ -156,7 +156,7 @@ To retrieve more information by label please use: ```scw marketplace image get l
 ~> **Important:** When updating `placement_group_id` the `state` must be set to `stopped`, otherwise it will fail.
 
 - `root_volume` - (Optional) Root [volume](https://developers.scaleway.com/en/products/instance/api/#volumes-7e8a39) attached to the server on creation.
-    - `volume_id` - (Optional) The volume ID of the root volume of the server, allows you to create server with an existing volume. If empty, will be computed to a created volume ID
+    - `volume_id` - (Optional) The volume ID of the root volume of the server, allows you to create server with an existing volume. If empty, will be computed to a created volume ID.
     - `size_in_gb` - (Required) Size of the root volume in gigabytes.
       To find the right size use [this endpoint](https://api.scaleway.com/instance/v1/zones/fr-par-1/products/servers) and
       check the `volumes_constraint.{min|max}_size` (in bytes) for your `commercial_type`.

scaleway/resource_instance_server.go

@@ -46,7 +46,7 @@ func resourceScalewayInstanceServer() *schema.Resource {
 			},
 			"image": {
 				Type:             schema.TypeString,
-				Required:         true,
+				Optional:         true,
 				ForceNew:         true,
 				Description:      "The UUID or the label of the base image used by the server",
 				DiffSuppressFunc: diffSuppressFuncLocality,
@@ -92,6 +92,11 @@ func resourceScalewayInstanceServer() *schema.Resource {
 				Description: "Root volume attached to the server on creation",
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"name": {
+							Type:        schema.TypeString,
+							Computed:    true,
+							Description: "Name of the root volume",
+						},
 						"size_in_gb": {
 							Type:        schema.TypeInt,
 							Optional:    true,
@@ -278,8 +283,8 @@ func resourceScalewayInstanceServerCreate(ctx context.Context, d *schema.Resourc
 
 	commercialType := d.Get("type").(string)
 
-	imageUUID := expandZonedID(d.Get("image")).ID
-	if !scwvalidation.IsUUID(imageUUID) {
+	imageUUID := expandID(d.Get("image"))
+	if imageUUID != "" && !scwvalidation.IsUUID(imageUUID) {
 		marketPlaceAPI := marketplace.NewAPI(meta.(*Meta).scwClient)
 		imageUUID, err = marketPlaceAPI.GetLocalImageIDByLabel(&marketplace.GetLocalImageIDByLabelRequest{
 			CommercialType: commercialType,
@@ -356,8 +361,13 @@ func resourceScalewayInstanceServerCreate(ctx context.Context, d *schema.Resourc
 		size = scw.Size(uint64(sizeInput) * gb)
 	}
 
+	rootVolumeName := newRandomName("vol")
+	if req.Image != "" {
+		rootVolumeName = ""
+	}
+
 	req.Volumes["0"] = &instance.VolumeServerTemplate{
-		Name:       newRandomName("vol"), // name is ignored by the API, any name will work here
+		Name:       rootVolumeName,
 		ID:         rootVolumeID,
 		VolumeType: instance.VolumeVolumeType(volumeType),
 		Size:       size,
@@ -583,6 +593,7 @@ func resourceScalewayInstanceServerRead(ctx context.Context, d *schema.ResourceD
 				rootVolume["delete_on_termination"] = d.Get("root_volume.0.delete_on_termination").(bool) || !rootVolumeAttributeSet
 				rootVolume["volume_type"] = volume.VolumeType
 				rootVolume["boot"] = volume.Boot
+				rootVolume["name"] = volume.Name
 
 				_ = d.Set("root_volume", []map[string]interface{}{rootVolume})
 			} else {

scaleway/resource_instance_server_test.go

@@ -76,6 +76,7 @@ func TestAccScalewayInstanceServer_Minimal1(t *testing.T) {
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.delete_on_termination", "true"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.size_in_gb", "20"),
 					resource.TestCheckResourceAttrSet("scaleway_instance_server.base", "root_volume.0.volume_id"),
+					testAccCheckScalewayInstanceServerHasNewVolume(tt, "scaleway_instance_server.base"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "enable_dynamic_ip", "false"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "tags.0", "terraform-test"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "tags.1", "scaleway_instance_server"),
@@ -98,6 +99,7 @@ func TestAccScalewayInstanceServer_Minimal1(t *testing.T) {
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.delete_on_termination", "true"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.size_in_gb", "20"),
 					resource.TestCheckResourceAttrSet("scaleway_instance_server.base", "root_volume.0.volume_id"),
+					testAccCheckScalewayInstanceServerHasNewVolume(tt, "scaleway_instance_server.base"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "tags.0", "terraform-test"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "tags.1", "scaleway_instance_server"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "tags.2", "minimal"),
@@ -198,6 +200,7 @@ func TestAccScalewayInstanceServer_RootVolume1(t *testing.T) {
 					}`,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.base"),
+					testAccCheckScalewayInstanceServerHasNewVolume(tt, "scaleway_instance_server.base"),
 				),
 			},
 		},
@@ -227,6 +230,7 @@ func TestAccScalewayInstanceServer_RootVolume_Boot(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.base"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.boot", "true"),
+					testAccCheckScalewayInstanceServerHasNewVolume(tt, "scaleway_instance_server.base"),
 				),
 			},
 			{
@@ -244,6 +248,7 @@ func TestAccScalewayInstanceServer_RootVolume_Boot(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.base"),
 					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.boot", "false"),
+					testAccCheckScalewayInstanceServerHasNewVolume(tt, "scaleway_instance_server.base"),
 				),
 			},
 		},
@@ -267,7 +272,6 @@ func TestAccScalewayInstanceServer_RootVolume_ID(t *testing.T) {
 					}
 
 					resource "scaleway_instance_server" "base" {
-						image = "ubuntu_focal"
 						type  = "DEV1-S"
 						state = "stopped"
 						root_volume {
@@ -978,6 +982,28 @@ func testAccCheckScalewayInstanceServerDestroy(tt *TestTools) resource.TestCheck
 	}
 }
 
+// testAccCheckScalewayInstanceServerHasNewVolume tests if volume name is generated by terraform
+// It is useful as volume should not be set in request when creating an instance from an image
+func testAccCheckScalewayInstanceServerHasNewVolume(tt *TestTools, n string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("resource not found: %s", n)
+		}
+
+		rootVolumeName, ok := rs.Primary.Attributes["root_volume.0.name"]
+		if !ok {
+			return fmt.Errorf("instance root_volume has no name")
+		}
+
+		if strings.HasPrefix(rootVolumeName, "tf") {
+			return fmt.Errorf("root volume name is generated by provider, should be generated by api (%s)", rootVolumeName)
+		}
+
+		return nil
+	}
+}
+
 func TestAccScalewayInstanceServer_Bootscript(t *testing.T) {
 	tt := NewTestTools(t)
 	defer tt.Cleanup()