Addressed PR comments

This commit is contained in:
Amrita Dutta 2018-11-14 01:47:48 +00:00
parent 2d6b18e63e
commit 67342750a3
6 changed files with 110 additions and 69 deletions

View File

@ -183,20 +183,13 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
NewStepGetOSDisk(azureClient, ui), NewStepGetOSDisk(azureClient, ui),
NewStepGetAdditionalDisks(azureClient, ui), NewStepGetAdditionalDisks(azureClient, ui),
NewStepPowerOffCompute(azureClient, ui), NewStepPowerOffCompute(azureClient, ui),
} NewStepSnapshotOSDisk(azureClient, ui, b.config.isManagedImage()),
// if managed image create snapshot NewStepSnapshotDataDisks(azureClient, ui, b.config.isManagedImage()),
if b.config.isManagedImage() {
steps = append(steps,
NewStepSnapshotOSDisk(azureClient, ui),
NewStepSnapshotDataDisks(azureClient, ui),
)
}
steps = append(steps,
NewStepCaptureImage(azureClient, ui), NewStepCaptureImage(azureClient, ui),
NewStepDeleteResourceGroup(azureClient, ui), NewStepDeleteResourceGroup(azureClient, ui),
NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteOSDisk(azureClient, ui),
NewStepDeleteAdditionalDisks(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui),
) }
} else if b.config.OSType == constants.Target_Windows { } else if b.config.OSType == constants.Target_Windows {
keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string) keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string)
steps = []multistep.Step{ steps = []multistep.Step{
@ -227,21 +220,14 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
&packerCommon.StepProvision{}, &packerCommon.StepProvision{},
NewStepGetOSDisk(azureClient, ui), NewStepGetOSDisk(azureClient, ui),
NewStepGetAdditionalDisks(azureClient, ui), NewStepGetAdditionalDisks(azureClient, ui),
} NewStepSnapshotOSDisk(azureClient, ui, b.config.isManagedImage()),
// if managed image create snapshot NewStepSnapshotDataDisks(azureClient, ui, b.config.isManagedImage()),
if b.config.isManagedImage() {
steps = append(steps,
NewStepSnapshotOSDisk(azureClient, ui),
NewStepSnapshotDataDisks(azureClient, ui),
)
}
steps = append(steps,
NewStepPowerOffCompute(azureClient, ui), NewStepPowerOffCompute(azureClient, ui),
NewStepCaptureImage(azureClient, ui), NewStepCaptureImage(azureClient, ui),
NewStepDeleteResourceGroup(azureClient, ui), NewStepDeleteResourceGroup(azureClient, ui),
NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteOSDisk(azureClient, ui),
NewStepDeleteAdditionalDisks(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui),
) }
} else { } else {
return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType)
} }

View File

