Merge pull request #7061 from boumenot/pr-6773-caching-type

azure: configuration for disk caching

builder/azure/arm/config.go

@@ -109,9 +109,9 @@ type Config struct {
 	ManagedImageResourceGroupName      string `mapstructure:"managed_image_resource_group_name"`
 	ManagedImageName                   string `mapstructure:"managed_image_name"`
 	ManagedImageStorageAccountType     string `mapstructure:"managed_image_storage_account_type"`
+	managedImageStorageAccountType     compute.StorageAccountTypes
 	ManagedImageOSDiskSnapshotName     string `mapstructure:"managed_image_os_disk_snapshot_name"`
 	ManagedImageDataDiskSnapshotPrefix string `mapstructure:"managed_image_data_disk_snapshot_prefix"`
-	managedImageStorageAccountType     compute.StorageAccountTypes
 	manageImageLocation                string
 
 	// Deployment
@@ -138,6 +138,8 @@ type Config struct {
 
 	// Additional Disks
 	AdditionalDiskSize []int32 `mapstructure:"disk_additional_size"`
+	DiskCachingType    string  `mapstructure:"disk_caching_type"`
+	diskCachingType    compute.CachingTypes
 
 	// Runtime Values
 	UserName               string
@@ -471,6 +473,10 @@ func provideDefaultValues(c *Config) {
 		c.managedImageStorageAccountType = compute.StorageAccountTypesStandardLRS
 	}
 
+	if c.DiskCachingType == "" {
+		c.diskCachingType = compute.CachingTypesReadWrite
+	}
+
 	if c.ImagePublisher != "" && c.ImageVersion == "" {
 		c.ImageVersion = DefaultImageVersion
 	}
@@ -745,6 +751,17 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
 	default:
 		errs = packer.MultiErrorAppend(errs, fmt.Errorf("The managed_image_storage_account_type %q is invalid", c.ManagedImageStorageAccountType))
 	}
+
+	switch c.DiskCachingType {
+	case string(compute.CachingTypesNone):
+		c.diskCachingType = compute.CachingTypesNone
+	case string(compute.CachingTypesReadOnly):
+		c.diskCachingType = compute.CachingTypesReadOnly
+	case "", string(compute.CachingTypesReadWrite):
+		c.diskCachingType = compute.CachingTypesReadWrite
+	default:
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("The disk_caching_type %q is invalid", c.DiskCachingType))
+	}
 }
 
 func assertManagedImageName(name, setting string) (bool, error) {

builder/azure/arm/config_test.go

@@ -48,6 +48,10 @@ func TestConfigShouldProvideReasonableDefaultValues(t *testing.T) {
 	if c.managedImageStorageAccountType == "" {
 		t.Errorf("Expected 'managedImageStorageAccountType' to be populated, but it was empty!")
 	}
+
+	if c.diskCachingType == "" {
+		t.Errorf("Expected 'diskCachingType' to be populated, but it was empty!")
+	}
 }
 
 func TestConfigShouldBeAbleToOverrideDefaultedValues(t *testing.T) {
@@ -57,6 +61,7 @@ func TestConfigShouldBeAbleToOverrideDefaultedValues(t *testing.T) {
 	builderValues["vm_size"] = "override_vm_size"
 	builderValues["communicator"] = "ssh"
 	builderValues["managed_image_storage_account_type"] = "Premium_LRS"
+	builderValues["disk_caching_type"] = "None"
 
 	c, _, err := newConfig(builderValues, getPackerConfiguration())
 
@@ -87,6 +92,10 @@ func TestConfigShouldBeAbleToOverrideDefaultedValues(t *testing.T) {
 	if c.managedImageStorageAccountType != compute.StorageAccountTypesPremiumLRS {
 		t.Errorf("Expected 'managed_image_storage_account_type' to be set to 'Premium_LRS', but found %q!", c.managedImageStorageAccountType)
 	}
+
+	if c.diskCachingType != compute.CachingTypesNone {
+		t.Errorf("Expected 'disk_caching_type' to be set to 'None', but found %q!", c.diskCachingType)
+	}
 }
 
 func TestConfigShouldDefaultVMSizeToStandardA1(t *testing.T) {
@@ -1165,6 +1174,27 @@ func TestConfigShouldRejectMalformedManageImageStorageAccountTypes(t *testing.T)
 	}
 }
 
+func TestConfigShouldRejectMalformedDiskCachingType(t *testing.T) {
+	config := map[string]interface{}{
+		"custom_managed_image_resource_group_name": "ignore",
+		"custom_managed_image_name":                "ignore",
+		"location":                                 "ignore",
+		"subscription_id":                          "ignore",
+		"communicator":                             "none",
+		"managed_image_resource_group_name":        "ignore",
+		"managed_image_name":                       "ignore",
+		"disk_caching_type":                        "--invalid--",
+
+		// Does not matter for this test case, just pick one.
+		"os_type": constants.Target_Linux,
+	}
+
+	_, _, err := newConfig(config, getPackerConfiguration())
+	if err == nil {
+		t.Fatal("expected config to reject custom and platform input for a managed image build")
+	}
+}
+
 func TestConfigShouldAcceptManagedImageStorageAccountTypes(t *testing.T) {
 	config := map[string]interface{}{
 		"custom_managed_image_resource_group_name": "ignore",
@@ -1190,6 +1220,31 @@ func TestConfigShouldAcceptManagedImageStorageAccountTypes(t *testing.T) {
 	}
 }
 
+func TestConfigShouldAcceptDiskCachingTypes(t *testing.T) {
+	config := map[string]interface{}{
+		"custom_managed_image_resource_group_name": "ignore",
+		"custom_managed_image_name":                "ignore",
+		"location":                                 "ignore",
+		"subscription_id":                          "ignore",
+		"communicator":                             "none",
+		"managed_image_resource_group_name":        "ignore",
+		"managed_image_name":                       "ignore",
+
+		// Does not matter for this test case, just pick one.
+		"os_type": constants.Target_Linux,
+	}
+
+	storage_account_types := []string{"None", "ReadOnly", "ReadWrite"}
+
+	for _, x := range storage_account_types {
+		config["disk_caching_type"] = x
+		_, _, err := newConfig(config, getPackerConfiguration())
+		if err != nil {
+			t.Fatalf("expected config to accept a disk_caching_type of %q", x)
+		}
+	}
+}
+
 func TestConfigShouldRejectTempAndBuildResourceGroupName(t *testing.T) {
 	config := map[string]interface{}{
 		"capture_name_prefix":    "ignore",

builder/azure/arm/template_factory.go

@@ -59,9 +59,9 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error)
 	}
 
 	if config.ImageUrl != "" {
-		builder.SetImageUrl(config.ImageUrl, osType)
+		builder.SetImageUrl(config.ImageUrl, osType, config.diskCachingType)
 	} else if config.CustomManagedImageName != "" {
-		builder.SetManagedDiskUrl(config.customManagedImageID, config.managedImageStorageAccountType)
+		builder.SetManagedDiskUrl(config.customManagedImageID, config.managedImageStorageAccountType, config.diskCachingType)
 	} else if config.ManagedImageName != "" && config.ImagePublisher != "" {
 		imageID := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Compute/locations/%s/publishers/%s/ArtifactTypes/vmimage/offers/%s/skus/%s/versions/%s",
 			config.SubscriptionID,
@@ -71,7 +71,7 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error)
 			config.ImageSku,
 			config.ImageVersion)
 
-		builder.SetManagedMarketplaceImage(config.Location, config.ImagePublisher, config.ImageOffer, config.ImageSku, config.ImageVersion, imageID, config.managedImageStorageAccountType)
+		builder.SetManagedMarketplaceImage(config.Location, config.ImagePublisher, config.ImageOffer, config.ImageSku, config.ImageVersion, imageID, config.managedImageStorageAccountType, config.diskCachingType)
 	} else if config.SharedGallery.Subscription != "" {
 		imageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/%s/images/%s",
 			config.SharedGallery.Subscription,
@@ -83,9 +83,9 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error)
 				config.SharedGallery.ImageVersion)
 		}
 
