Skip to content

Commit 0e0ef11

Browse files
dangradangra
authored
Merge a7d4657 into c12bb49 
2 parents c12bb49 + a7d4657 commit 0e0ef11

File tree

10 files changed

+93
-154
lines changed

10 files changed

+93
-154
lines changed

‎internal/command/deploy/machines_launchinput.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package deploy
22

33
import (
44
"fmt"
5+
"slices"
56
"strconv"
67
"strings"
78

@@ -297,18 +298,14 @@ func skipLaunch(origMachineRaw *fly.Machine, mConfig *fly.MachineConfig) bool {
297298
}
298299

299300
switch {
300-
case state == fly.MachineStateStarted:
301+
case slices.Contains([]string{fly.MachineStateStarted, "starting", "failed"}, state):
301302
return false
302303
case len(mConfig.Standbys) > 0:
303304
return true
304-
case state == fly.MachineStateStopped, state == fly.MachineStateSuspended:
305-
for _, s := range mConfig.Services {
306-
if (s.Autostop != nil && *s.Autostop != fly.MachineAutostopOff) || (s.Autostart != nil && *s.Autostart) {
307-
return true
308-
}
309-
}
305+
case origMachineRaw == nil:
306+
return false
310307
}
311-
return false
308+
return true
312309
}
313310

314311
// updateContainerImage sets container.Image = mConfig.Image in any container where image == "."

‎internal/command/deploy/machines_launchinput_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
)
1616

