Skip to content

fix(hetznercloud): poll zone action endpoint with adequate timeout#1149

Open
buddhaCode wants to merge 5 commits into
qdm12:masterfrom
buddhaCode:fix/hetznercloud-zone-action-endpoint
Open

fix(hetznercloud): poll zone action endpoint with adequate timeout#1149
buddhaCode wants to merge 5 commits into
qdm12:masterfrom
buddhaCode:fix/hetznercloud-zone-action-endpoint

Conversation

@buddhaCode

Copy link
Copy Markdown

What

The hetznercloud provider marks successful record updates as failed,
leaving the container permanently unhealthy after an IP change. This fixes two
distinct bugs in the asynchronous action handling.

Fixes #1136

Background: server actions vs zone actions

The Hetzner Cloud API returns an asynchronous action (a background job with
an id and a status) for many operations, and you poll that action until it
reports success. Crucially, actions are scoped to the resource type they
belong to, and each type has its own lookup path /{resource}/actions/{id}:

  • /v1/servers/actions/{id} — actions for Cloud Servers (VPS instances),
    a completely separate Hetzner Cloud product.
  • /v1/zones/actions/{id} — actions for DNS zones, which is what this
    provider works with.

A DNS record update produces a zone action, so it must be queried on the
zone path. Querying it on the server path asks the VPS/server subsystem for an
id it has never seen → 404 not_found.

Root cause

Record updates via Zone RRSet operations (e.g. set_records) return a zone
action that waitAction then polls until it reaches success. Two things
were wrong:

  1. Wrong endpoint. It polled the server actions endpoint
    https://api.hetzner.cloud/v1/servers/actions/{id} instead of the zone
    actions endpoint https://api.hetzner.cloud/v1/zones/actions/{id}, so the
    lookup always returned 404 not_found. The record itself was already
    updated server-side, but the 404 was treated as an update failure.
    (waitaction.go#L20)

  2. Too short a timeout. Even against the correct endpoint, zone actions
    take ~20s to reach success, but waitAction only polled 3 times with a
    1s sleep (~3s total) and then gave up with "did not complete after 3 tries".
    Fixing only the URL would just swap the 404 for a timeout error — the
    container would stay unhealthy.

The action path scheme (/{resource}/actions/{id} with resource = "zones")
matches Hetzner's official hcloud-go
library.

Fix

  • Poll the zone actions endpoint.
  • Poll every 5s for up to 60s (12 tries) to accommodate the real action
    duration, while keeping the existing success / error / completion checks.

Verified against the live Hetzner Cloud API

  • Old endpoint …/v1/servers/actions/{id}404 not_found; new endpoint
    …/v1/zones/actions/{id}200 with "command": "set_rrset_records" and
    "resources": [{ "type": "zone" }].
  • Measured action completion time: ~18–21s (hence the 60s budget).
  • The real ddns-updater binary, before: ERROR … 404: not_found; after:
    the update completes with no error (verified by reading the record back).

Alternatives considered

  1. Skip the action poll (optimistic update). As suggested in the issue,
    treat the write as synchronous and consider the update successful as soon
    as set_records/add_records returns 201, without waiting for the
    action to reach success. The record value is in fact applied server-side
    before the action completes (confirmed by reading it back while the action
    is still running). This could be offered as an opt-in provider setting
    (e.g. "skip_action_check": true) for users who prefer speed/robustness
    over confirmation. It is not the default here because it gives up genuine
    confirmation that the zone change was accepted.
  2. Treat a 404 on the action lookup as success. More robust against
    further API quirks, but masks real failures.

This PR keeps the (now working) status check as the default. Happy to add the
opt-in skip_action_check option as well if you'd like it in the same PR.

Open question on the timing values

I went with a fixed 5s interval / 60s total, which comfortably covers the
~20s I measured. Happy to change this if you'd prefer:

  • a backoff strategy (e.g. 1s, 2s, 4s … capped) instead of a fixed interval,
  • a longer overall budget, or
  • making the interval/timeout configurable as optional fields in the provider
    settings (the provider already documents optional params like ttl in
    docs/hetznercloud.md).

Let me know your preference and I'll adjust.

Tests

waitaction_test.go covers the queried URL (the regression), the
running → success polling loop, the timeout path, and the success / 404 /
action-error cases. go test ./... and golangci-lint run pass.

Record updates submitted via Zone RRSet operations (e.g. set_records) return
an asynchronous action that belongs to the zone resource. Two problems made
these updates fail even though the record was changed correctly server-side:

1. waitAction polled the server actions endpoint (/v1/servers/actions/{id})
   instead of the zone actions endpoint (/v1/zones/actions/{id}), so the
   lookup always returned 404 (action not found). The action path scheme
   matches Hetzner's official hcloud-go library (ResourceActionClient with
   resource "zones").