-		builder.SetSharedGalleryImage(config.Location, imageID)
+		builder.SetSharedGalleryImage(config.Location, imageID, config.diskCachingType)
 	} else {
-		builder.SetMarketPlaceImage(config.ImagePublisher, config.ImageOffer, config.ImageSku, config.ImageVersion)
+		builder.SetMarketPlaceImage(config.ImagePublisher, config.ImageOffer, config.ImageSku, config.ImageVersion, config.diskCachingType)
 	}
 
 	if config.OSDiskSizeGB > 0 {
@@ -93,7 +93,8 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error)
 	}
 
 	if len(config.AdditionalDiskSize) > 0 {
-		builder.SetAdditionalDisks(config.AdditionalDiskSize, config.CustomManagedImageName != "" || (config.ManagedImageName != "" && config.ImagePublisher != ""))
+		isManaged := config.CustomManagedImageName != "" || (config.ManagedImageName != "" && config.ImagePublisher != "")
+		builder.SetAdditionalDisks(config.AdditionalDiskSize, isManaged, config.diskCachingType)
 	}
 
 	if config.customData != "" {

builder/azure/common/template/template_builder.go

@@ -14,7 +14,6 @@ const (
 	jsonIndent = "  "
 
 	resourceKeyVaults         = "Microsoft.KeyVault/vaults"
-	resourceManagedDisk       = "Microsoft.Compute/images"
 	resourceNetworkInterfaces = "Microsoft.Network/networkInterfaces"
 	resourcePublicIPAddresses = "Microsoft.Network/publicIPAddresses"
 	resourceVirtualMachine    = "Microsoft.Compute/virtualMachines"
@@ -101,7 +100,7 @@ func (s *TemplateBuilder) BuildWindows(keyVaultName, winRMCertificateUrl string)
 	return nil
 }
 
-func (s *TemplateBuilder) SetManagedDiskUrl(managedImageId string, storageAccountType compute.StorageAccountTypes) error {
+func (s *TemplateBuilder) SetManagedDiskUrl(managedImageId string, storageAccountType compute.StorageAccountTypes, cachingType compute.CachingTypes) error {
 	resource, err := s.getResourceByType(resourceVirtualMachine)
 	if err != nil {
 		return err
@@ -114,6 +113,7 @@ func (s *TemplateBuilder) SetManagedDiskUrl(managedImageId string, storageAccoun
 	profile.OsDisk.OsType = s.osType
 	profile.OsDisk.CreateOption = compute.DiskCreateOptionTypesFromImage
 	profile.OsDisk.Vhd = nil
+	profile.OsDisk.Caching = cachingType
 	profile.OsDisk.ManagedDisk = &compute.ManagedDiskParameters{
 		StorageAccountType: storageAccountType,
 	}
@@ -121,7 +121,7 @@ func (s *TemplateBuilder) SetManagedDiskUrl(managedImageId string, storageAccoun
 	return nil
 }
 
-func (s *TemplateBuilder) SetManagedMarketplaceImage(location, publisher, offer, sku, version, imageID string, storageAccountType compute.StorageAccountTypes) error {
+func (s *TemplateBuilder) SetManagedMarketplaceImage(location, publisher, offer, sku, version, imageID string, storageAccountType compute.StorageAccountTypes, cachingType compute.CachingTypes) error {
 	resource, err := s.getResourceByType(resourceVirtualMachine)
 	if err != nil {
 		return err
@@ -133,11 +133,11 @@ func (s *TemplateBuilder) SetManagedMarketplaceImage(location, publisher, offer,
 		Offer:     &offer,
 		Sku:       &sku,
 		Version:   &version,
-		//ID:        &imageID,
 	}
 	profile.OsDisk.OsType = s.osType
 	profile.OsDisk.CreateOption = compute.DiskCreateOptionTypesFromImage
 	profile.OsDisk.Vhd = nil
+	profile.OsDisk.Caching = cachingType
 	profile.OsDisk.ManagedDisk = &compute.ManagedDiskParameters{
 		StorageAccountType: storageAccountType,
 	}
@@ -145,7 +145,7 @@ func (s *TemplateBuilder) SetManagedMarketplaceImage(location, publisher, offer,
 	return nil
 }
 
-func (s *TemplateBuilder) SetSharedGalleryImage(location, imageID string) error {
+func (s *TemplateBuilder) SetSharedGalleryImage(location, imageID string, cachingType compute.CachingTypes) error {
 	resource, err := s.getResourceByType(resourceVirtualMachine)
 	if err != nil {
 		return err
@@ -156,17 +156,19 @@ func (s *TemplateBuilder) SetSharedGalleryImage(location, imageID string) error
 	profile.ImageReference = &compute.ImageReference{ID: &imageID}
 	profile.OsDisk.OsType = s.osType
 	profile.OsDisk.Vhd = nil
+	profile.OsDisk.Caching = cachingType
 
 	return nil
 }
 
-func (s *TemplateBuilder) SetMarketPlaceImage(publisher, offer, sku, version string) error {
+func (s *TemplateBuilder) SetMarketPlaceImage(publisher, offer, sku, version string, cachingType compute.CachingTypes) error {
 	resource, err := s.getResourceByType(resourceVirtualMachine)
 	if err != nil {
 		return err
 	}
 
 	profile := resource.Properties.StorageProfile
+	profile.OsDisk.Caching = cachingType
 	profile.ImageReference = &compute.ImageReference{
 		Publisher: to.StringPtr(publisher),
 		Offer:     to.StringPtr(offer),
@@ -177,7 +179,7 @@ func (s *TemplateBuilder) SetMarketPlaceImage(publisher, offer, sku, version str
 	return nil
 }
 
-func (s *TemplateBuilder) SetImageUrl(imageUrl string, osType compute.OperatingSystemTypes) error {
+func (s *TemplateBuilder) SetImageUrl(imageUrl string, osType compute.OperatingSystemTypes, cachingType compute.CachingTypes) error {
 	resource, err := s.getResourceByType(resourceVirtualMachine)
 	if err != nil {
 		return err
@@ -185,6 +187,8 @@ func (s *TemplateBuilder) SetImageUrl(imageUrl string, osType compute.OperatingS
 
 	profile := resource.Properties.StorageProfile
 	profile.OsDisk.OsType = osType
+	profile.OsDisk.Caching = cachingType
+
 	profile.OsDisk.Image = &compute.VirtualHardDisk{
 		URI: to.StringPtr(imageUrl),
 	}
@@ -224,7 +228,7 @@ func (s *TemplateBuilder) SetOSDiskSizeGB(diskSizeGB int32) error {
 	return nil
 }
 
-func (s *TemplateBuilder) SetAdditionalDisks(diskSizeGB []int32, isManaged bool) error {
+func (s *TemplateBuilder) SetAdditionalDisks(diskSizeGB []int32, isManaged bool, cachingType compute.CachingTypes) error {
 	resource, err := s.getResourceByType(resourceVirtualMachine)
 	if err != nil {
 		return err
@@ -233,12 +237,12 @@ func (s *TemplateBuilder) SetAdditionalDisks(diskSizeGB []int32, isManaged bool)
 	profile := resource.Properties.StorageProfile
 	dataDisks := make([]DataDiskUnion, len(diskSizeGB))
 
-	for i, additionalsize := range diskSizeGB {
-		dataDisks[i].DiskSizeGB = to.Int32Ptr(additionalsize)
+	for i, additionalSize := range diskSizeGB {
+		dataDisks[i].DiskSizeGB = to.Int32Ptr(additionalSize)
 		dataDisks[i].Lun = to.IntPtr(i)
 		dataDisks[i].Name = to.StringPtr(fmt.Sprintf("datadisk-%d", i+1))
 		dataDisks[i].CreateOption = "Empty"
-		dataDisks[i].Caching = "ReadWrite"
+		dataDisks[i].Caching = cachingType
 		if isManaged {
 			dataDisks[i].Vhd = nil
 			dataDisks[i].ManagedDisk = profile.OsDisk.ManagedDisk

builder/azure/common/template/template_builder_test.TestSharedImageGallery00.approved.json

@@ -139,7 +139,7 @@
             "id": "/subscriptions/ignore/resourceGroups/ignore/providers/Microsoft.Compute/galleries/ignore/images/ignore"
           },
           "osDisk": {
-            "caching": "ReadWrite",
+            "caching": "ReadOnly",
             "createOption": "FromImage",
             "name": "[parameters('osDiskName')]",
             "osType": "Linux"

builder/azure/common/template/template_builder_test.go

@@ -20,7 +20,7 @@ func TestBuildLinux00(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = testSubject.SetMarketPlaceImage("Canonical", "UbuntuServer", "16.04", "latest")
+	err = testSubject.SetMarketPlaceImage("Canonical", "UbuntuServer", "16.04", "latest", compute.CachingTypesReadWrite)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -48,7 +48,7 @@ func TestBuildLinux01(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux)
+	err = testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux, compute.CachingTypesReadWrite)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -72,7 +72,7 @@ func TestBuildLinux02(t *testing.T) {
 	}
 
 	testSubject.BuildLinux("--test-ssh-authorized-key--")
-	testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux)
+	testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux, compute.CachingTypesReadWrite)
 	testSubject.SetOSDiskSizeGB(100)
 
 	err = testSubject.SetVirtualNetwork("--virtual-network-resource-group--", "--virtual-network--", "--subnet-name--")
@@ -105,7 +105,7 @@ func TestBuildWindows00(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = testSubject.SetMarketPlaceImage("MicrosoftWindowsServer", "WindowsServer", "2012-R2-Datacenter", "latest")
+	err = testSubject.SetMarketPlaceImage("MicrosoftWindowsServer", "WindowsServer", "2012-R2-Datacenter", "latest", compute.CachingTypesReadWrite)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -133,12 +133,12 @@ func TestBuildWindows01(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = testSubject.SetManagedMarketplaceImage("MicrosoftWindowsServer", "WindowsServer", "2012-R2-Datacenter", "latest", "2015-1", "1", "Premium_LRS")
+	err = testSubject.SetManagedMarketplaceImage("MicrosoftWindowsServer", "WindowsServer", "2012-R2-Datacenter", "latest", "2015-1", "1", "Premium_LRS", compute.CachingTypesReadWrite)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	err = testSubject.SetAdditionalDisks([]int32{32, 64}, true)
+	err = testSubject.SetAdditionalDisks([]int32{32, 64}, true, compute.CachingTypesReadWrite)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -166,7 +166,7 @@ func TestBuildWindows02(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = testSubject.SetAdditionalDisks([]int32{32, 64}, false)
+	err = testSubject.SetAdditionalDisks([]int32{32, 64}, false, compute.CachingTypesReadWrite)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -195,7 +195,7 @@ func TestSharedImageGallery00(t *testing.T) {
 	}
 
 	imageID := "/subscriptions/ignore/resourceGroups/ignore/providers/Microsoft.Compute/galleries/ignore/images/ignore"
-	err = testSubject.SetSharedGalleryImage("westcentralus", imageID)
+	err = testSubject.SetSharedGalleryImage("westcentralus", imageID, compute.CachingTypesReadOnly)
 	if err != nil {
 		t.Fatal(err)
 	}

website/source/docs/builders/azure.html.md

@@ -190,6 +190,9 @@ Providing `temp_resource_group_name` or `location` in combination with
 -   `os_disk_size_gb` (number) Specify the size of the OS disk in GB
     (gigabytes). Values of zero or less than zero are ignored.
 
+-   `disk_caching_type` (string) Specify the disk caching type. Valid values are None, ReadOnly,
+     and ReadWrite. The default value is ReadWrite.
+
 -   `disk_additional_size` (array of integers) - The size(s) of any additional
     hard disks for the VM in gigabytes. If this is not specified then the VM
     will only contain an OS disk. The number of additional disks and maximum