fix describing snapshots issue when image_ignore_data_disks is provided

builder/alicloud/ecs/builder_acc_test.go

@@ -57,7 +57,7 @@ const testBuilderAccBasic = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_name": "packer-test-basic_{{timestamp}}"
@@ -81,7 +81,7 @@ const testBuilderAccWithDiskSettings = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_name": "packer-test-withDiskSettings_{{timestamp}}",
@@ -189,6 +189,69 @@ func checkImageDisksSettings() builderT.TestCheckFunc {
 	}
 }
 
+func TestBuilderAcc_withIgnoreDataDisks(t *testing.T) {
+	t.Parallel()
+	builderT.Test(t, builderT.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+		},
+		Builder:  &Builder{},
+		Template: testBuilderAccIgnoreDataDisks,
+		Check:    checkIgnoreDataDisks(),
+	})
+}
+
+const testBuilderAccIgnoreDataDisks = `
+{	"builders": [{
+		"type": "test",
+		"region": "cn-beijing",
+		"instance_type": "ecs.gn5-c8g1.2xlarge",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
+		"io_optimized":"true",
+		"ssh_username":"root",
+		"image_name": "packer-test-ignoreDataDisks_{{timestamp}}",
+		"image_ignore_data_disks": true
+	}]
+}`
+
+func checkIgnoreDataDisks() builderT.TestCheckFunc {
+	return func(artifacts []packer.Artifact) error {
+		if len(artifacts) > 1 {
+			return fmt.Errorf("more than 1 artifact")
+		}
+
+		// Get the actual *Artifact pointer so we can access the AMIs directly
+		artifactRaw := artifacts[0]
+		artifact, ok := artifactRaw.(*Artifact)
+		if !ok {
+			return fmt.Errorf("unknown artifact: %#v", artifactRaw)
+		}
+		imageId := artifact.AlicloudImages[defaultTestRegion]
+
+		// describe the image, get block devices with a snapshot
+		client, _ := testAliyunClient()
+
+		describeImagesRequest := ecs.CreateDescribeImagesRequest()
+		describeImagesRequest.RegionId = defaultTestRegion
+		describeImagesRequest.ImageId = imageId
+		imagesResponse, err := client.DescribeImages(describeImagesRequest)
+		if err != nil {
+			return fmt.Errorf("describe images failed due to %s", err)
+		}
+
+		if len(imagesResponse.Images.Image) == 0 {
+			return fmt.Errorf("image %s generated can not be found", imageId)
+		}
+
+		image := imagesResponse.Images.Image[0]
+		if len(image.DiskDeviceMappings.DiskDeviceMapping) != 1 {
+			return fmt.Errorf("image %s should only contain one disks", imageId)
+		}
+
+		return nil
+	}
+}
+
 func TestBuilderAcc_windows(t *testing.T) {
 	t.Parallel()
 	builderT.Test(t, builderT.TestCase{
@@ -234,7 +297,7 @@ const testBuilderAccRegionCopy = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_name": "packer-test-regionCopy_{{timestamp}}",
@@ -338,7 +401,7 @@ const testBuilderAccForceDelete = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_force_delete": "%s",
@@ -366,7 +429,7 @@ const testBuilderAccSharing = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_name": "packer-test-ECSImageSharing_{{timestamp}}",
@@ -461,7 +524,7 @@ const testBuilderAccForceDeleteSnapshot = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_force_delete_snapshots": "%s",
@@ -513,7 +576,7 @@ const testBuilderAccImageTags = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"ssh_username": "root",
 		"io_optimized":"true",
 		"image_name": "packer-test-imageTags_{{timestamp}}",
@@ -627,7 +690,7 @@ const testBuilderAccDataDiskEncrypted = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_name": "packer-test-dataDiskEncrypted_{{timestamp}}",
@@ -744,7 +807,7 @@ const testBuilderAccSystemDiskEncrypted = `
 		"type": "test",
 		"region": "cn-beijing",
 		"instance_type": "ecs.n1.tiny",
-		"source_image":"ubuntu_18_04_64_20G_alibase_20190223.vhd",
+		"source_image":"ubuntu_18_04_64_20G_alibase_20190509.vhd",
 		"io_optimized":"true",
 		"ssh_username":"root",
 		"image_name": "packer-test_{{timestamp}}",

builder/alicloud/ecs/client.go

@@ -247,6 +247,32 @@ func (c *ClientWrapper) WaitForImageStatus(regionId string, imageId string, expe
 	})
 }
 
+func (c *ClientWrapper) WaitForSnapshotStatus(regionId string, snapshotId string, expectedStatus string, timeout time.Duration) (responses.AcsResponse, error) {
+	return c.WaitForExpected(&WaitForExpectArgs{
+		RequestFunc: func() (responses.AcsResponse, error) {
+			request := ecs.CreateDescribeSnapshotsRequest()
+			request.RegionId = regionId
+			request.SnapshotIds = fmt.Sprintf("[\"%s\"]", snapshotId)
+			return c.DescribeSnapshots(request)
+		},
+		EvalFunc: func(response responses.AcsResponse, err error) WaitForExpectEvalResult {
+			if err != nil {
+				return WaitForExpectToRetry
+			}
+
+			snapshotsResponse := response.(*ecs.DescribeSnapshotsResponse)
+			snapshots := snapshotsResponse.Snapshots.Snapshot
+			for _, snapshot := range snapshots {
+				if snapshot.Status == expectedStatus {
+					return WaitForExpectSuccess
+				}
+			}
+			return WaitForExpectToRetry
+		},
+		RetryTimeout: timeout,
+	})
+}
+
 type EvalErrorType bool
 
 const (

builder/alicloud/ecs/step_create_snapshot.go

@@ -3,9 +3,9 @@ package ecs
 import (
 	"context"
 	"fmt"
+	"github.com/aliyun/alibaba-cloud-sdk-go/sdk/errors"
 	"time"
 
-	"github.com/aliyun/alibaba-cloud-sdk-go/sdk/responses"
 	"github.com/aliyun/alibaba-cloud-sdk-go/services/ecs"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
@@ -36,9 +36,6 @@ func (s *stepCreateAlicloudSnapshot) Run(ctx context.Context, state multistep.St
 		return halt(state, err, "Unable to find system disk of instance")
 	}
 
-	// Create the alicloud snapshot
-	ui.Say(fmt.Sprintf("Creating snapshot from system disk: %s", disks[0].DiskId))
-
 	createSnapshotRequest := ecs.CreateCreateSnapshotRequest()
 	createSnapshotRequest.DiskId = disks[0].DiskId
 	snapshot, err := client.CreateSnapshot(createSnapshotRequest)
@@ -46,44 +43,20 @@ func (s *stepCreateAlicloudSnapshot) Run(ctx context.Context, state multistep.St
 		return halt(state, err, "Error creating snapshot")
 	}
 
-	_, err = client.WaitForExpected(&WaitForExpectArgs{
+	// Create the alicloud snapshot
-		RequestFunc: func() (responses.AcsResponse, error) {
+	ui.Say(fmt.Sprintf("Creating snapshot from system disk %s: %s", disks[0].DiskId, snapshot.SnapshotId))
-			request := ecs.CreateDescribeSnapshotsRequest()
+
-			request.RegionId = config.AlicloudRegion
+	snapshotsResponse, err := client.WaitForSnapshotStatus(config.AlicloudRegion, snapshot.SnapshotId, SnapshotStatusAccomplished, time.Duration(s.WaitSnapshotReadyTimeout)*time.Second)
-			request.SnapshotIds = snapshot.SnapshotId
-			return client.DescribeSnapshots(request)
-		},
-		EvalFunc: func(response responses.AcsResponse, err error) WaitForExpectEvalResult {
-			if err != nil {
-				return WaitForExpectToRetry
-			}
-
-			snapshotsResponse := response.(*ecs.DescribeSnapshotsResponse)
-			snapshots := snapshotsResponse.Snapshots.Snapshot
-			for _, snapshot := range snapshots {
-				if snapshot.Status == SnapshotStatusAccomplished {
-					return WaitForExpectSuccess
-				}
-			}
-			return WaitForExpectToRetry
-		},
-		RetryTimeout: time.Duration(s.WaitSnapshotReadyTimeout) * time.Second,
-	})
-
-	if err != nil {
-		return halt(state, err, "Timeout waiting for snapshot to be created")
-	}
-
-	describeSnapshotsRequest := ecs.CreateDescribeSnapshotsRequest()
-	describeSnapshotsRequest.RegionId = config.AlicloudRegion
-	describeSnapshotsRequest.SnapshotIds = snapshot.SnapshotId
-
-	snapshotsResponse, err := client.DescribeSnapshots(describeSnapshotsRequest)
 	if err != nil {
+		_, ok := err.(errors.Error)
+		if ok {
 			return halt(state, err, "Error querying created snapshot")
 		}
 
-	snapshots := snapshotsResponse.Snapshots.Snapshot
+		return halt(state, err, "Timeout waiting for snapshot to be created")
+	}
+
+	snapshots := snapshotsResponse.(*ecs.DescribeSnapshotsResponse).Snapshots.Snapshot
 	if len(snapshots) == 0 {
 		return halt(state, err, "Unable to find created snapshot")
 	}