Skip to content

Commit 617d89c

Browse files
nbhatt-atlassiannbhatt-atlassian
committed
Reduce no-AMI EBS cleanup waits
1 parent 8b0c09b commit 617d89c

12 files changed

Lines changed: 256 additions & 12 deletions

‎builder/common/step_modify_ebs_instance.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,24 @@ type StepModifyEBSBackedInstance struct {
1919
Skip bool
2020
EnableAMIENASupport config.Trilean
2121
EnableAMISriovNetSupport bool
22+
AMISkipCreateImage bool
2223
}
2324

2425
func (s *StepModifyEBSBackedInstance) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
25-
ec2conn := state.Get("ec2").(ec2iface.EC2API)
26-
instance := state.Get("instance").(*ec2.Instance)
27-
ui := state.Get("ui").(packersdk.Ui)
28-
2926
// Skip when it is a spot instance
3027
if s.Skip {
3128
return multistep.ActionContinue
3229
}
3330

31+
ui := state.Get("ui").(packersdk.Ui)
32+
if s.AMISkipCreateImage {
33+
ui.Say("skip_create_ami was set; skipping source instance attribute modification")
34+
return multistep.ActionContinue
35+
}
36+
37+
ec2conn := state.Get("ec2").(ec2iface.EC2API)
38+
instance := state.Get("instance").(*ec2.Instance)
39+
3440
// Set SriovNetSupport to "simple". See http://goo.gl/icuXh5
3541
// As of February 2017, this applies to C3, C4, D2, I2, R3, and M4 (excluding m4.16xlarge)
3642
if s.EnableAMISriovNetSupport {

‎builder/common/step_modify_ebs_instance_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright IBM Corp. 2013, 2026
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package common
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/hashicorp/packer-plugin-sdk/multistep"
11+
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
12+
"github.com/hashicorp/packer-plugin-sdk/template/config"
13+
)
14+
15+
func TestStepModifyEBSBackedInstance_SkipCreateAMISkipsModification(t *testing.T) {
16+
state := new(multistep.BasicStateBag)
17+
state.Put("ui", packersdk.TestUi(t))
18+
19+
step := &StepModifyEBSBackedInstance{
20+
AMISkipCreateImage: true,
21+
EnableAMISriovNetSupport: true,
22+
EnableAMIENASupport: config.TriTrue,
23+
}
24+
25+
action := step.Run(context.Background(), state)
26+
27+
if action != multistep.ActionContinue {
28+
t.Fatalf("expected ActionContinue, got %v", action)
29+
}
30+
31+
if rawErr, ok := state.GetOk("error"); ok {
32+
t.Fatalf("expected no error, got %v", rawErr)
33+
}
34+
}

‎builder/common/step_run_source_instance.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ type StepRunSourceInstance struct {
5656
NoEphemeral bool
5757
EnableNitroEnclave bool
5858
IsBurstableInstanceType bool
59+
SkipTerminationWait bool
5960

6061
instanceId string
6162
}
@@ -488,6 +489,11 @@ func (s *StepRunSourceInstance) Cleanup(state multistep.StateBag) {
488489
return
489490
}
490491

492+
if s.SkipTerminationWait {
493+
ui.Say("Source instance termination was accepted; skipping final termination wait")
494+
return
495+
}
496+
491497
if err := s.PollingConfig.WaitUntilInstanceTerminated(aws.BackgroundContext(), ec2conn, s.instanceId); err != nil {
492498
ui.Error(err.Error())
493499
}

‎builder/common/step_stop_ebs_instance.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,25 @@ type StepStopEBSBackedInstance struct {
1919
PollingConfig *AWSPollingConfig
2020
Skip bool
2121
DisableStopInstance bool
22+
AMISkipCreateImage bool
2223
}
2324

2425
func (s *StepStopEBSBackedInstance) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
25-
ec2conn := state.Get("ec2").(*ec2.EC2)
26-
instance := state.Get("instance").(*ec2.Instance)
2726
ui := state.Get("ui").(packersdk.Ui)
2827

2928
// Skip when it is a spot instance
3029
if s.Skip {
3130
return multistep.ActionContinue
3231
}
3332

33+
if s.AMISkipCreateImage {
34+
ui.Say("skip_create_ami was set; skipping source instance stop")
35+
return multistep.ActionContinue
36+
}
37+
38+
ec2conn := state.Get("ec2").(*ec2.EC2)
39+
instance := state.Get("instance").(*ec2.Instance)
40+
3441
var err error
3542

