diff --git a/internal/provider/server_resource.go b/internal/provider/server_resource.go index cab4103..340c13b 100644 --- a/internal/provider/server_resource.go +++ b/internal/provider/server_resource.go @@ -1361,28 +1361,36 @@ func (r *serverResource) ImportState( resp.Diagnostics.Append(diags...) } +var ( + serverActionPollInterval = 5 * time.Second + serverActionTimeout = 20 * time.Minute +) + func (r *serverResource) waitForServerAction(ctx context.Context, serverId int64, actionId int64) error { + if _, ok := ctx.Deadline(); !ok { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, serverActionTimeout) + defer cancel() + } + var lastReadyResp *binarylane.GetServersServerIdActionsActionIdResponse for { - select { - case <-ctx.Done(): - if lastReadyResp == nil { - return fmt.Errorf("timed out waiting for server action: server_id=%d, action_id=%d", serverId, actionId) - } else { - return fmt.Errorf("timed out waiting for server action: server_id=%d, action_id=%d, last response was status=%s, body: %s", - serverId, actionId, lastReadyResp.Status(), lastReadyResp.Body) + readyResp, err := r.bc.client.GetServersServerIdActionsActionIdWithResponse(ctx, serverId, actionId) + if err != nil && ctx.Err() == nil { + return fmt.Errorf("unexpected error waiting for server action: server_id=%d, action_id=%d, error: %w", serverId, actionId, err) + } + if err == nil { + if readyResp.StatusCode() == http.StatusNotFound { + return fmt.Errorf("server action not found: server_id=%d, action_id=%d, body: %s", serverId, actionId, readyResp.Body) } - default: - readyResp, err := r.bc.client.GetServersServerIdActionsActionIdWithResponse(ctx, serverId, actionId) - if err != nil { - return fmt.Errorf("unexpected error waiting for server action: server_id=%d, action_id=%d, error: %w", serverId, actionId, err) - } - if readyResp.StatusCode() == http.StatusOK && readyResp.JSON200.Action.Status == binarylane.Errored { - return fmt.Errorf("server action failed to with error: server_id=%d, action_id=%d, error: %s", serverId, actionId, readyResp.Body) - } - if readyResp.StatusCode() == http.StatusOK && readyResp.JSON200.Action.CompletedAt != nil { - return nil + if readyResp.StatusCode() == http.StatusOK { + switch { + case readyResp.JSON200.Action.Status == binarylane.Errored: + return fmt.Errorf("server action failed to with error: server_id=%d, action_id=%d, error: %s", serverId, actionId, readyResp.Body) + case readyResp.JSON200.Action.Status == binarylane.Completed, readyResp.JSON200.Action.CompletedAt != nil: + return nil + } } lastReadyResp = readyResp tflog.Debug(ctx, @@ -1391,7 +1399,16 @@ func (r *serverResource) waitForServerAction(ctx context.Context, serverId int64 ), ) } - time.Sleep(time.Second * 5) + + select { + case <-ctx.Done(): + if lastReadyResp == nil { + return fmt.Errorf("timed out waiting for server action: server_id=%d, action_id=%d", serverId, actionId) + } + return fmt.Errorf("timed out waiting for server action: server_id=%d, action_id=%d, last response was status=%s, body: %s", + serverId, actionId, lastReadyResp.Status(), lastReadyResp.Body) + case <-time.After(serverActionPollInterval): + } } } diff --git a/internal/provider/server_reverse_name_resource.go b/internal/provider/server_reverse_name_resource.go index ff792f1..c385af7 100644 --- a/internal/provider/server_reverse_name_resource.go +++ b/internal/provider/server_reverse_name_resource.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" "terraform-provider-binarylane/internal/binarylane" + "time" "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" @@ -262,17 +263,40 @@ func (r *serverReverseNameResource) changeReverseName(ctx context.Context, serve if actionResp.StatusCode() != http.StatusOK && actionResp.StatusCode() != http.StatusAccepted { return fmt.Errorf("unexpected HTTP status code changing reverse name for server: server_id=%d, details: %s", serverId, actionResp.Body) } - // 202 Accepted carries no action ID to poll - if actionResp.JSON200 == nil { + // The change_reverse_name action can stay unresolvable, so wait for the result on the server instead + if reverseName == nil { return nil } + return r.waitForReverseName(ctx, serverId, ipv4Address, *reverseName) +} - sr := serverResource{bc: r.bc} - if err := sr.waitForServerAction(ctx, serverId, actionResp.JSON200.Action.Id); err != nil { - return fmt.Errorf("error changing reverse name: %w", err) +func (r *serverReverseNameResource) waitForReverseName(ctx context.Context, serverId int64, ipv4Address string, want string) error { + ctx, cancel := context.WithTimeout(ctx, serverActionTimeout) + defer cancel() + + var last string + for { + serverResp, err := r.bc.client.GetServersServerIdWithResponse(ctx, serverId) + if err == nil && serverResp.StatusCode() == http.StatusOK { + for _, n := range serverResp.JSON200.Server.Networks.V4 { + if n.IpAddress != ipv4Address { + continue + } + if n.ReverseName != nil { + if *n.ReverseName == want { + return nil + } + last = *n.ReverseName + } + } + } + + select { + case <-ctx.Done(): + return fmt.Errorf("timed out waiting for reverse name: server_id=%d, ipv4_address=%s, want=%q, last=%q", serverId, ipv4Address, want, last) + case <-time.After(serverActionPollInterval): + } } - - return nil } func formatServerReverseNameId(serverId int64, ipv4Address string) string { diff --git a/internal/provider/server_reverse_name_resource_unit_test.go b/internal/provider/server_reverse_name_resource_unit_test.go index 5f5cb51..d50ee67 100644 --- a/internal/provider/server_reverse_name_resource_unit_test.go +++ b/internal/provider/server_reverse_name_resource_unit_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "terraform-provider-binarylane/internal/binarylane" @@ -40,8 +41,6 @@ func TestServerReverseNameResourceUnit(t *testing.T) { if postStatus == http.StatusOK { _, _ = w.Write([]byte(`{"action":{"id":9,"status":"in-progress"}}`)) } - case r.Method == http.MethodGet && r.URL.Path == "/servers/1/actions/9": - _, _ = w.Write([]byte(`{"action":{"id":9,"status":"completed","completed_at":"2026-01-01T00:00:00Z"}}`)) case r.Method == http.MethodGet && r.URL.Path == "/servers/1": if !serverFound { w.WriteHeader(http.StatusNotFound) @@ -120,6 +119,7 @@ func TestServerReverseNameResourceUnit(t *testing.T) { t.Fatal("read: expected state removed when server is 404") } + serverFound, serverIp, serverName = true, "203.0.113.7", "ptr.example.com" postStatus = http.StatusAccepted updateResp := resource.UpdateResponse{State: emptyState()} r.Update(ctx, resource.UpdateRequest{Plan: plan, State: state}, &updateResp) @@ -139,3 +139,84 @@ func TestServerReverseNameResourceUnit(t *testing.T) { t.Fatalf("delete: unexpected request body %v", lastBody) } } + +// The change_reverse_name action never resolves while the PTR is already applied on the server. +func TestServerReverseNameCreateStuckActionUnit(t *testing.T) { + serverActionPollInterval = time.Millisecond + t.Cleanup(func() { serverActionPollInterval = 5 * time.Second }) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + serverGets := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodPost && r.URL.Path == "/servers/1/actions": + _, _ = w.Write([]byte(`{"action":{"id":9,"status":"in-progress"}}`)) + case r.Method == http.MethodGet && r.URL.Path == "/servers/1/actions/9": + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/servers/1": + serverGets++ + name := "default.example.net" + if serverGets > 2 { + name = "ptr.example.com" + } + _, _ = fmt.Fprintf(w, `{"server":{"id":1,"networks":{"v4":[{"ip_address":"203.0.113.7","type":"public","reverse_name":%q}]}}}`, name) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusTeapot) + } + })) + defer srv.Close() + + client, err := binarylane.NewClientWithConfig(srv.URL, "test") + if err != nil { + t.Fatal(err) + } + r := &serverReverseNameResource{bc: &BinarylaneClient{client: client}} + want := "ptr.example.com" + if err := r.changeReverseName(ctx, 1, "203.0.113.7", &want); err != nil { + t.Fatalf("create must finish once the server reports the reverse name: %v", err) + } + if serverGets != 3 { + t.Fatalf("expected to poll the server until the reverse name applied, got %d GETs", serverGets) + } +} + +func TestWaitForServerActionUnit(t *testing.T) { + serverActionPollInterval = time.Millisecond + t.Cleanup(func() { serverActionPollInterval = 5 * time.Second }) + + for name, tc := range map[string]struct { + status int + body string + wantErr bool + }{ + "completed_at set": {http.StatusOK, `{"action":{"id":9,"status":"in-progress","completed_at":"2026-01-01T00:00:00Z"}}`, false}, + "completed status only": {http.StatusOK, `{"action":{"id":9,"status":"completed"}}`, false}, + "errored": {http.StatusOK, `{"action":{"id":9,"status":"errored"}}`, true}, + "not found": {http.StatusNotFound, `{"message":"Action not found."}`, true}, + "in progress until timeout": {http.StatusOK, `{"action":{"id":9,"status":"in-progress"}}`, true}, + } { + t.Run(name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + + client, err := binarylane.NewClientWithConfig(srv.URL, "test") + if err != nil { + t.Fatal(err) + } + sr := serverResource{bc: &BinarylaneClient{client: client}} + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if err := sr.waitForServerAction(ctx, 1, 9); (err != nil) != tc.wantErr { + t.Fatalf("wantErr=%v, got %v", tc.wantErr, err) + } + }) + } +}