From cbb8f84ef94fe1150660d112144bfbb699f464d7 Mon Sep 17 00:00:00 2001 From: Christopher Schinnerl Date: Fri, 25 Oct 2024 09:51:25 +0200 Subject: [PATCH] Fix `TestEphemeralAccountSync` NDF (#1622) 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. --- autopilot/workerpool.go | 3 +-- internal/test/e2e/cluster_test.go | 19 +++++++++---------- worker/client/client.go | 2 +- worker/worker.go | 3 +-- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/autopilot/workerpool.go b/autopilot/workerpool.go index 54617aee9..000bc9860 100644 --- a/autopilot/workerpool.go +++ b/autopilot/workerpool.go @@ -5,7 +5,6 @@ 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" @@ -13,7 +12,7 @@ import ( ) 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 diff --git a/internal/test/e2e/cluster_test.go b/internal/test/e2e/cluster_test.go index 80a77f751..31eeeae36 100644 --- a/internal/test/e2e/cluster_test.go +++ b/internal/test/e2e/cluster_test.go @@ -1373,19 +1373,18 @@ 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() @@ -1393,8 +1392,8 @@ func TestEphemeralAccountSync(t *testing.T) { 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 }) diff --git a/worker/client/client.go b/worker/client/client.go index 6a64fc31b..0063938d6 100644 --- a/worker/client/client.go +++ b/worker/client/client.go @@ -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 } diff --git a/worker/worker.go b/worker/worker.go index 074680cf2..bd95d4def 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -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) {