Skip to content

Commit

Permalink
Fix TestEphemeralAccountSync NDF (#1622)
Browse files Browse the repository at this point in the history
Test ran 6*50 times without failing with this fix.

NDF that was fixed: `cluster_test.go:1386: account shouldn't be marked
as clean shutdown or not require a sync, got true false`

Example:
https://github.com/SiaFoundation/renterd/actions/runs/11496423699/job/31998008008

The problem was that after the restart of the cluster, when we first
fetch the account, the account gets created and sometimes synced (since
created accounts are unclean and need to be synced).
Whenever it got synced before the check the test failed.

While debugging I noticed that the GET route for fetching accounts only
returns the account id which I thought was odd anyway so I updated it to
return the full account. So the route returns the freshly created,
unclean account right away without a chance to sync it. Which fixes the
NDF.
  • Loading branch information
ChrisSchinnerl authored Oct 25, 2024
1 parent 609d9c3 commit cbb8f84
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 15 deletions.
3 changes: 1 addition & 2 deletions autopilot/workerpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ import (
"sync"
"time"

rhpv3 "go.sia.tech/core/rhp/v3"
"go.sia.tech/core/types"
"go.sia.tech/renterd/api"
"go.sia.tech/renterd/object"
"lukechampine.com/frand"
)

type Worker interface {
Account(ctx context.Context, hostKey types.PublicKey) (rhpv3.Account, error)
Account(ctx context.Context, hostKey types.PublicKey) (api.Account, error)
Contracts(ctx context.Context, hostTimeout time.Duration) (api.ContractsResponse, error)
ID(ctx context.Context) (string, error)
MigrateSlab(ctx context.Context, s object.Slab, set string) error
Expand Down
19 changes: 9 additions & 10 deletions internal/test/e2e/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1373,28 +1373,27 @@ func TestEphemeralAccountSync(t *testing.T) {
cluster.sync()

// ask for the account, this should trigger its creation
tt.OKAll(cluster.Worker.Account(context.Background(), hk))
account, err := cluster.Worker.Account(context.Background(), hk)
tt.OK(err)
if account.ID != acc.ID {
t.Fatalf("account ID mismatch, expected %v got %v", acc.ID, account.ID)
} else if account.CleanShutdown || !account.RequiresSync {
t.Fatalf("account shouldn't be marked as clean shutdown or not require a sync, got %v %v", account.CleanShutdown, accounts[0].RequiresSync)
}

// make sure we form a contract
cluster.WaitForContracts()
cluster.MineBlocks(1)

accounts = cluster.Accounts()
if len(accounts) != 1 || accounts[0].ID != acc.ID {
t.Fatal("account should exist")
} else if accounts[0].CleanShutdown || !accounts[0].RequiresSync {
t.Fatal("account shouldn't be marked as clean shutdown or not require a sync, got", accounts[0].CleanShutdown, accounts[0].RequiresSync)
}

// assert account was funded
tt.Retry(100, 100*time.Millisecond, func() error {
accounts = cluster.Accounts()
if len(accounts) != 1 || accounts[0].ID != acc.ID {
return errors.New("account should exist")
} else if accounts[0].Balance.Cmp(types.ZeroCurrency.Big()) == 0 {
return errors.New("account isn't funded")
} else if accounts[0].RequiresSync {
return fmt.Errorf("account shouldn't require a sync, got %v", accounts[0].RequiresSync)
} else if !accounts[0].CleanShutdown || accounts[0].RequiresSync {
return fmt.Errorf("account should be marked as clean shutdown and not require a sync, got %v %v", accounts[0].CleanShutdown, accounts[0].RequiresSync)
}
return nil
})
Expand Down
2 changes: 1 addition & 1 deletion worker/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func New(addr, password string) *Client {
}

// Account returns the account id for a given host.
func (c *Client) Account(ctx context.Context, hostKey types.PublicKey) (account rhpv3.Account, err error) {
func (c *Client) Account(ctx context.Context, hostKey types.PublicKey) (account api.Account, err error) {
err = c.c.WithContext(ctx).GET(fmt.Sprintf("/account/%s", hostKey), &account)
return
}
Expand Down
3 changes: 1 addition & 2 deletions worker/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -840,8 +840,7 @@ func (w *Worker) accountHandlerGET(jc jape.Context) {
if jc.DecodeParam("hostkey", &hostKey) != nil {
return
}
account := rhpv3.Account(w.accounts.ForHost(hostKey).ID())
jc.Encode(account)
jc.Encode(w.accounts.Account(hostKey))
}

func (w *Worker) accountsHandlerGET(jc jape.Context) {
Expand Down

0 comments on commit cbb8f84

Please sign in to comment.