3643
if !s.DisableStopInstance {

‎builder/common/step_stop_ebs_instance_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package common
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/hashicorp/packer-plugin-sdk/multistep"
11+
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
12+
)
13+
14+
func TestStepStopEBSBackedInstance_SkipCreateAMISkipsStop(t *testing.T) {
15+
state := new(multistep.BasicStateBag)
16+
state.Put("ui", packersdk.TestUi(t))
17+
18+
step := &StepStopEBSBackedInstance{
19+
AMISkipCreateImage: true,
20+
}
21+
22+
action := step.Run(context.Background(), state)
23+
24+
if action != multistep.ActionContinue {
25+
t.Fatalf("expected ActionContinue, got %v", action)
26+
}
27+
28+
if rawErr, ok := state.GetOk("error"); ok {
29+
t.Fatalf("expected no error, got %v", rawErr)
30+
}
31+
}

‎builder/ebs/builder.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ type Builder struct {
9696
runner multistep.Runner
9797
}
9898

99+
func shouldSkipSourceInstanceTerminationWait(config *Config) bool {
100+
return config.AMISkipCreateImage &&
101+
(len(config.SecurityGroupIds) > 0 || !config.SecurityGroupFilter.Empty()) &&
102+
config.TemporaryIamInstanceProfilePolicyDocument == nil
103+
}
104+
99105
func (b *Builder) ConfigSpec() hcldec.ObjectSpec { return b.config.FlatMapstructure().HCL2Spec() }
100106

