fix TestAccServer_CustomDiffImage

Author: Mia-Cross

internal/services/instance/server_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	instanceSDK "github.com/scaleway/scaleway-sdk-go/api/instance/v1"
+	"github.com/scaleway/scaleway-sdk-go/api/marketplace/v2"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/acctest"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/locality"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/locality/zonal"
@@ -20,6 +21,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const marketplaceImageType = "instance_sbs"
+
 func TestAccServer_Minimal1(t *testing.T) {
 	tt := acctest.NewTestTools(t)
 	defer tt.Cleanup()
@@ -1391,6 +1394,8 @@ func TestAccServer_CustomDiffImage(t *testing.T) {
 	tt := acctest.NewTestTools(t)
 	defer tt.Cleanup()
 
+	var mainServerID, controlServerID string
+
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.PreCheck(t) },
 		ProtoV6ProviderFactories: tt.ProviderFactories,
@@ -1399,143 +1404,125 @@ func TestAccServer_CustomDiffImage(t *testing.T) {
 			{
 				Config: `
 					resource "scaleway_instance_server" "main" {
+						name = "main-server"
 						image = "ubuntu_jammy"
 						type = "DEV1-S"
 						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
 					}
-				`,
-				Check: resource.ComposeTestCheckFunc(
-					isServerPresent(tt, "scaleway_instance_server.main"),
-					resource.TestCheckResourceAttr("scaleway_instance_server.main", "image", "ubuntu_jammy"),
-				),
-			},
-			{
-				Config: `
-					resource "scaleway_instance_server" "main" {
+					resource "scaleway_instance_server" "control" {
+						name = "control-server"
 						image = "ubuntu_jammy"
 						type = "DEV1-S"
 						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
-					}
-					resource "scaleway_instance_server" "copy" {
-						image = "ubuntu_jammy"
-						type = "DEV1-S"
-						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
 					}
 				`,
 				Check: resource.ComposeTestCheckFunc(
 					isServerPresent(tt, "scaleway_instance_server.main"),
-					isServerPresent(tt, "scaleway_instance_server.copy"),
+					isServerPresent(tt, "scaleway_instance_server.control"),
 					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"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.control", "image", "ubuntu_jammy"),
+					acctest.CheckResourceIDPersisted("scaleway_instance_server.main", &mainServerID),
+					acctest.CheckResourceIDPersisted("scaleway_instance_server.control", &controlServerID),
 				),
-				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: `
+				Config: fmt.Sprintf(`
 					data "scaleway_marketplace_image" "jammy" {
 						label = "ubuntu_jammy"
+						image_type = "%s"
 					}
 					resource "scaleway_instance_server" "main" {
+						name = "main-server"
 						image = data.scaleway_marketplace_image.jammy.id
 						type = "DEV1-S"
 						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
 					}
-					resource "scaleway_instance_server" "copy" {
+					resource "scaleway_instance_server" "control" {
+						name = "control-server"
 						image = "ubuntu_jammy"
 						type = "DEV1-S"
 						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
 					}
-				`,
-				Check: resource.ComposeTestCheckFunc(
-					isServerPresent(tt, "scaleway_instance_server.main"),
-					isServerPresent(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,
+				`, marketplaceImageType),
 				Check: resource.ComposeTestCheckFunc(
 					isServerPresent(tt, "scaleway_instance_server.main"),
-					isServerPresent(tt, "scaleway_instance_server.copy"),
+					isServerPresent(tt, "scaleway_instance_server.control"),
 					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"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.control", "image", "ubuntu_jammy"),
+					imageIDMatchLabel(tt, "scaleway_instance_server.main", "scaleway_instance_server.control", true),
+					acctest.CheckResourceIDPersisted("scaleway_instance_server.main", &mainServerID),
+					acctest.CheckResourceIDPersisted("scaleway_instance_server.control", &controlServerID),
 				),
 			},
 			{
-				Config: `
+				Config: fmt.Sprintf(`
 					data "scaleway_marketplace_image" "focal" {
 						label = "ubuntu_focal"
+						image_type = "%s"
 					}
 					resource "scaleway_instance_server" "main" {
+						name = "main-server"
 						image = data.scaleway_marketplace_image.focal.id
 						type = "DEV1-S"
 						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
 					}
-					resource "scaleway_instance_server" "copy" {
+					resource "scaleway_instance_server" "control" {
+						name = "control-server"
 						image = "ubuntu_jammy"
 						type = "DEV1-S"
 						state = "stopped"
-						root_volume {
-							volume_type = "l_ssd" // Keep this while data.scaleway_marketplace_image does not provide a image_type filter.
-						}
 					}
-				`,
-				PlanOnly:           true,
-				ExpectNonEmptyPlan: true,
+				`, marketplaceImageType),
 				Check: resource.ComposeTestCheckFunc(
 					isServerPresent(tt, "scaleway_instance_server.main"),
+					isServerPresent(tt, "scaleway_instance_server.control"),
 					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "image", "data.scaleway_marketplace_image.focal", "id"),
-					resource.TestCheckResourceAttr("scaleway_instance_server.copy", "image", "ubuntu_jammy"),
-					serverIDsAreDifferent("scaleway_instance_server.main", "scaleway_instance_server.copy"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.control", "image", "ubuntu_jammy"),
+					imageIDMatchLabel(tt, "scaleway_instance_server.main", "scaleway_instance_server.control", false),
+					acctest.CheckResourceIDChanged("scaleway_instance_server.main", &mainServerID),
+					acctest.CheckResourceIDPersisted("scaleway_instance_server.control", &controlServerID),
 				),
 			},
 		},
 	})
 }
 
