From c2bf4ec47f4c0056f58ac4f7df5c9d2ef675b20d Mon Sep 17 00:00:00 2001
From: Robert Abeyta <robert.abeyta@broadcom.com>
Date: Thu, 17 Oct 2024 14:20:47 -0700
Subject: [PATCH] vmlifecycle - aws - add support for setting volume type
default to gp2 (current behavior). allows user to define boot_disk_type in their aws config to instruct the type of volume to use when modifying the volume after vm creation
refactoring aws lifecycle create to no longer perform a modify volume after initial creation. pass those disk options to initial creation to allow users to modify the volume if necessary after initial create without a penalty. aws imposes a 6 hour cool down between changes and this allows a user to modify vs the create causing a 6 hour cooldown before the next change is allowed
Co-authored-by: Shawn Neal <sneal@sneal.net>
---
vmlifecycle/configfetchers/aws.go | 3 +-
vmlifecycle/configfetchers/aws_test.go | 6 +-
vmlifecycle/vmmanagers/aws.go | 78 +++-----------------------
vmlifecycle/vmmanagers/aws_test.go | 76 +++++++------------------
4 files changed, 35 insertions(+), 128 deletions(-)
diff --git a/vmlifecycle/configfetchers/aws.go b/vmlifecycle/configfetchers/aws.go
index 50363b80..2683d35d 100644
--- a/vmlifecycle/configfetchers/aws.go
+++ b/vmlifecycle/configfetchers/aws.go
@@ -93,6 +93,7 @@ func (a *AWSConfigFetcher) createConfig(output *ec2.DescribeInstancesOutput, vol
PrivateIP: *instance.PrivateIpAddress,
InstanceType: *instance.InstanceType,
BootDiskSize: strconv.Itoa(int(*volumesOutput.Volumes[0].Size)),
+ BootDiskType: *volumesOutput.Volumes[0].VolumeType,
},
},
}
@@ -103,4 +104,4 @@ func (a *AWSConfigFetcher) createConfig(output *ec2.DescribeInstancesOutput, vol
}
return opsmanConfig
-}
+}
\ No newline at end of file
diff --git a/vmlifecycle/configfetchers/aws_test.go b/vmlifecycle/configfetchers/aws_test.go
index fac987fd..5ea5ad28 100644
--- a/vmlifecycle/configfetchers/aws_test.go
+++ b/vmlifecycle/configfetchers/aws_test.go
@@ -42,6 +42,7 @@ var _ = Describe("aws", func() {
PrivateIP: "5.6.7.8",
VMName: "opsman-vm",
BootDiskSize: "160",
+ BootDiskType: "gp3",
InstanceType: "some-instance-type",
},
},
@@ -94,7 +95,8 @@ var _ = Describe("aws", func() {
return &ec2.DescribeVolumesOutput{
Volumes: []*ec2.Volume{{
- Size: aws.Int64(160),
+ Size: aws.Int64(160),
+ VolumeType: aws.String("gp3"),
}},
}, nil
}
@@ -188,4 +190,4 @@ var _ = Describe("aws", func() {
Expect(err).To(HaveOccurred())
})
})
-})
+})
\ No newline at end of file
diff --git a/vmlifecycle/vmmanagers/aws.go b/vmlifecycle/vmmanagers/aws.go
index fc85ada7..e600676f 100644
--- a/vmlifecycle/vmmanagers/aws.go
+++ b/vmlifecycle/vmmanagers/aws.go
@@ -37,6 +37,7 @@ type AWSConfig struct {
PrivateIP string `yaml:"private_ip" validate:"omitempty,ip"`
VMName string `yaml:"vm_name"`
BootDiskSize string `yaml:"boot_disk_size"`
+ BootDiskType string `yaml:"boot_disk_type"`
InstanceType string `yaml:"instance_type"`
Tags map[string]string `yaml:"tags"`
AuthenticationType string `yaml:"authentication_type"`
@@ -131,16 +132,6 @@ func (a *AWSVMManager) CreateVM() (Status, StateInfo, error) {
}
latestState.ID = instanceID
- volumeID, err := a.getVolumeID(instanceID)
- if err != nil {
- return Incomplete, latestState, err
- }
-
- err = a.modifyVolume(volumeID)
- if err != nil {
- return Incomplete, latestState, err
- }
-
if iaasConfig.PublicIP != "" {
addressID, err := a.getIPAddressID()
if err != nil {
@@ -188,6 +179,9 @@ func (a *AWSVMManager) addDefaultConfigFields() {
if a.Config.OpsmanConfig.AWS.BootDiskSize == "" {
a.Config.OpsmanConfig.AWS.BootDiskSize = "200"
}
+ if a.Config.OpsmanConfig.AWS.BootDiskType == "" {
+ a.Config.OpsmanConfig.AWS.BootDiskType = "gp2"
+ }
if a.Config.OpsmanConfig.AWS.InstanceType == "" {
a.Config.OpsmanConfig.AWS.InstanceType = "m5.large"
}
@@ -247,6 +241,10 @@ func (a *AWSVMManager) createVM(ami string) (string, error) {
),
"--image-id", ami,
"--subnet-id", config.VPCSubnetId,
+ "--block-device-mappings", fmt.Sprintf(
+ "[{\"DeviceName\": \"/dev/xvda\", \"Ebs\": {\"VolumeType\": \"%s\", \"VolumeSize\": %s}}]",
+ a.Config.OpsmanConfig.AWS.BootDiskType, a.Config.OpsmanConfig.AWS.BootDiskSize,
+ ),
"--security-group-ids",
}
for _, sgID := range config.SecurityGroupIds {
@@ -272,64 +270,6 @@ func (a *AWSVMManager) createVM(ami string) (string, error) {
return cleanupString(instanceID.String()), nil
}
-func (a *AWSVMManager) getVolumeID(instanceID string) (volumeID string, err error) {
- // wait until available
- for {
- volumeID, _, err := a.ExecuteWithInstanceProfile(a.addEnvVars(),
- []interface{}{
- "ec2", "describe-volumes",
- "--filters",
- fmt.Sprintf("Name=attachment.instance-id,Values=%s", instanceID),
- "Name=attachment.status,Values=attached",
- "Name=status,Values=in-use",
- "--query", "Volumes[0].VolumeId",
- })
- if err != nil {
- return "", fmt.Errorf("aws error finding volumeID of the root device: %s", err)
- }
- if !strings.Contains(volumeID.String(), "null") {
- return cleanupString(volumeID.String()), nil
- }
- _, _ = a.stderr.Write([]byte(fmt.Sprintf("volume not available yet, polling in %s\n", a.pollingInterval)))
- time.Sleep(a.pollingInterval)
- }
-}
-
-func (a *AWSVMManager) modifyVolume(volumeID string) error {
- // wait until available
- var currentRetryCount float64
- timeoutTime := time.Now().Add(time.Hour)
- for {
- _, _, err := a.ExecuteWithInstanceProfile(a.addEnvVars(),
- []interface{}{
- "ec2", "modify-volume",
- "--volume-id", volumeID,
- "--size", a.Config.OpsmanConfig.AWS.BootDiskSize,
- })
-
- if err == nil {
- break
- }
-
- if !strings.Contains(err.Error(), "exit status 255") {
- return fmt.Errorf("aws error modifying size of root device: %s", err)
- }
-
- // Encrypted volume is not available
- waitTime := getExponentialWaitTime(currentRetryCount, a.pollingInterval)
- _, _ = a.stderr.Write([]byte(fmt.Sprintf("volume not available to configure yet, polling in %s\n", waitTime)))
-
- if time.Now().After(timeoutTime) {
- return errors.New("failed to modify VM disk volume within the hour allowed")
- }
-
- time.Sleep(waitTime)
- currentRetryCount += 1
- }
-
- return nil
-}
-
func (a *AWSVMManager) getIPAddressID() (ipAddress string, err error) {
allocationID, _, err := a.ExecuteWithInstanceProfile(a.addEnvVars(),
[]interface{}{
@@ -530,4 +470,4 @@ func getExponentialWaitTime(currentRetryCount float64, pollingMultiplier time.Du
}
return waitTime
-}
+}
\ No newline at end of file
diff --git a/vmlifecycle/vmmanagers/aws_test.go b/vmlifecycle/vmmanagers/aws_test.go
index 1988fb58..2ef7ad74 100644
--- a/vmlifecycle/vmmanagers/aws_test.go
+++ b/vmlifecycle/vmmanagers/aws_test.go
@@ -49,6 +49,7 @@ opsman-configuration:
key_pair_name: superuser
iam_instance_profile_name: awesome-profile
boot_disk_size: 200
+ boot_disk_type: gp3
public_ip: %s
private_ip: %s
instance_type: m3.large
@@ -60,11 +61,9 @@ opsman-configuration:
command, runner := createCommand(fmt.Sprintf(configStrTemplate, accessKey, secretAccessKey, assumeRole, region, publicIP, privateIP))
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString("null\r\n"), bytes.NewBufferString("TestStatus: pending creation"), nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("\"vol-0cf5b911680a78bb9\"\r\n"), bytes.NewBufferString("TestStatus: volume created"), nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(4, bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(7, bytes.NewBufferString("\"stopping\"\r\n"), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(8, bytes.NewBufferString("\"stopped\"\r\n"), nil, nil)
+ runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil)
+ runner.ExecuteWithEnvVarsReturnsOnCall(4, bytes.NewBufferString("\"stopping\"\r\n"), nil, nil)
+ runner.ExecuteWithEnvVarsReturnsOnCall(5, bytes.NewBufferString("\"stopped\"\r\n"), nil, nil)
return command, runner
}
@@ -75,13 +74,14 @@ opsman-configuration:
Context("create vm", func() {
Context("with a config with all values filled out", func() {
It("calls aws with correct cli arguments and does not describe instances", func() {
- numCLICalls := 10
+ numCLICalls := 7
commands := [][]string{
- { //1
+ {
`ec2`, `run-instances`,
`--tag-specifications`, `ResourceType=instance,Tags=[{Key=Name,Value=awesome-vm},{Key=Owner,Value=DbAdmin},{Key=Stack,Value=Test}]`,
`--image-id`, `ami-789dc900`,
`--subnet-id`, `awesome-subnet`,
+ `--block-device-mappings`, `[{"DeviceName": "/dev/xvda", "Ebs": {"VolumeType": "gp3", "VolumeSize": 200}}]`,
`--security-group-ids`, `sg-awesome`, `sg-great`,
`--count`, `1`,
`--instance-type`, "m3.large",
@@ -91,50 +91,31 @@ opsman-configuration:
`--query`, `Instances[0].InstanceId`,
`--private-ip-address`, `10.10.10.10`,
},
- { //2
- `ec2`, `describe-volumes`,
- `--filters`, `Name=attachment.instance-id,Values=i-0016d0fe3a11c73c2`,
- `Name=attachment.status,Values=attached`,
- `Name=status,Values=in-use`,
- `--query`, `Volumes[0].VolumeId`,
- },
- { //3
- `ec2`, `describe-volumes`,
- `--filters`, `Name=attachment.instance-id,Values=i-0016d0fe3a11c73c2`,
- `Name=attachment.status,Values=attached`,
- `Name=status,Values=in-use`,
- `--query`, `Volumes[0].VolumeId`,
- },
- { //4
- `ec2`, `modify-volume`,
- `--volume-id`, `vol-0cf5b911680a78bb9`,
- `--size`, `200`,
- },
- { //5
+ {
`ec2`, `describe-addresses`,
`--filters`, `Name=public-ip,Values=1.2.3.4`,
`--query`, `Addresses[0].AllocationId`,
},
- { //6
+ {
`ec2`, `associate-address`,
`--allocation-id`, `eipalloc-18643c24`,
`--instance-id`, `i-0016d0fe3a11c73c2`,
},
- { //7
+ {
`ec2`, `stop-instances`,
`--instance-ids`, `i-0016d0fe3a11c73c2`,
},
- { //8
+ {
`ec2`, `describe-instances`,
`--instance-ids`, `i-0016d0fe3a11c73c2`,
`--query`, `Reservations[*].Instances[*].State.Name`,
},
- { //9
+ {
`ec2`, `describe-instances`,
`--instance-ids`, `i-0016d0fe3a11c73c2`,
`--query`, `Reservations[*].Instances[*].State.Name`,
},
- { //10
+ {
`ec2`, `start-instances`,
`--instance-ids`, `i-0016d0fe3a11c73c2`,
},
@@ -174,7 +155,7 @@ opsman-configuration:
Expect(invokes).ToNot(HaveLen(0))
for _, args := range invokes {
Expect(args[1]).ToNot(ContainElement(MatchRegexp("associate-address")))
- Expect(args[1]).ToNot(ContainElement(MatchRegexp("decsribe-address")))
+ Expect(args[1]).ToNot(ContainElement(MatchRegexp("describe-address")))
}
})
})
@@ -269,11 +250,6 @@ credential_source = Ec2InstanceMetadata`)), comment)
command, runner := createValidCommand("1.2.3.4", "10.10.10.10", "us-west-2")
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString("terminated\r\n"), nil, nil)
runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("null\r\n"), bytes.NewBufferString("TestStatus: pending creation"), nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(3, bytes.NewBufferString("\"vol-0cf5b911680a78bb9\"\r\n"), bytes.NewBufferString("TestStatus: volume created"), nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(5, bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(8, bytes.NewBufferString("\"stopping\"\r\n"), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(9, bytes.NewBufferString("\"stopped\"\r\n"), nil, nil)
command.State = vmmanagers.StateInfo{
IAAS: "aws",
@@ -293,11 +269,6 @@ credential_source = Ec2InstanceMetadata`)), comment)
command, runner := createValidCommand("1.2.3.4", "10.10.10.10", "us-west-2")
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString("[]\n"), nil, nil)
runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("null\r\n"), bytes.NewBufferString("TestStatus: pending creation"), nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(3, bytes.NewBufferString("\"vol-0cf5b911680a78bb9\"\r\n"), bytes.NewBufferString("TestStatus: volume created"), nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(5, bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(8, bytes.NewBufferString("\"stopping\"\r\n"), nil, nil)
- runner.ExecuteWithEnvVarsReturnsOnCall(9, bytes.NewBufferString("\"stopped\"\r\n"), nil, nil)
command.State = vmmanagers.StateInfo{
IAAS: "aws",
@@ -579,9 +550,8 @@ opsman-configuration:
_, args := runner.ExecuteWithEnvVarsArgsForCall(0)
Expect(args).To(ContainElement(MatchRegexp("ops-manager-vm")))
Expect(args).To(ContainElement(MatchRegexp("m5\\.large")))
-
- _, args = runner.ExecuteWithEnvVarsArgsForCall(2)
Expect(args).To(ContainElement(MatchRegexp("200")))
+ Expect(args).To(ContainElement(MatchRegexp("gp2")))
})
})
@@ -843,23 +813,17 @@ func happyPathAWSRunnerStubFunc(vmID string) func() (*bytes.Buffer, *bytes.Buffe
case 0:
return bytes.NewBufferString(fmt.Sprintf("%q\r\n", vmID)), nil, nil
case 1:
- return bytes.NewBufferString("null\r\n"), bytes.NewBufferString("TestStatus: pending creation"), nil
- case 2:
- return bytes.NewBufferString("\"vol-0cf5b911680a78bb9\"\r\n"), bytes.NewBufferString("TestStatus: volume created"), nil
- case 3:
- return nil, nil, nil
- case 4:
return bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil
- case 5, 6:
+ case 2, 3:
return nil, nil, nil
- case 7:
+ case 4:
return bytes.NewBufferString("\"stopping\"\r\n"), nil, nil
- case 8:
+ case 5:
return bytes.NewBufferString("\"stopped\"\r\n"), nil, nil
- case 9, 10:
+ case 6, 7:
return nil, nil, nil
default:
panic("stub for nth call not implemented")
}
}
-}
+}
\ No newline at end of file