@ -912,6 +912,48 @@ func TestConfigShouldAcceptManagedImageOSDiskSnapshotNameAndManagedImageDataDisk
} }
} }
func TestConfigShouldRejectManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefixWithCaptureContainerName(t *testing.T) {
config := map[string]interface{}{
"image_offer": "ignore",
"image_publisher": "ignore",
"image_sku": "ignore",
"location": "ignore",
"subscription_id": "ignore",
"communicator": "none",
"capture_container_name": "ignore",
"managed_image_os_disk_snapshot_name": "ignore_ignore",
"managed_image_data_disk_snapshot_prefix": "ignore_ignore",
// 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 Managed Image build with data disk snapshot prefix and OS disk snapshot name with capture container name")
}
}
func TestConfigShouldRejectManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefixWithCaptureNamePrefix(t *testing.T) {
config := map[string]interface{}{
"image_offer": "ignore",
"image_publisher": "ignore",
"image_sku": "ignore",
"location": "ignore",
"subscription_id": "ignore",
"communicator": "none",
"capture_name_prefix": "ignore",
"managed_image_os_disk_snapshot_name": "ignore_ignore",
"managed_image_data_disk_snapshot_prefix": "ignore_ignore",
// 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 Managed Image build with data disk snapshot prefix and OS disk snapshot name with capture name prefix")
}
}
func TestConfigShouldAcceptPlatformManagedImageBuild(t *testing.T) { func TestConfigShouldAcceptPlatformManagedImageBuild(t *testing.T) {
config := map[string]interface{}{ config := map[string]interface{}{
"image_offer": "ignore", "image_offer": "ignore",

View File

@ -13,17 +13,19 @@ import (
) )
type StepSnapshotDataDisks struct { type StepSnapshotDataDisks struct {
client *AzureClient client *AzureClient
create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error
say func(message string) say func(message string)
error func(e error) error func(e error)
isManagedImage bool
} }
func NewStepSnapshotDataDisks(client *AzureClient, ui packer.Ui) *StepSnapshotDataDisks { func NewStepSnapshotDataDisks(client *AzureClient, ui packer.Ui, isManagedImage bool) *StepSnapshotDataDisks {
var step = &StepSnapshotDataDisks{ var step = &StepSnapshotDataDisks{
client: client, client: client,
say: func(message string) { ui.Say(message) }, say: func(message string) { ui.Say(message) },
error: func(e error) { ui.Error(e.Error()) }, error: func(e error) { ui.Error(e.Error()) },
isManagedImage: isManagedImage,
} }
step.create = step.createDataDiskSnapshot step.create = step.createDataDiskSnapshot
@ -70,25 +72,27 @@ func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, reso
} }
func (s *StepSnapshotDataDisks) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { func (s *StepSnapshotDataDisks) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction {
s.say("Taking snapshot of OS disk ...") if s.isManagedImage {
var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) s.say("Taking snapshot of data disk ...")
var location = stateBag.Get(constants.ArmLocation).(string)
var tags = stateBag.Get(constants.ArmTags).(map[string]*string)
var additionalDisks = stateBag.Get(constants.ArmAdditionalDiskVhds).([]string)
var dstSnapshotPrefix = stateBag.Get(constants.ArmManagedImageDataDiskSnapshotPrefix).(string)
for i, disk := range additionalDisks { var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string)
dstSnapshotName := dstSnapshotPrefix + strconv.Itoa(i) var location = stateBag.Get(constants.ArmLocation).(string)
err := s.create(ctx, resourceGroupName, disk, location, tags, dstSnapshotName) var tags = stateBag.Get(constants.ArmTags).(map[string]*string)
var additionalDisks = stateBag.Get(constants.ArmAdditionalDiskVhds).([]string)
var dstSnapshotPrefix = stateBag.Get(constants.ArmManagedImageDataDiskSnapshotPrefix).(string)
if err != nil { for i, disk := range additionalDisks {
stateBag.Put(constants.Error, err) dstSnapshotName := dstSnapshotPrefix + strconv.Itoa(i)
s.error(err) err := s.create(ctx, resourceGroupName, disk, location, tags, dstSnapshotName)
return multistep.ActionHalt if err != nil {
stateBag.Put(constants.Error, err)
s.error(err)
return multistep.ActionHalt
}
} }
} }
return multistep.ActionContinue return multistep.ActionContinue

View File

@ -13,8 +13,9 @@ func TestStepSnapshotDataDisksShouldFailIfSnapshotFails(t *testing.T) {
create: func(context.Context, string, string, string, map[string]*string, string) error { create: func(context.Context, string, string, string, map[string]*string, string) error {
return fmt.Errorf("!! Unit Test FAIL !!") return fmt.Errorf("!! Unit Test FAIL !!")
}, },
say: func(message string) {}, say: func(message string) {},
error: func(e error) {}, error: func(e error) {},
isManagedImage: true,
} }
stateBag := createTestStateBagStepSnapshotDataDisks() stateBag := createTestStateBagStepSnapshotDataDisks()
@ -34,8 +35,9 @@ func TestStepSnapshotDataDisksShouldPassIfSnapshotPasses(t *testing.T) {
create: func(context.Context, string, string, string, map[string]*string, string) error { create: func(context.Context, string, string, string, map[string]*string, string) error {
return nil return nil
}, },
say: func(message string) {}, say: func(message string) {},
error: func(e error) {}, error: func(e error) {},
isManagedImage: true,
} }
stateBag := createTestStateBagStepSnapshotDataDisks() stateBag := createTestStateBagStepSnapshotDataDisks()

View File

@ -11,17 +11,19 @@ import (
) )
type StepSnapshotOSDisk struct { type StepSnapshotOSDisk struct {
client *AzureClient client *AzureClient
create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error
say func(message string) say func(message string)
error func(e error) error func(e error)
isManagedImage bool
} }
func NewStepSnapshotOSDisk(client *AzureClient, ui packer.Ui) *StepSnapshotOSDisk { func NewStepSnapshotOSDisk(client *AzureClient, ui packer.Ui, isManagedImage bool) *StepSnapshotOSDisk {
var step = &StepSnapshotOSDisk{ var step = &StepSnapshotOSDisk{
client: client, client: client,
say: func(message string) { ui.Say(message) }, say: func(message string) { ui.Say(message) },
error: func(e error) { ui.Error(e.Error()) }, error: func(e error) { ui.Error(e.Error()) },
isManagedImage: isManagedImage,
} }
step.create = step.createSnapshot step.create = step.createSnapshot
@ -68,21 +70,24 @@ func (s *StepSnapshotOSDisk) createSnapshot(ctx context.Context, resourceGroupNa
} }
func (s *StepSnapshotOSDisk) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { func (s *StepSnapshotOSDisk) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction {
s.say("Taking snapshot of OS disk ...") if s.isManagedImage {
var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) s.say("Taking snapshot of OS disk ...")
var location = stateBag.Get(constants.ArmLocation).(string)
var tags = stateBag.Get(constants.ArmTags).(map[string]*string)
var srcUriVhd = stateBag.Get(constants.ArmOSDiskVhd).(string)
var dstSnapshotName = stateBag.Get(constants.ArmManagedImageOSDiskSnapshotName).(string)
err := s.create(ctx, resourceGroupName, srcUriVhd, location, tags, dstSnapshotName) var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string)
var location = stateBag.Get(constants.ArmLocation).(string)
var tags = stateBag.Get(constants.ArmTags).(map[string]*string)
var srcUriVhd = stateBag.Get(constants.ArmOSDiskVhd).(string)
var dstSnapshotName = stateBag.Get(constants.ArmManagedImageOSDiskSnapshotName).(string)
if err != nil { err := s.create(ctx, resourceGroupName, srcUriVhd, location, tags, dstSnapshotName)
stateBag.Put(constants.Error, err)
s.error(err)
return multistep.ActionHalt if err != nil {
stateBag.Put(constants.Error, err)
s.error(err)
return multistep.ActionHalt
}
} }
return multistep.ActionContinue return multistep.ActionContinue

View File

@ -13,8 +13,9 @@ func TestStepSnapshotOSDiskShouldFailIfSnapshotFails(t *testing.T) {
create: func(context.Context, string, string, string, map[string]*string, string) error { create: func(context.Context, string, string, string, map[string]*string, string) error {
return fmt.Errorf("!! Unit Test FAIL !!") return fmt.Errorf("!! Unit Test FAIL !!")
}, },
say: func(message string) {}, say: func(message string) {},
error: func(e error) {}, error: func(e error) {},
isManagedImage: true,
} }
stateBag := createTestStateBagStepSnapshotOSDisk() stateBag := createTestStateBagStepSnapshotOSDisk()
@ -34,8 +35,9 @@ func TestStepSnapshotOSDiskShouldPassIfSnapshotPasses(t *testing.T) {
create: func(context.Context, string, string, string, map[string]*string, string) error { create: func(context.Context, string, string, string, map[string]*string, string) error {
return nil return nil
}, },
say: func(message string) {}, say: func(message string) {},
error: func(e error) {}, error: func(e error) {},
isManagedImage: true,
} }
stateBag := createTestStateBagStepSnapshotOSDisk() stateBag := createTestStateBagStepSnapshotOSDisk()