1717
func makeTerminalLoggerQuiet(tb testing.TB) {
18-
var originalLogger = terminal.DefaultLogger
18+
originalLogger := terminal.DefaultLogger
1919
terminal.DefaultLogger = logger.New(os.Stdout, logger.Error, true)
2020

2121
tb.Cleanup(func() {
@@ -85,6 +85,7 @@ func testLaunchInputForBasic(t *testing.T) {
8585
Region: li.Region,
8686
Config: helpers.Clone(li.Config),
8787
HostStatus: fly.HostStatusOk,
88+
State: fly.MachineStateStarted,
8889
}
8990
// also must preserve any user's added metadata except for known fly metadata keys
9091
origMachineRaw.Config.Metadata["user-added-me"] = "keep it"
@@ -104,6 +105,7 @@ func testLaunchInputForBasic(t *testing.T) {
104105
Region: li.Region,
105106
Config: helpers.Clone(li.Config),
106107
HostStatus: fly.HostStatusOk,
108+
State: fly.MachineStateStarted,
107109
}
108110
want.Config.Image = "super/globe"
109111
want.Config.Env["NOT_SET_ON_RESTART_ONLY"] = "true"
@@ -253,7 +255,6 @@ func testLaunchInputForOnMounts(t *testing.T) {
253255
assert.Equal(t, "ab1234567890", li.ID)
254256
assert.True(t, li.RequiresReplacement)
255257
assert.Empty(t, li.Config.Mounts)
256-
257258
}
258259

259260
// test mounts with auto volume resize

‎internal/command/deploy/machines_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ func Test_resolveUpdatedMachineConfig_Mounts(t *testing.T) {
279279
}, li)
280280

281281
origMachine := &fly.Machine{
282+
State: fly.MachineStateStarted,
282283
HostStatus: fly.HostStatusOk,
283284
Config: &fly.MachineConfig{
284285
Mounts: []fly.MachineMount{{
@@ -329,6 +330,7 @@ func Test_resolveUpdatedMachineConfig_restartOnly(t *testing.T) {
329330
md.img = "SHOULD-NOT-USE-THIS-TAG"
330331

331332
origMachine := &fly.Machine{
333+
State: fly.MachineStateStarted,
332334
HostStatus: fly.HostStatusOk,
333335
ID: "OrigID",
334336
Config: &fly.MachineConfig{
@@ -371,6 +373,7 @@ func Test_resolveUpdatedMachineConfig_restartOnlyProcessGroup(t *testing.T) {
371373
md.img = "SHOULD-NOT-USE-THIS-TAG"
372374

373375
origMachine := &fly.Machine{
376+
State: fly.MachineStateStarted,
374377
HostStatus: fly.HostStatusOk,
375378
ID: "OrigID",
376379
Config: &fly.MachineConfig{

‎internal/command/machine/update.go‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/superfly/flyctl/internal/flag"
1717
"github.com/superfly/flyctl/internal/flyerr"
1818
mach "github.com/superfly/flyctl/internal/machine"
19-
"github.com/superfly/flyctl/internal/watch"
2019
)
2120

2221
func newUpdate() *cobra.Command {
@@ -79,9 +78,7 @@ func newUpdate() *cobra.Command {
7978

8079
func runUpdate(ctx context.Context) (err error) {
8180
var (
82-
io = iostreams.FromContext(ctx)
83-
colorize = io.ColorScheme()
84-
81+
io = iostreams.FromContext(ctx)
8582
autoConfirm = flag.GetBool(ctx, "yes")
8683
skipHealthChecks = flag.GetBool(ctx, "skip-health-checks")
8784
skipStart = flag.GetBool(ctx, "skip-start")
@@ -169,20 +166,10 @@ func runUpdate(ctx context.Context) (err error) {
169166
Descript: timeoutErr.Description(),
170167
Suggest: "Try increasing the --wait-timeout",
171168
}
172-
173169
}
174170
return err
175171
}
176172

177-
if !(input.SkipLaunch || flag.GetDetach(ctx)) {
178-
fmt.Fprintln(io.Out, colorize.Green("==> "+"Monitoring health checks"))
179-
180-
if err := watch.MachinesChecks(ctx, appName, []*fly.Machine{machine}); err != nil {
181-
return err
182-
}
183-
fmt.Fprintln(io.Out)
184-
}
185-
186173
fmt.Fprintf(io.Out, "\nMonitor machine status here:\nhttps://fly.io/apps/%s/machines/%s\n", appName, machine.ID)
187174

188175
return nil

‎internal/machine/update.go‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,25 +85,19 @@ func Update(ctx context.Context, appName string, m *fly.Machine, input *fly.Laun
8585
return fmt.Errorf("could not update machine %s: %w", m.ID, err)
8686
}
8787

88-
waitForAction := "start"
89-
if input.SkipLaunch || m.Config.Schedule != "" || m.State != fly.MachineStateStarted {
90-
waitForAction = "stop"
91-
}
92-
9388
waitTimeout := time.Second * 300
9489
if input.Timeout != 0 {
9590
waitTimeout = time.Duration(input.Timeout) * time.Second
9691
}
9792

98-
if err := WaitForStartOrStop(ctx, appName, updatedMachine, waitForAction, waitTimeout); err != nil {
99-
return err
93+
state, err := WaitForState(ctx, appName, updatedMachine, "settled", waitTimeout)
94+
if err != nil {
95+
return fmt.Errorf("error while waiting for machine to update: %w", err)
10096
}
10197

102-
if !input.SkipLaunch {
103-
if !input.SkipHealthChecks {
104-
if err := watch.MachinesChecks(ctx, appName, []*fly.Machine{updatedMachine}); err != nil {
105-
return fmt.Errorf("failed to wait for health checks to pass: %w", err)
106-
}
98+
if state == "started" && !input.SkipHealthChecks {
99+
if err := watch.MachinesChecks(ctx, appName, []*fly.Machine{updatedMachine}); err != nil {
100+
return fmt.Errorf("failed to wait for health checks to pass: %w", err)
107101
}
108102
}
109103

‎internal/machine/wait.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ import (
1515
"github.com/superfly/flyctl/internal/flyerr"
1616
)
1717

18+
func WaitForState(ctx context.Context, appName string, machine *fly.Machine, desiredState string, timeout time.Duration) (string, error) {
19+
flapsClient := flapsutil.ClientFromContext(ctx)
20+
21+
if err := WaitForStartOrStop(ctx, appName, machine, desiredState, timeout); err != nil {
22+
return "", err
23+
}
24+
if desiredState == "settled" {
25+
m, err := flapsClient.Get(ctx, appName, machine.ID)
26+
if err != nil {
27+
return "", fmt.Errorf("failed to get machine after waiting for it to settle: %w", err)
28+
}
29+
return m.State, nil
30+
}
31+
return desiredState, nil
32+
}
33+
1834
func WaitForStartOrStop(ctx context.Context, appName string, machine *fly.Machine, action string, timeout time.Duration) error {
1935
flapsClient := flapsutil.ClientFromContext(ctx)
2036

@@ -27,6 +43,8 @@ func WaitForStartOrStop(ctx context.Context, appName string, machine *fly.Machin
2743
waitOnAction = "started"
2844
case "stop":
2945
waitOnAction = "stopped"
46+
case "settled":
47+
waitOnAction = "settled"
3048
default:
3149
return invalidAction
3250
}

‎test/preflight/apps_v2_integration_test.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestAppsV2Example(t *testing.T) {
8484
ENV BUILT_BY_DOCKERFILE=true
8585
`
8686
dockerfilePath := filepath.Join(f.WorkDir(), "Dockerfile")
87-
err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0644)
87+
err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0o644)
8888
if err != nil {
8989
f.Fatalf("failed to write dockerfile at %s error: %v", dockerfilePath, err)
9090
}
@@ -109,7 +109,7 @@ func TestAppsV2ConfigChanges(t *testing.T) {
109109
newConfigFile := strings.Replace(string(configFileBytes), `FOO = 'BAR'`, `BAR = "QUX"`, 1)
110110
require.Contains(f, newConfigFile, `BAR = "QUX"`)
111111

112-
err = os.WriteFile(configFilePath, []byte(newConfigFile), 0666)
112+
err = os.WriteFile(configFilePath, []byte(newConfigFile), 0o666)
113113
require.NoError(t, err)
114114

115115
f.Fly("deploy --buildkit --remote-only --detach")
@@ -177,7 +177,7 @@ func TestAppsV2Config_ParseExperimental(t *testing.T) {
177177
auto_rollback = true
178178
`
179179

180-
err := os.WriteFile(configFilePath, []byte(config), 0644)
180+
err := os.WriteFile(configFilePath, []byte(config), 0o644)
181181
require.NoError(t, err, "error trying to write %s", configFilePath)
182182

183183
result := f.Fly("launch --no-deploy --ha=false --name %s --region ord --copy-config --org %s", appName, f.OrgSlug())
@@ -208,7 +208,7 @@ func TestAppsV2Config_ProcessGroups(t *testing.T) {
208208

209209
deployToml := func(toml string) *testlib.FlyctlResult {
210210
toml = "app = \"" + appName + "\"\n" + toml
211-
err := os.WriteFile(configFilePath, []byte(toml), 0666)
211+
err := os.WriteFile(configFilePath, []byte(toml), 0o666)
212212
require.NoError(t, err, "error trying to write %s", configFilePath)
213213
cmd := f.Fly("deploy --buildkit --remote-only --detach --now --image nginx --ha=false")
214214
cmd.AssertSuccessfulExit()
@@ -252,10 +252,8 @@ func TestAppsV2Config_ProcessGroups(t *testing.T) {
252252

253253
deployOut := deployToml(`
254254
[[services]]
255-
http_checks = []
256255
internal_port = 8080
257256
protocol = "tcp"
258-
script_checks = []
259257
260258
[[services.ports]]
261259
port = 80
@@ -279,10 +277,8 @@ bar_web = "bash -c 'while true; do sleep 10; done'"
279277
280278
[[services]]
281279
processes = ["web"] # this service only applies to the web process
282-
http_checks = []
283280
internal_port = 8080
284281
protocol = "tcp"
285-
script_checks = []
286282
287283
[[services.ports]]
288284
port = 80
@@ -320,6 +316,7 @@ bar_web = "bash -c 'while true; do sleep 10; done'"
320316
if len(f.OtherRegions()) > 0 {
321317
secondaryRegion = f.OtherRegions()[0]
322318
}
319+
323320
f.Fly("m clone %s --region %s", barWebMachId, secondaryRegion)
324321
f.Fly("machine update %s -m ABCD=EFGH -y", webMachId).AssertSuccessfulExit()
325322

@@ -512,7 +509,7 @@ func TestImageLabel(t *testing.T) {
512509
ENV BUILT_BY_DOCKERFILE=true
513510
`
514511
dockerfilePath := filepath.Join(f.WorkDir(), "Dockerfile")
515-
err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0644)
512+
err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0o644)
516513
if err != nil {
517514
f.Fatalf("failed to write dockerfile at %s error: %v", dockerfilePath, err)
518515
}

‎test/preflight/fly_deploy_test.go‎

Lines changed: 7 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
"github.com/stretchr/testify/assert"
1919
"github.com/stretchr/testify/require"
2020

21-
//fly "github.com/superfly/fly-go"
21+
// fly "github.com/superfly/fly-go"
2222

2323
"github.com/superfly/flyctl/test/preflight/testlib"
2424
)
@@ -84,36 +84,11 @@ func TestFlyDeployHAPlacement(t *testing.T) {
8484
f := testlib.NewTestEnvFromEnv(t)
8585
appName := f.CreateRandomAppName()
8686

87-
// Create the app without deploying to avoid the Corrosion replication race
8887
f.Fly(
89-
"launch --org %s --name %s --region %s --image nginx --internal-port 80 --no-deploy",
88+
"launch --org %s --name %s --region %s --image nginx --internal-port 80 --ha",
9089
f.OrgSlug(), appName, f.PrimaryRegion(),
9190
)
9291

93-
// Retry the deploy command to handle Corrosion replication lag race conditions
94-
// The backend may not have replicated the app record to all hosts yet when
95-
// creating the second machine for HA, resulting in "sql: no rows in result set" errors
96-
var lastError string
97-
require.EventuallyWithT(t, func(c *assert.CollectT) {
98-
result := f.FlyAllowExitFailure("deploy --buildkit --remote-only")
99-
if result.ExitCode() != 0 {
100-
stderr := result.StdErrString()
101-
lastError = stderr
102-
// Only retry if it's the known Corrosion replication lag error
103-
if strings.Contains(stderr, "failed to get app: sql: no rows in result set") {
104-
t.Logf("Corrosion replication lag detected, retrying... (error: %s)", stderr)
105-
assert.Fail(c, "Corrosion replication lag, retrying...")
106-
} else {
107-
// Log the unexpected error and fail without retrying
108-
t.Logf("Deploy failed with unexpected error (will not retry): %s", stderr)
109-
assert.Fail(c, fmt.Sprintf("deploy failed with unexpected error: %s", stderr))
110-
}
111-
} else {
112-
// Explicitly assert success so EventuallyWithT knows we passed
113-
assert.True(c, true, "deploy succeeded")
114-
}
115-
}, 30*time.Second, 5*time.Second, "deploy should succeed after Corrosion replication, last error: %s", lastError)
116-
11792
assertHostDistribution(t, f, appName, 2)
11893
}
11994

@@ -241,35 +216,10 @@ func testDeployNodeAppWithRemoteBuilder(tt *testing.T) {
241216
require.NoError(t, err)
242217

243218
t.Logf("deploy %s", appName)
244-
// Retry deploy to handle transient network errors (DNS, WireGuard, buildkit connection issues)
245-
// BuildKit deployments can fail with various transient errors during the initial connection
246-
var lastError string
247-
require.EventuallyWithT(tt, func(c *assert.CollectT) {
248-
result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --ha=false")
249-
if result.ExitCode() != 0 {
250-
stderr := result.StdErrString()
251-
lastError = stderr
252-
t.Logf("Deploy failed (will retry), error: %s", stderr)
253-
assert.Fail(c, "deploy failed, retrying...")
254-
} else {
255-
assert.True(c, true, "deploy succeeded")
256-
}
257-
}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
219+
f.Fly("deploy --remote-only --ha=false")
258220

259221
t.Logf("deploy %s again", appName)
260-
// Retry second deploy as well
261-
lastError = ""
262-
require.EventuallyWithT(tt, func(c *assert.CollectT) {
263-
result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --strategy immediate --ha=false")
264-
if result.ExitCode() != 0 {
265-
stderr := result.StdErrString()
266-
lastError = stderr
267-
t.Logf("Deploy failed (will retry), error: %s", stderr)
268-
assert.Fail(c, "deploy failed, retrying...")
269-
} else {
270-
assert.True(c, true, "deploy succeeded")
271-
}
272-
}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
222+
f.Fly("deploy --remote-only --strategy immediate")
273223

274224
body, err := testlib.RunHealthCheck(fmt.Sprintf("https://%s.fly.dev", appName))
275225
require.NoError(t, err)
@@ -298,35 +248,10 @@ func testDeployNodeAppWithBuildKitRemoteBuilder(tt *testing.T) {
298248
require.NoError(t, err)
299249

300250
t.Logf("deploy %s with BuildKit", appName)
301-
// Retry deploy to handle transient network errors (DNS, WireGuard, buildkit connection issues)
302-
// BuildKit deployments can fail with various transient errors during the initial connection
303-
var lastError string
304-
require.EventuallyWithT(tt, func(c *assert.CollectT) {
305-
result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --ha=false")
306-
if result.ExitCode() != 0 {
307-
stderr := result.StdErrString()
308-
lastError = stderr
309-
t.Logf("Deploy failed (will retry), error: %s", stderr)
310-
assert.Fail(c, "deploy failed, retrying...")
311-
} else {
312-
assert.True(c, true, "deploy succeeded")
313-
}
314-
}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
251+
f.Fly("deploy --buildkit --remote-only --ha=false")
315252

316253
t.Logf("deploy %s again with BuildKit", appName)
317-
// Retry second deploy as well
318-
lastError = ""
319-
require.EventuallyWithT(tt, func(c *assert.CollectT) {
320-
result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --strategy immediate --ha=false")
321-
if result.ExitCode() != 0 {
322-
stderr := result.StdErrString()
323-
lastError = stderr
324-
t.Logf("Deploy failed (will retry), error: %s", stderr)
325-
assert.Fail(c, "deploy failed, retrying...")
326-
} else {
327-
assert.True(c, true, "deploy succeeded")
328-
}
329-
}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
254+
f.Fly("deploy --buildkit --remote-only --strategy immediate")
330255

331256
body, err := testlib.RunHealthCheck(fmt.Sprintf("https://%s.fly.dev", appName))
332257
require.NoError(t, err)
@@ -461,7 +386,7 @@ func TestDeployManifest(t *testing.T) {
461386
appName := f.CreateRandomAppName()
462387
f.Fly("launch --org %s --name %s --region %s --image nginx:latest --internal-port 80 --ha=false --strategy rolling", f.OrgSlug(), appName, f.PrimaryRegion())
463388

464-
var manifestPath = filepath.Join(f.WorkDir(), "manifest.json")
389+
manifestPath := filepath.Join(f.WorkDir(), "manifest.json")
465390

466391
f.Fly("deploy --buildkit --remote-only --export-manifest %s", manifestPath)
467392

0 commit comments

Comments
 (0)