fix(instance): avoid ForceNew if server.Image.ID matches the image label (#2097)

Author: Mia-Cross

scaleway/resource_instance_server.go

@@ -47,7 +47,6 @@ func resourceScalewayInstanceServer() *schema.Resource {
 			"image": {
 				Type:             schema.TypeString,
 				Optional:         true,
-				ForceNew:         true,
 				Description:      "The UUID or the label of the base image used by the server",
 				DiffSuppressFunc: diffSuppressFuncLocality,
 				ExactlyOneOf:     []string{"image", "root_volume.0.volume_id"},
@@ -284,6 +283,7 @@ func resourceScalewayInstanceServer() *schema.Resource {
 				"ip_id",
 			),
 			customDiffInstanceServerType,
+			customDiffInstanceServerImage,
 		),
 	}
 }
@@ -558,8 +558,6 @@ func resourceScalewayInstanceServerRead(ctx context.Context, d *schema.ResourceD
 		// Image could be empty in an import context.
 		image := expandRegionalID(d.Get("image").(string))
 		if server.Image != nil && (image.ID == "" || scwvalidation.IsUUID(image.ID)) {
-			// TODO: If image is a label, check that server.Image.ID match the label.
-			// It could be useful if the user edit the image with another tool.
 			_ = d.Set("image", newZonedID(zone, server.Image.ID).String())
 		}
 
@@ -999,7 +997,7 @@ func resourceScalewayInstanceServerDelete(ctx context.Context, d *schema.Resourc
 	}
 
 	_, err = waitForInstanceServer(ctx, instanceAPI, zone, id, d.Timeout(schema.TimeoutDelete))
-	if err != nil {
+	if err != nil && !is404Error(err) {
 		return diag.FromErr(err)
 	}
 
@@ -1097,6 +1095,55 @@ func customDiffInstanceServerType(_ context.Context, diff *schema.ResourceDiff,
 	return nil
 }
 
+func customDiffInstanceServerImage(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) error {
+	if diff.Get("image") == "" || !diff.HasChange("image") || diff.Id() == "" {
+		return nil
+	}
+
+	// We get the server to fetch the UUID of the image
+	instanceAPI, zone, id, err := instanceAPIWithZoneAndID(meta, diff.Id())
+	if err != nil {
+		return err
+	}
+	server, err := instanceAPI.GetServer(&instance.GetServerRequest{
+		Zone:     zone,
+		ServerID: id,
+	}, scw.WithContext(ctx))
+	if err != nil {
+		return err
+	}
+
+	// If 'image' field is defined by the user and server.Image is empty, we should create a new server
+	if server.Server.Image == nil {
+		return diff.ForceNew("image")
+	}
+
+	// We get the image as it is defined by the user
+	image := expandRegionalID(diff.Get("image").(string))
+	if scwvalidation.IsUUID(image.ID) {
+		if image.ID == expandZonedID(server.Server.Image.ID).ID {
+			return nil
+		}
+	}
+
+	// If image is a label, we check that server.Image.ID matches the label in case the user has edited
+	// the image with another tool.
+	marketplaceAPI := marketplace.NewAPI(meta.(*Meta).scwClient)
+	if err != nil {
+		return err
+	}
+	marketplaceImage, err := marketplaceAPI.GetLocalImage(&marketplace.GetLocalImageRequest{
+		LocalImageID: server.Server.Image.ID,
+	}, scw.WithContext(ctx))
+	if err != nil {
+		return err
+	}
+	if marketplaceImage.Label != image.ID {
+		return diff.ForceNew("image")
+	}
+	return nil
+}
+
 func resourceScalewayInstanceServerMigrate(ctx context.Context, d *schema.ResourceData, instanceAPI *instance.API, zone scw.Zone, id string) error {
 	server, err := waitForInstanceServer(ctx, instanceAPI, zone, id, d.Timeout(schema.TimeoutUpdate))
 	if err != nil {

scaleway/resource_instance_server_test.go

@@ -1404,3 +1404,133 @@ func TestAccScalewayInstanceServer_MigrateInvalidLocalVolumeSize(t *testing.T) {
 		},
 	})
 }
+
+func TestAccScalewayInstanceServer_CustomDiffImage(t *testing.T) {
+	tt := NewTestTools(t)
+	defer tt.Cleanup()
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: tt.ProviderFactories,
+		CheckDestroy:      testAccCheckScalewayInstanceServerDestroy(tt),
+		Steps: []resource.TestStep{
+			{
+				Config: `
+					resource "scaleway_instance_server" "main" {
+						image = "ubuntu_jammy"
+						type = "DEV1-S"
+						state = "stopped"
+					}
+				`,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.main"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.main", "image", "ubuntu_jammy"),
+				),
+			},
+			{
+				Config: `
+					resource "scaleway_instance_server" "main" {
+						image = "ubuntu_jammy"
+						type = "DEV1-S"
+						state = "stopped"
+					}
+					resource "scaleway_instance_server" "copy" {
+						image = "ubuntu_jammy"
+						type = "DEV1-S"
+						state = "stopped"
+					}
+				`,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.main"),
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.copy"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.main", "image", "ubuntu_jammy"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.copy", "image", "ubuntu_jammy"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "id", "scaleway_instance_server.copy", "id"),
+				),
+				ResourceName: "scaleway_instance_server.copy",
+				ImportState:  true,
+				ImportStateIdFunc: func(state *terraform.State) (string, error) {
+					return state.RootModule().Resources["scaleway_instance_server.main"].Primary.ID, nil
+				},
+				ImportStatePersist: true,
+			},
+			{
+				Config: `
+					data "scaleway_marketplace_image" "jammy" {
+						label = "ubuntu_jammy"
+					}
+					resource "scaleway_instance_server" "main" {
+						image = data.scaleway_marketplace_image.jammy.id
+						type = "DEV1-S"
+						state = "stopped"
+					}
+					resource "scaleway_instance_server" "copy" {
+						image = "ubuntu_jammy"
+						type = "DEV1-S"
+						state = "stopped"
+					}
+				`,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.main"),
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.copy"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "image", "data.scaleway_marketplace_image.jammy", "id"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "id", "scaleway_instance_server.copy", "id"),
+				),
+			},
+			{
+				RefreshState: true,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.main"),
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.copy"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "image", "data.scaleway_marketplace_image.jammy", "id"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "id", "scaleway_instance_server.copy", "id"),
+				),
+			},
+			{
+				Config: `
+					data "scaleway_marketplace_image" "focal" {
+						label = "ubuntu_focal"
+					}
+					resource "scaleway_instance_server" "main" {
+						image = data.scaleway_marketplace_image.focal.id
+						type = "DEV1-S"
+						state = "stopped"
+					}
+					resource "scaleway_instance_server" "copy" {
+						image = "ubuntu_jammy"
+						type = "DEV1-S"
+						state = "stopped"
+					}
+				`,
+				PlanOnly:           true,
+				ExpectNonEmptyPlan: true,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.main"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "image", "data.scaleway_marketplace_image.focal", "id"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.copy", "image", "ubuntu_jammy"),
+					testAccCheckScalewayInstanceServerIDsAreDifferent("scaleway_instance_server.main", "scaleway_instance_server.copy"),
+				),
+			},
+		},
+	})
+}
+
+func testAccCheckScalewayInstanceServerIDsAreDifferent(nameFirst, nameSecond string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[nameFirst]
+		if !ok {
+			return fmt.Errorf("resource was not found: %s", nameFirst)
+		}
+		idFirst := rs.Primary.ID
+
+		rs, ok = s.RootModule().Resources[nameSecond]
+		if !ok {
+			return fmt.Errorf("resource was not found: %s", nameSecond)
+		}
+		idSecond := rs.Primary.ID
+
+		if idFirst == idSecond {
+			return fmt.Errorf("IDs of both resources were equal when they should not have been (%s and %s)", nameFirst, nameSecond)
+		}
+		return nil
+	}
+}