101107
func (b *Builder) Prepare(raws ...interface{}) ([]string, []string, error) {
@@ -221,6 +227,8 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
221227

222228
var instanceStep multistep.Step
223229

230+
skipTerminationWait := shouldSkipSourceInstanceTerminationWait(&b.config)
231+
224232
if b.config.IsSpotInstance() {
225233
instanceStep = &awscommon.StepRunSpotInstance{
226234
PollingConfig: b.config.PollingConfig,
@@ -296,6 +304,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
296304
UserDataFile: b.config.UserDataFile,
297305
VolumeTags: b.config.VolumeRunTags,
298306
NoEphemeral: b.config.NoEphemeral,
307+
SkipTerminationWait: skipTerminationWait,
299308
}
300309
}
301310

@@ -400,10 +409,12 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
400409
PollingConfig: b.config.PollingConfig,
401410
Skip: b.config.IsSpotInstance(),
402411
DisableStopInstance: b.config.DisableStopInstance,
412+
AMISkipCreateImage: b.config.AMISkipCreateImage,
403413
},
404414
&awscommon.StepModifyEBSBackedInstance{
405415
EnableAMISriovNetSupport: b.config.AMISriovNetSupport,
406416
EnableAMIENASupport: b.config.AMIENASupport,
417+
AMISkipCreateImage: b.config.AMISkipCreateImage,
407418
},
408419
&awscommon.StepDeregisterAMI{
409420
AccessConfig: &b.config.AccessConfig,

‎builder/ebs/builder_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import (
88
"testing"
99
"time"
1010

11+
awscommon "github.com/hashicorp/packer-plugin-amazon/common"
1112
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
13+
sdkconfig "github.com/hashicorp/packer-plugin-sdk/template/config"
1214
)
1315

1416
func testConfig() map[string]interface{} {
@@ -133,6 +135,69 @@ func TestBuilderPrepare_InvalidShutdownBehavior(t *testing.T) {
133135
}
134136
}
135137

138+
func TestShouldSkipSourceInstanceTerminationWait(t *testing.T) {
139+
tests := []struct {
140+
name string
141+
config Config
142+
want bool
143+
}{
144+
{
145+
name: "skip create image with existing security group",
146+
config: Config{
147+
AMISkipCreateImage: true,
148+
RunConfig: awscommon.RunConfig{
149+
SecurityGroupIds: []string{"sg-123"},
150+
},
151+
},
152+
want: true,
153+
},
154+
{
155+
name: "skip create image with security group filter",
156+
config: Config{
157+
AMISkipCreateImage: true,
158+
RunConfig: awscommon.RunConfig{
159+
SecurityGroupFilter: awscommon.SecurityGroupFilterOptions{
160+
NameValueFilter: sdkconfig.NameValueFilter{
161+
Filters: map[string]string{"tag:Name": "packer-build"},
162+
},
163+
},
164+
},
165+
},
166+
want: true,
167+
},
168+
{
169+
name: "image creation still waits for termination",
170+
config: Config{
171+
RunConfig: awscommon.RunConfig{
172+
SecurityGroupIds: []string{"sg-123"},
173+
},
174+
},
175+
want: false,
176+
},
177+
{
178+
name: "temporary instance profile still waits for cleanup ordering",
179+
config: Config{
180+
AMISkipCreateImage: true,
181+
RunConfig: awscommon.RunConfig{
182+
SecurityGroupIds: []string{"sg-123"},
183+
TemporaryIamInstanceProfilePolicyDocument: &awscommon.PolicyDocument{
184+
Version: "2012-10-17",
185+
},
186+
},
187+
},
188+
want: false,
189+
},
190+
}
191+
192+
for _, tt := range tests {
193+
t.Run(tt.name, func(t *testing.T) {
194+
if got := shouldSkipSourceInstanceTerminationWait(&tt.config); got != tt.want {
195+
t.Fatalf("got %v, want %v", got, tt.want)
196+
}
197+
})
198+
}
199+
}
200+
136201
func TestBuilderPrepare_DeprecationTime(t *testing.T) {
137202
var b Builder
138203
config := testConfig()

‎common/step_modify_ebs_instance.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,24 @@ type StepModifyEBSBackedInstance struct {
2020
Skip bool
2121
EnableAMIENASupport config.Trilean
2222
EnableAMISriovNetSupport bool
23+
AMISkipCreateImage bool
2324
}
2425

2526
func (s *StepModifyEBSBackedInstance) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
26-
ec2Client := state.Get("ec2v2").(clients.Ec2Client)
27-
instance := state.Get("instance").(ec2types.Instance)
28-
ui := state.Get("ui").(packersdk.Ui)
29-
3027
// Skip when it is a spot instance
3128
if s.Skip {
3229
return multistep.ActionContinue
3330
}
3431

32+
ui := state.Get("ui").(packersdk.Ui)
33+
if s.AMISkipCreateImage {
34+
ui.Say("skip_create_ami was set; skipping source instance attribute modification")
35+
return multistep.ActionContinue
36+
}
37+
38+
ec2Client := state.Get("ec2v2").(clients.Ec2Client)
39+
instance := state.Get("instance").(ec2types.Instance)
40+
3541
// Set SriovNetSupport to "simple". See http://goo.gl/icuXh5
3642
// As of February 2017, this applies to C3, C4, D2, I2, R3, and M4 (excluding m4.16xlarge)
3743
if s.EnableAMISriovNetSupport {

‎common/step_modify_ebs_instance_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright IBM Corp. 2013, 2026
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package common
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/hashicorp/packer-plugin-sdk/multistep"
11+
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
12+
"github.com/hashicorp/packer-plugin-sdk/template/config"
13+
)
14+
15+
func TestStepModifyEBSBackedInstance_SkipCreateAMISkipsModification(t *testing.T) {
16+
state := new(multistep.BasicStateBag)
17+
state.Put("ui", packersdk.TestUi(t))
18+
19+
step := &StepModifyEBSBackedInstance{
20+
AMISkipCreateImage: true,
21+
EnableAMISriovNetSupport: true,
22+
EnableAMIENASupport: config.TriTrue,
23+
}
24+
25+
action := step.Run(context.Background(), state)
26+
27+
if action != multistep.ActionContinue {
28+
t.Fatalf("expected ActionContinue, got %v", action)
29+
}
30+
31+
if rawErr, ok := state.GetOk("error"); ok {
32+
t.Fatalf("expected no error, got %v", rawErr)
33+
}
34+
}

‎common/step_run_source_instance.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ type StepRunSourceInstance struct {
5757
NoEphemeral bool
5858
EnableNitroEnclave bool
5959
IsBurstableInstanceType bool
60+
SkipTerminationWait bool
6061

6162
instanceId string
6263
}
@@ -493,6 +494,11 @@ func (s *StepRunSourceInstance) Cleanup(state multistep.StateBag) {
493494
return
494495
}
495496

497+
if s.SkipTerminationWait {
498+
ui.Say("Source instance termination was accepted; skipping final termination wait")
499+
return
500+
}
501+
496502
if err := s.PollingConfig.WaitUntilInstanceTerminated(ctx, ec2Client, s.instanceId); err != nil {
497503
ui.Error(err.Error())
498504
}

0 commit comments

Comments
 (0)