rofl: Replace expired machines still returned by the market

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 == "" {

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)
+				}
+			}
+		})
+	}
+}

docs/rofl.md

@@ -275,7 +275,9 @@ offer:
   available provider term by default.
 - `--term-count <number>` 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)