-func serverIDsAreDifferent(nameFirst, nameSecond string) resource.TestCheckFunc {
+func imageIDMatchLabel(tt *acctest.TestTools, resourceWithImageID, resourceWithImageLabel string, expectMatch bool) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
-		rs, ok := s.RootModule().Resources[nameFirst]
+		rs, ok := s.RootModule().Resources[resourceWithImageID]
 		if !ok {
-			return fmt.Errorf("resource was not found: %s", nameFirst)
+			return fmt.Errorf("resource was not found: %s", resourceWithImageID)
 		}
 
-		idFirst := rs.Primary.ID
+		zonedImageID := zonal.ExpandID(rs.Primary.Attributes["image"])
+		expectedImageID := zonedImageID.ID
+		zone := zonedImageID.Zone
+		commercialType := rs.Primary.Attributes["type"]
 
-		rs, ok = s.RootModule().Resources[nameSecond]
+		rs, ok = s.RootModule().Resources[resourceWithImageLabel]
 		if !ok {
-			return fmt.Errorf("resource was not found: %s", nameSecond)
+			return fmt.Errorf("resource was not found: %s", resourceWithImageLabel)
 		}
 
-		idSecond := rs.Primary.ID
+		imageLabel := rs.Primary.Attributes["image"]
+
+		client := meta.ExtractScwClient(tt.Meta)
+		api := marketplace.NewAPI(client)
+
+		localImageIDFromLabel, err := api.GetLocalImageByLabel(&marketplace.GetLocalImageByLabelRequest{
+			ImageLabel:     imageLabel,
+			Zone:           zone,
+			CommercialType: commercialType,
+			Type:           marketplaceImageType,
+		})
+		if err != nil {
+			return fmt.Errorf("failed to get local image by label: %w", err)
+		}
 
-		if idFirst == idSecond {
-			return fmt.Errorf("IDs of both resources were equal when they should not have been (%s and %s)", nameFirst, nameSecond)
+		if expectMatch && expectedImageID != localImageIDFromLabel.ID {
+			return fmt.Errorf("unexpected image ID for label %q: expected %s, got %s", imageLabel, expectedImageID, localImageIDFromLabel.ID)
+		} else if !expectMatch && expectedImageID == localImageIDFromLabel.ID {
+			return fmt.Errorf("images IDs match when they should not")
 		}
 
 		return nil
@@ -2200,6 +2187,7 @@ func TestAccServer_PrivateNetworkMissingPNIC(t *testing.T) {
 }
 
 func TestAccServer_AdminPasswordEncryptionSSHKeyID(t *testing.T) {
+	t.Skip("There is currently a bug when resetting the field, we should reinstate the test once the fix has been deployed")
 	tt := acctest.NewTestTools(t)
 	defer tt.Cleanup()
 
@@ -2272,6 +2260,7 @@ func TestAccServer_AdminPasswordEncryptionSSHKeyID(t *testing.T) {
 }
 
 func TestGetEndOfServiceDate(t *testing.T) {
+	t.Skip("There are currently no Instance type scheduled for EndOfService")
 	tt := acctest.NewTestTools(t)
 
 	client := meta.ExtractScwClient(tt.Meta)