diff --git a/cmd/rofl/deploy.go b/cmd/rofl/deploy.go index 6c188e70..44b1fa79 100644 --- a/cmd/rofl/deploy.go +++ b/cmd/rofl/deploy.go @@ -291,19 +291,15 @@ var ( var insDsc *roflmarket.Instance insDsc, err = conn.Runtime(npa.ParaTime).ROFLMarket.Instance(ctx, client.RoundLatest, *providerAddr, machineID) - // The "instance not found" error originates from Rust code, - // so we can't compare it nicely here. - if err != nil && strings.Contains(err.Error(), "instance not found") { - if deployReplaceMachine { - fmt.Printf("Machine instance not found. Obtaining new one...") - machine.ID = "" - _, insDsc, err = obtainMachine() - return insDsc, err - } - - cobra.CheckErr("Machine instance not found.\nTip: If your instance expired, run this command with --replace-machine flag to replace it with a new machine.") + insDsc, replaced, err := replaceUnavailableMachine(insDsc, err, time.Now(), deployReplaceMachine, func() (*roflmarket.Instance, error) { + fmt.Printf("Machine instance expired or not found. Obtaining new one...\n") + machine.ID = "" + _, replacement, obtainErr := obtainMachine() + return replacement, obtainErr + }) + if err != nil || replaced { + return insDsc, err } - cobra.CheckErr(err) if insDsc.Deployment != nil && insDsc.Deployment.AppID != machineDeployment.AppID && !deployForce { fmt.Printf("Machine already contains a deployment of ROFL app '%s'.\n", insDsc.Deployment.AppID) @@ -361,6 +357,39 @@ var ( } ) +// replaceUnavailableMachine checks the paid period independently of the instance status. +// A provider may not have removed an expired instance yet. +func replaceUnavailableMachine( + instance *roflmarket.Instance, + queryErr error, + now time.Time, + replace bool, + obtain func() (*roflmarket.Instance, error), +) (*roflmarket.Instance, bool, error) { + // Compare Unix seconds directly to avoid overflowing time.Time for large paid periods. + nowUnix := now.Unix() + var reason string + switch { + case queryErr != nil: + // The "instance not found" error originates from Rust code, + // so we can't compare it nicely here. + if !strings.Contains(queryErr.Error(), "instance not found") { + return nil, false, queryErr + } + reason = "not found" + case nowUnix >= 0 && uint64(nowUnix) >= instance.PaidUntil: + reason = "expired" + default: + return instance, false, nil + } + + if !replace { + return nil, false, fmt.Errorf("machine instance %s; use --replace-machine to rent a new machine", reason) + } + instance, err := obtain() + return instance, true, err +} + // ociRepository returns the OCI repository for the deployment (or a default one if not set). func ociRepository(deployment *buildRofl.Deployment) string { if deployment.OCIRepository == "" { diff --git a/cmd/rofl/deploy_test.go b/cmd/rofl/deploy_test.go new file mode 100644 index 00000000..d6f63d4e --- /dev/null +++ b/cmd/rofl/deploy_test.go @@ -0,0 +1,90 @@ +package rofl + +import ( + "errors" + "math" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/oasisprotocol/oasis-sdk/client-sdk/go/modules/roflmarket" +) + +func TestReplaceUnavailableMachine(t *testing.T) { + queryFailure := errors.New("query failed") + rentFailure := errors.New("rental failed") + missing := errors.New("roflmarket: instance not found") + // time.Time stores seconds relative to year 1 instead of the Unix epoch. + const unixToInternalSeconds = 62135596800 + for _, tc := range []struct { + name string + paidUntil uint64 + now time.Time + queryErr error + replace bool + rentErr error + wantRent bool + wantErr error + wantErrMsg string + }{ + {name: "unexpired", paidUntil: 100, now: time.Unix(99, 999999999), replace: true}, + {name: "unexpired without replacement", paidUntil: 100, now: time.Unix(99, 0)}, + {name: "before internal time overflow", paidUntil: math.MaxInt64 - unixToInternalSeconds - 1, now: time.Unix(101, 0), replace: true}, + {name: "internal time overflow boundary", paidUntil: math.MaxInt64 - unixToInternalSeconds, now: time.Unix(101, 0), replace: true}, + {name: "after internal time overflow", paidUntil: math.MaxInt64 - unixToInternalSeconds + 1, now: time.Unix(101, 0), replace: true}, + {name: "maximum signed timestamp", paidUntil: math.MaxInt64, now: time.Unix(101, 0), replace: true}, + {name: "before epoch", paidUntil: 100, now: time.Unix(-1, 999999999), replace: true}, + {name: "zero timestamp before epoch", paidUntil: 0, now: time.Unix(-1, 999999999), replace: true}, + {name: "zero timestamp at epoch", paidUntil: 0, now: time.Unix(0, 0), replace: true, wantRent: true}, + {name: "zero timestamp after epoch", paidUntil: 0, now: time.Unix(0, 1), wantErrMsg: "machine instance expired; use --replace-machine"}, + {name: "beyond signed timestamp range", paidUntil: math.MaxInt64 + 1, now: time.Unix(101, 0), replace: true}, + {name: "maximum timestamp", paidUntil: math.MaxUint64, now: time.Unix(101, 0)}, + {name: "expiration boundary", paidUntil: 100, now: time.Unix(100, 0), replace: true, wantRent: true}, + {name: "expired accepted", paidUntil: 100, now: time.Unix(101, 0), replace: true, wantRent: true}, + {name: "expired without replacement", paidUntil: 100, now: time.Unix(101, 0), wantErrMsg: "machine instance expired; use --replace-machine"}, + {name: "missing", paidUntil: 100, queryErr: missing, replace: true, wantRent: true}, + {name: "missing without replacement", paidUntil: 100, queryErr: missing, wantErrMsg: "machine instance not found; use --replace-machine"}, + {name: "query failure", paidUntil: 100, queryErr: queryFailure, replace: true, wantErr: queryFailure}, + {name: "expired rental failure", paidUntil: 100, now: time.Unix(100, 0), replace: true, rentErr: rentFailure, wantRent: true, wantErr: rentFailure}, + {name: "missing rental failure", paidUntil: 100, queryErr: missing, replace: true, rentErr: rentFailure, wantRent: true, wantErr: rentFailure}, + } { + t.Run(tc.name, func(t *testing.T) { + original := &roflmarket.Instance{PaidUntil: tc.paidUntil, Status: roflmarket.InstanceStatusAccepted} + instance := original + if tc.queryErr != nil { + instance = nil + } + replacement := &roflmarket.Instance{PaidUntil: 200} + rentals := 0 + got, replaced, err := replaceUnavailableMachine(instance, tc.queryErr, tc.now, tc.replace, func() (*roflmarket.Instance, error) { + rentals++ + if tc.rentErr != nil { + return nil, tc.rentErr + } + return replacement, nil + }) + require.Equal(t, tc.wantRent, replaced) + if tc.wantRent { + require.Equal(t, 1, rentals) + } else { + require.Zero(t, rentals) + } + switch { + case tc.wantErr != nil: + require.ErrorIs(t, err, tc.wantErr) + require.Nil(t, got) + case tc.wantErrMsg != "": + require.ErrorContains(t, err, tc.wantErrMsg) + require.Nil(t, got) + default: + require.NoError(t, err) + if tc.wantRent { + require.Same(t, replacement, got) + } else { + require.Same(t, original, got) + } + } + }) + } +} diff --git a/docs/rofl.md b/docs/rofl.md index ab399693..e9955d3d 100644 --- a/docs/rofl.md +++ b/docs/rofl.md @@ -275,7 +275,9 @@ offer: available provider term by default. - `--term-count ` specifies the multiplier. Default is `1`. - `--replace-machine` if the existing machine in the manifest file expired, - rent a new machine and replace the existing machine ID. + rent a new machine and replace the existing machine ID. This also applies + when an expired machine is still listed as accepted. Without this flag, + deploying to an expired machine fails. An unexpired machine is reused. ![code shell](../examples/rofl/deploy.in.static)