Skip to content

Commit e469d38

Browse files
authored
fix: wait for AWS OpsManager VM to be running (#702)
PR #696 added a wait for the AWS OpsManager VM to start running. This builds on top of that change, updating the expected output from the AWS CLI so that it works. Tests were also updated. Note that the tests are quite fragile and would benefit from re-work. Specifically the `runner.ExecuteWithEnvVarsReturnsOnCall()` test fake is problematic because (a) it's possible to write tests that pass but do not match the actual implmentation of the AWS CLI, and (b) adding/removing/changing the order of calls requires the tests to be re-written.
1 parent 4607227 commit e469d38

File tree

2 files changed

+28
-28
lines changed

2 files changed

+28
-28
lines changed

vmlifecycle/vmmanagers/aws.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ func (a *AWSVMManager) CreateVM() (Status, StateInfo, error) {
132132
}
133133
latestState.ID = instanceID
134134

135-
if err := a.waitTillVmRunning(instanceID); err != nil {
136-
return Incomplete, latestState, fmt.Errorf("aws error creating the vm: %s", err)
135+
if err := a.waitUntilVMRunning(instanceID); err != nil {
136+
return Incomplete, latestState, fmt.Errorf("aws error waiting for VM to enter running state: %w", err)
137137
}
138138

139139
if iaasConfig.PublicIP != "" {
@@ -273,28 +273,26 @@ func (a *AWSVMManager) createVM(ami string) (string, error) {
273273
return cleanupString(instanceID.String()), nil
274274
}
275275

276-
func (a *AWSVMManager) waitTillVmRunning(instanceID string) error {
277-
// describe state and wait till running
278-
for i := 0; i <= 200; i++ {
279-
state, _, err := a.ExecuteWithInstanceProfile(a.addEnvVars(),
280-
[]interface{}{
276+
func (a *AWSVMManager) waitUntilVMRunning(instanceID string) error {
277+
for range 200 {
278+
stdout, _, err := a.ExecuteWithInstanceProfile(a.addEnvVars(),
279+
[]any{
281280
"ec2", "describe-instances",
282281
"--instance-ids", instanceID,
283-
"--query", "Reservations[*].Instances[*].State.Name",
284-
})
282+
"--query", "Reservations[0].Instances[0].State.Name",
283+
},
284+
)
285285
if err != nil {
286-
return fmt.Errorf("could not check the instance state for %s", instanceID)
286+
return fmt.Errorf("could not check the instance state for %s: %w", instanceID, err)
287287
}
288-
if cleanupString(state.String()) == "running" {
288+
if cleanupString(stdout.String()) == "running" {
289289
return nil
290290
}
291-
if i == 200 {
292-
return fmt.Errorf("timeout exceeded in running status")
293-
}
294291
time.Sleep(a.pollingInterval)
295292
}
296-
return nil
293+
return fmt.Errorf("timeout exceeded waiting for the VM to enter the running state")
297294
}
295+
298296
func (a *AWSVMManager) getIPAddressID() (ipAddress string, err error) {
299297
allocationID, _, err := a.ExecuteWithInstanceProfile(a.addEnvVars(),
300298
[]interface{}{

vmlifecycle/vmmanagers/aws_test.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,10 @@ opsman-configuration:
6060
`
6161

6262
command, runner := createCommand(fmt.Sprintf(configStrTemplate, accessKey, secretAccessKey, assumeRole, region, publicIP, privateIP))
63-
// Set default return value for all calls (for waitTillVmRunning)
64-
runner.ExecuteWithEnvVarsReturns(bytes.NewBufferString("\"running\"\r\n"), nil, nil)
6563
// Override specific calls with their expected return values
66-
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
67-
runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil)
64+
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil) // ec2 run-instances
65+
runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString("\"running\"\r\n"), nil, nil) // ec2 describe-instances
66+
runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("\"eipalloc-18643c24\"\r\n"), nil, nil) // ec2 describe-addresses
6867
runner.ExecuteWithEnvVarsReturnsOnCall(5, bytes.NewBufferString("\"stopping\"\r\n"), nil, nil)
6968
runner.ExecuteWithEnvVarsReturnsOnCall(6, bytes.NewBufferString("\"stopped\"\r\n"), nil, nil)
7069
runner.ExecuteWithEnvVarsReturnsOnCall(7, bytes.NewBufferString("\"stopped\"\r\n"), nil, nil)
@@ -98,7 +97,7 @@ opsman-configuration:
9897
{
9998
`ec2`, `describe-instances`,
10099
`--instance-ids`, `i-0016d0fe3a11c73c2`,
101-
`--query`, `Reservations[*].Instances[*].State.Name`,
100+
`--query`, `Reservations[0].Instances[0].State.Name`,
102101
},
103102
{
104103
`ec2`, `describe-addresses`,
@@ -258,7 +257,9 @@ credential_source = Ec2InstanceMetadata`)), comment)
258257
It("execute as if no vm has been deployed", func() {
259258
command, runner := createValidCommand("1.2.3.4", "10.10.10.10", "us-west-2")
260259
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString("terminated\r\n"), nil, nil)
261-
runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
260+
runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil) // ec2 run-instances
261+
runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("\"running\"\r\n"), nil, nil) // ec2 describe-instances
262+
runner.ExecuteWithEnvVarsReturnsOnCall(3, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
262263

263264
command.State = vmmanagers.StateInfo{
264265
IAAS: "aws",
@@ -278,6 +279,7 @@ credential_source = Ec2InstanceMetadata`)), comment)
278279
command, runner := createValidCommand("1.2.3.4", "10.10.10.10", "us-west-2")
279280
runner.ExecuteWithEnvVarsReturnsOnCall(0, bytes.NewBufferString("[]\n"), nil, nil)
280281
runner.ExecuteWithEnvVarsReturnsOnCall(1, bytes.NewBufferString(fmt.Sprintf("\"%s\"\r\n", vmID)), nil, nil)
282+
runner.ExecuteWithEnvVarsReturnsOnCall(2, bytes.NewBufferString("\"running\"\r\n"), nil, nil) // ec2 describe-instances
281283

282284
command.State = vmmanagers.StateInfo{
283285
IAAS: "aws",
@@ -393,11 +395,12 @@ credential_source = Ec2InstanceMetadata`)), comment)
393395
Expect(err.Error()).To(ContainSubstring("aws error "))
394396
},
395397
Entry("create vm", 0, vmmanagers.Unknown),
396-
Entry("find root device", 1, vmmanagers.Incomplete),
397-
Entry("modify root device size", 2, vmmanagers.Incomplete),
398-
Entry("find public ip address", 3, vmmanagers.Incomplete),
399-
Entry("assign public ip address", 4, vmmanagers.Incomplete),
400-
Entry("reboot vm", 5, vmmanagers.Incomplete),
398+
Entry("wait for running", 1, vmmanagers.Incomplete),
399+
Entry("find root device", 2, vmmanagers.Incomplete),
400+
Entry("modify root device size", 3, vmmanagers.Incomplete),
401+
Entry("find public ip address", 4, vmmanagers.Incomplete),
402+
Entry("assign public ip address", 5, vmmanagers.Incomplete),
403+
Entry("reboot vm", 6, vmmanagers.Incomplete),
401404
)
402405
})
403406

@@ -551,8 +554,7 @@ opsman-configuration:
551554
private_ip: 1.2.3.4
552555
`)
553556
runner.ExecuteWithEnvVarsReturnsOnCall(4, bytes.NewBufferString("some-id\r\n"), nil, nil)
554-
// Add return values for waitTillVmRunning calls
555-
runner.ExecuteWithEnvVarsReturnsOnCall(5, bytes.NewBufferString("running\r\n"), nil, nil)
557+
runner.ExecuteWithEnvVarsReturnsOnCall(5, bytes.NewBufferString("\"running\"\r\n"), nil, nil) // waitUntilVMRunning()
556558
runner.ExecuteWithEnvVarsReturnsOnCall(11, bytes.NewBufferString("stopped\r\n"), nil, nil)
557559

558560
_, _, err := command.CreateVM()

0 commit comments

Comments
 (0)