Skip to content

Provisioners: replace blind sleeps in proxmox_lxc create/TUN path with readiness checks #30

Description

@Zorlin

What

Two blind sleeps remain in proxmox_lxc.rs's create/TUN path — pre-existing antipatterns left out of #28 (which only fixed the destroy footgun). They violate the repo's "no sleeps" rule and make create timing fragile/host-dependent.

In ensure_exists, the needs_tun block (src/provisioners/proxmox_lxc.rs:1465):

if needs_tun {
    // Wait for container to be fully created
    std::thread::sleep(std::time::Duration::from_secs(2));   // line 1467 — blind
    let _ = self.configure_tun(&conn, vmid)?;
    // Now start the container
    self.start_container(&conn, vmid)?;
    std::thread::sleep(std::time::Duration::from_secs(3));   // line 1471 — blind
}
  • 1467 (2s): "wait for container to be fully created" — a guess at how long the async create takes.
  • 1471 (3s): after start_container — a guess at how long start takes.

Both are unconditional; on a slow node they race (configure_tun / start hits a not-yet-ready container), on a fast node they waste time.

Fix

Replace each with a readiness check (no blind sleeps):

  • The create and start endpoints return a Proxmox UPID (async task). The cleanest fix is to poll the task to completion — there's already a task-wait poll pattern in this file (~line 918, polls /tasks/{upid}/status until status == "stopped"). Generalise/reuse it.
  • Alternatively, poll the container's /status/current until it reports the desired state (created/running) — the same shape as the wait_for_stopped readiness poll added in feat(provisioners): safe destroy state model (stopped/absent/destroyed) #29 (proxmox_vm.rs / proxmox_lxc.rs).

Scope note

These are unrelated to the destroy footgun (#28 / #29), which is why they were deferred. The other sleeps in this file are fine: ~918 and ~866 are conditional polls (task/template readiness with attempt caps), and ~1282 is #29's bounded wait_for_stopped.

Acceptance

  • grep -rn "thread::sleep\|time::sleep" src/provisioners/ shows only bounded, conditional readiness polls (no unconditional from_secs(N)).
  • cargo test / clippy / fmt / build green.
  • A TUN-provisioning create still configures TUN + starts reliably (ideally exercised against a real cluster).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions