Skip to content

Commit 476f88e

Browse files
crohrcrohr
committed
Retry stale Lightsail restores with fresh create
1 parent 23a9745 commit 476f88e

5 files changed

Lines changed: 196 additions & 36 deletions

File tree

‎internal/providers/lightsail/lightsail.go‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,10 @@ func (p *Provider) Terminate(name string) error {
132132
if err != nil {
133133
return err
134134
}
135-
if len(resp.Operations) == 0 {
136-
return nil
137-
}
138-
if resp.Operations[0].ErrorCode != nil {
135+
if len(resp.Operations) > 0 && resp.Operations[0].ErrorCode != nil {
139136
return errors.New(*resp.Operations[0].ErrorCode)
140137
}
141-
return nil
138+
return p.waitUntilDeleted(name)
142139
}
143140

144141
func (p *Provider) Launch(name string, opts pullpreview.LaunchOptions) (pullpreview.AccessDetails, error) {
@@ -207,12 +204,30 @@ func (p *Provider) launchOrRestore(name string, opts pullpreview.LaunchOptions)
207204
}
208205

209206
snapshot := p.latestSnapshot(name)
210-
if strings.EqualFold(strings.TrimSpace(opts.Tags["pullpreview_target"]), string(pullpreview.DeploymentTargetHelm)) {
207+
switch {
208+
case opts.SkipRestore:
209+
if p.logger != nil {
210+
p.logger.Infof("Skipping snapshot restore for %q: fresh-create retry requested", name)
211+
}
212+
snapshot = nil
213+
case strings.EqualFold(strings.TrimSpace(opts.Tags["pullpreview_target"]), string(pullpreview.DeploymentTargetHelm)):
214+
if p.logger != nil {
215+
p.logger.Infof("Skipping snapshot restore for %q: deployment_target=helm", name)
216+
}
211217
snapshot = nil
212218
}
213219
if snapshot != nil {
214220
if p.logger != nil {
215-
p.logger.Infof("Found snapshot to restore from: %s", aws.ToString(snapshot.Name))
221+
createdAt := "unknown"
222+
if snapshot.CreatedAt != nil {
223+
createdAt = snapshot.CreatedAt.UTC().Format(time.RFC3339)
224+
}
225+
p.logger.Infof(
226+
"Restoring Lightsail snapshot name=%s created_at=%s from_instance=%s",
227+
aws.ToString(snapshot.Name),
228+
createdAt,
229+
aws.ToString(snapshot.FromInstanceName),
230+
)
216231
}
217232
_, err := p.client.CreateInstancesFromSnapshot(p.ctx, &ls.CreateInstancesFromSnapshotInput{
218233
InstanceNames: []string{name},
@@ -224,6 +239,9 @@ func (p *Provider) launchOrRestore(name string, opts pullpreview.LaunchOptions)
224239
})
225240
return err
226241
}
242+
if p.logger != nil {
243+
p.logger.Infof("Creating fresh Lightsail instance name=%s", name)
244+
}
227245

228246
_, err = p.client.CreateInstances(p.ctx, params)
229247
return err

‎internal/providers/lightsail/lightsail_test.go‎

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,22 @@ import (
1212
)
1313

1414
type fakeLightsailClient struct {
15-
instanceStateOutput *ls.GetInstanceStateOutput
16-
instanceStateByName map[string]*ls.GetInstanceStateOutput
17-
deleteInstanceCalls int
18-
createInstancesCalls int
19-
createInstancesInput *ls.CreateInstancesInput
20-
createInstancesFromSnapshotCalls int
21-
createInstancesFromSnapshotInput *ls.CreateInstancesFromSnapshotInput
22-
putInstancePublicPortsCalls int
23-
getInstanceAccessDetailsOutput *ls.GetInstanceAccessDetailsOutput
24-
getInstanceOutput *ls.GetInstanceOutput
25-
getInstanceByName map[string]*types.Instance
26-
getInstanceSnapshotsOutput *ls.GetInstanceSnapshotsOutput
27-
getInstancesOutput *ls.GetInstancesOutput
28-
getRegionsOutput *ls.GetRegionsOutput
29-
getBlueprintsOutput *ls.GetBlueprintsOutput
30-
getBundlesOutput *ls.GetBundlesOutput
15+
instanceStateOutput *ls.GetInstanceStateOutput
16+
instanceStateByName map[string]*ls.GetInstanceStateOutput
17+
deleteInstanceCalls int
18+
createInstancesCalls int
19+
createInstancesInput *ls.CreateInstancesInput
20+
createInstancesFromSnapshotCalls int
21+
createInstancesFromSnapshotInput *ls.CreateInstancesFromSnapshotInput
22+
putInstancePublicPortsCalls int
23+
getInstanceAccessDetailsOutput *ls.GetInstanceAccessDetailsOutput
24+
getInstanceOutput *ls.GetInstanceOutput
25+
getInstanceByName map[string]*types.Instance
26+
getInstanceSnapshotsOutput *ls.GetInstanceSnapshotsOutput
27+
getInstancesOutput *ls.GetInstancesOutput
28+
getRegionsOutput *ls.GetRegionsOutput
29+
getBlueprintsOutput *ls.GetBlueprintsOutput
30+
getBundlesOutput *ls.GetBundlesOutput
3131
}
3232

3333
func (f *fakeLightsailClient) GetInstanceState(ctx context.Context, input *ls.GetInstanceStateInput, optFns ...func(*ls.Options)) (*ls.GetInstanceStateOutput, error) {
@@ -335,6 +335,35 @@ func TestLaunchOrRestoreRestoresSnapshotForCompose(t *testing.T) {
335335
}
336336
}
337337

338+
func TestLaunchOrRestoreSkipsSnapshotWhenFreshCreateRetryRequested(t *testing.T) {
339+
client := &fakeLightsailClient{
340+
getInstanceSnapshotsOutput: &ls.GetInstanceSnapshotsOutput{
341+
InstanceSnapshots: []types.InstanceSnapshot{
342+
{
343+
Name: aws.String("snap-1"),
344+
FromInstanceName: aws.String("demo"),
345+
State: types.InstanceSnapshotStateAvailable,
346+
},
347+
},
348+
},
349+
}
350+
p := &Provider{client: client, ctx: context.Background(), region: DefaultRegion}
351+
352+
err := p.launchOrRestore("demo", pullpreview.LaunchOptions{
353+
Tags: map[string]string{"pullpreview_target": "compose"},
354+
SkipRestore: true,
355+
})
356+
if err != nil {
357+
t.Fatalf("launchOrRestore() error: %v", err)
358+
}
359+
if client.createInstancesCalls != 1 {
360+
t.Fatalf("expected plain create for skip-restore launch, got %d", client.createInstancesCalls)
361+
}
362+
if client.createInstancesFromSnapshotCalls != 0 {
363+
t.Fatalf("did not expect snapshot restore when skip_restore=true, got %d", client.createInstancesFromSnapshotCalls)
364+
}
365+
}
366+
338367
func TestLaunchRecreatesMismatchedDeploymentIdentity(t *testing.T) {
339368
name := "gh-1-pr-1"
340369
client := &fakeLightsailClient{

‎internal/pullpreview/instance.go‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ var runSSHCombinedOutput = func(cmd *exec.Cmd) ([]byte, error) {
5050
return cmd.CombinedOutput()
5151
}
5252

53+
var waitUntilInstanceSSHReady = func(ctx context.Context, probe func() bool) bool {
54+
return WaitUntilContext(ctx, pollAttemptsForWindow(instanceSSHReadyWaitWindow, instanceSSHReadyInterval), instanceSSHReadyInterval, probe)
55+
}
56+
57+
var errInstanceSSHUnavailable = errors.New("can't connect to instance over SSH")
58+
5359
type Instance struct {
5460
Name string
5561
Subdomain string
@@ -207,8 +213,37 @@ func (i *Instance) ValidateDeploymentConfig() error {
207213
}
208214

209215
func (i *Instance) LaunchAndWait() error {
216+
if err := i.launchAndWait(false); err == nil {
217+
return nil
218+
} else {
219+
restoreProvider, ok := i.Provider.(SupportsRestore)
220+
if !errors.Is(err, errInstanceSSHUnavailable) || !ok || !restoreProvider.SupportsRestore() {
221+
return err
222+
}
223+
if i.Logger != nil {
224+
i.Logger.Warnf(
225+
"Instance name=%s never became SSH ready after %s. Terminating restored instance and retrying once with snapshot restore disabled.",
226+
i.Name,
227+
instanceSSHReadyWaitWindow,
228+
)
229+
}
230+
if terminateErr := i.Terminate(); terminateErr != nil {
231+
return fmt.Errorf("fresh-create retry failed to terminate %s: %w", i.Name, terminateErr)
232+
}
233+
if i.Logger != nil {
234+
i.Logger.Infof("Retrying instance launch name=%s skip_restore=true", i.Name)
235+
}
236+
return i.launchAndWait(true)
237+
}
238+
}
239+
240+
func (i *Instance) launchAndWait(skipRestore bool) error {
241+
createMessage := "Creating or restoring"
242+
if skipRestore {
243+
createMessage = "Creating fresh"
244+
}
210245
if i.Logger != nil {
211-
i.Logger.Infof("Creating or restoring instance name=%s size=%s", i.Name, i.Size)
246+
i.Logger.Infof("%s instance name=%s size=%s", createMessage, i.Name, i.Size)
212247
}
213248

214249
userData := UserData{
@@ -229,11 +264,12 @@ func (i *Instance) LaunchAndWait() error {
229264
userData = generatedUserData
230265
}
231266
access, err := i.Provider.Launch(i.Name, LaunchOptions{
232-
Size: i.Size,
233-
UserData: userData,
234-
Ports: i.PortsWithDefaults(),
235-
CIDRs: i.CIDRs,
236-
Tags: i.Tags,
267+
Size: i.Size,
268+
UserData: userData,
269+
Ports: i.PortsWithDefaults(),
270+
CIDRs: i.CIDRs,
271+
Tags: i.Tags,
272+
SkipRestore: skipRestore,
237273
})
238274
if err != nil {
239275
return err
@@ -247,7 +283,7 @@ func (i *Instance) LaunchAndWait() error {
247283
i.Username(),
248284
)
249285
}
250-
if ok := WaitUntilContext(i.Context, pollAttemptsForWindow(instanceSSHReadyWaitWindow, instanceSSHReadyInterval), instanceSSHReadyInterval, func() bool {
286+
if ok := waitUntilInstanceSSHReady(i.Context, func() bool {
251287
if i.Logger != nil {
252288
i.Logger.Infof(
253289
"Waiting for SSH username=%s ip=%s ssh=\"ssh %s\"",
@@ -263,7 +299,7 @@ func (i *Instance) LaunchAndWait() error {
263299
i.Logger.Warnf("SSH readiness diagnostics: %v", diagErr)
264300
}
265301
}
266-
return errors.New("can't connect to instance over SSH")
302+
return errInstanceSSHUnavailable
267303
}
268304
if i.Logger != nil {
269305
i.Logger.Infof("Instance ssh access OK")

‎internal/pullpreview/instance_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package pullpreview
22

33
import (
4+
"context"
45
"errors"
56
"os"
67
"os/exec"
@@ -18,6 +19,41 @@ func (r *captureRunner) Run(cmd *exec.Cmd) error {
1819
return nil
1920
}
2021

22+
type restoreRetryProvider struct {
23+
launchOpts []LaunchOptions
24+
terminateCalls int
25+
accessByAttempt []AccessDetails
26+
}
27+
28+
func (p *restoreRetryProvider) Launch(name string, opts LaunchOptions) (AccessDetails, error) {
29+
p.launchOpts = append(p.launchOpts, opts)
30+
if len(p.accessByAttempt) >= len(p.launchOpts) {
31+
return p.accessByAttempt[len(p.launchOpts)-1], nil
32+
}
33+
return AccessDetails{IPAddress: "1.2.3.4", Username: "ec2-user", PrivateKey: "PRIVATE"}, nil
34+
}
35+
36+
func (p *restoreRetryProvider) Terminate(name string) error {
37+
p.terminateCalls++
38+
return nil
39+
}
40+
41+
func (p *restoreRetryProvider) Running(name string) (bool, error) {
42+
return false, nil
43+
}
44+
45+
func (p *restoreRetryProvider) ListInstances(tags map[string]string) ([]InstanceSummary, error) {
46+
return nil, nil
47+
}
48+
49+
func (p *restoreRetryProvider) Username() string {
50+
return "ec2-user"
51+
}
52+
53+
func (p *restoreRetryProvider) SupportsRestore() bool {
54+
return true
55+
}
56+
2157
func TestPortsWithDefaultsDeduplicatesValues(t *testing.T) {
2258
inst := NewInstance("example", CommonOptions{
2359
Ports: []string{"443/tcp", "22", "443/tcp"},
@@ -242,3 +278,43 @@ func TestSSHReadyDiagnosticIncludesRemoteDetails(t *testing.T) {
242278
t.Fatalf("expected cloud-init details in error, got %v", err)
243279
}
244280
}
281+
282+
func TestLaunchAndWaitRetriesWithoutRestoreAfterSSHTimeout(t *testing.T) {
283+
provider := &restoreRetryProvider{}
284+
inst := NewInstance("my-app", CommonOptions{}, provider, nil)
285+
286+
originalWait := waitUntilInstanceSSHReady
287+
defer func() { waitUntilInstanceSSHReady = originalWait }()
288+
289+
waitCalls := 0
290+
waitUntilInstanceSSHReady = func(ctx context.Context, probe func() bool) bool {
291+
waitCalls++
292+
if waitCalls == 1 {
293+
return false
294+
}
295+
return true
296+
}
297+
298+
originalSSH := runSSHCombinedOutput
299+
defer func() { runSSHCombinedOutput = originalSSH }()
300+
301+
runSSHCombinedOutput = func(cmd *exec.Cmd) ([]byte, error) {
302+
return []byte("ready-marker-missing"), errors.New("exit status 1")
303+
}
304+
305+
if err := inst.LaunchAndWait(); err != nil {
306+
t.Fatalf("LaunchAndWait() error: %v", err)
307+
}
308+
if len(provider.launchOpts) != 2 {
309+
t.Fatalf("expected two launch attempts, got %d", len(provider.launchOpts))
310+
}
311+
if provider.launchOpts[0].SkipRestore {
312+
t.Fatalf("did not expect first launch to skip restore")
313+
}
314+
if !provider.launchOpts[1].SkipRestore {
315+
t.Fatalf("expected second launch to skip restore")
316+
}
317+
if provider.terminateCalls != 1 {
318+
t.Fatalf("expected one terminate before retry, got %d", provider.terminateCalls)
319+
}
320+
}

‎internal/pullpreview/types.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,12 @@ type UserDataOptions struct {
6060
}
6161

6262
type LaunchOptions struct {
63-
Size string
64-
UserData string
65-
Ports []string
66-
CIDRs []string
67-
Tags map[string]string
63+
Size string
64+
UserData string
65+
Ports []string
66+
CIDRs []string
67+
Tags map[string]string
68+
SkipRestore bool
6869
}
6970

7071
type InstanceSummary struct {

0 commit comments

Comments
 (0)