2. Even against the correct endpoint, zone actions take ~20s to reach the
   success status, but waitAction only polled 3 times with a 1s sleep (~3s),
   giving up with "did not complete after 3 tries". Poll every 5s for up to
   60s instead.

Both were verified against the live Hetzner Cloud API: the old endpoint
returns 404 for a zone action id while the new one returns 200, and the real
ddns-updater binary now completes the update without error.

Add unit tests covering the queried URL, the running->success polling loop,
the timeout, and the success/404/action-error cases.

Fixes qdm12#1136
Add a regression test for the wildcard rapid-update loop reported in
issue qdm12#1039: a wildcard owner "*" must keep the literal "*" in the
rrset path (the API returns 404 for "%2A") and poll the zone actions
endpoint. Covers both the create and set_records paths via a recording
transport. Extract the shared dummy token into a testToken constant.
@buddhaCode

Copy link
Copy Markdown
Author

Additional note: this also fixes the wildcard rapid-update loop reported in #1039 (comment by @Daxolion).

That report looked like a separate, wildcard-specific bug, but it is the same Zone-action polling issue fixed here. A wildcard owner * is handled like any other owner: createRRSet / set_records succeed server-side, but on master the resulting action was polled at /v1/servers/actions/{id}404 not_found → the update was treated as failed and retried on every cycle.

I verified this against the live Hetzner Cloud API with a real wildcard owner, on both the create and the set_records paths: master loops, and with this PR the wildcard updates once and then converges. I also added a regression test (update_wildcard_test.go) that covers the wildcard owner (literal * in the rrset path, never %2A, and the zone actions endpoint).

I'm intentionally not adding a Fixes #1039 keyword, since #1039 is the broader Hetzner Cloud migration tracking issue and should stay open. Relates to #1039.

The Hetzner Cloud API returns an action error as an object {code, message}, not a bare string. Mock that shape so the test reflects the real API response.
Use the canonical reference anchor (#tag/zone-actions/get_zone_action) so the link points at the right operation.
@buddhaCode

Copy link
Copy Markdown
Author

Pushed two small follow-up commits while cross-checking the provider against the official API reference and hcloud-go:

  • test(hetznercloud): the action_error case in waitaction_test.go mocked the error as a bare string ("error":"boom"), but the API returns it as an object {"code": ..., "message": ...} (reference, and schema.ActionError in hcloud-go). The test now uses that realistic shape.
  • docs(hetznercloud): corrected the doc link anchor to the canonical #tag/zone-actions/get_zone_action.

While doing this I noticed a related, pre-existing point that is outside this PR's scope: actionResponse.Action.Error is typed as any (in types.go, unchanged by this PR), so on a real failure the %v formatting in waitAction would print something like map[code:action_failed message:Action failed] instead of a clean action_failed: Action failed. Typing it as a small struct (like the existing errorResponse) would make the error message readable.

Would you prefer I fold that fix into this PR, or keep it separate so this one stays focused on the zone-action endpoint? Happy either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: hetznercloud provider: action polling 404 marks successful updates as failed, container stays unhealthy

1 participant