From ac86302f5296b47eb5fdf1ba1ee15016ff514a6f Mon Sep 17 00:00:00 2001 From: dtoro Date: Fri, 10 Jul 2026 00:50:54 +0200 Subject: [PATCH] fix: pct_create make vmid optional, fix dhcp+gw, longer boot settle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups found while verifying the approve→provision path end to end: - vmid is now optional: the early required-field check rejected vmid:0 before the cluster VMID guard could auto-assign a free id. Only hostname is required now; 0 (or a collision) resolves to `pvesh get /cluster/nextid`. - net0: use ip=dhcp with no gateway when no static IP is given (Proxmox rejects gw alongside dhcp); only attach gw for a static CIDR. - bump post-create settle to 10s so a DHCP lease is up before apt runs. Co-Authored-By: Claude Opus 4.8 --- internal/httpapi/phase3.go | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/internal/httpapi/phase3.go b/internal/httpapi/phase3.go index ccbdca8..011386c 100644 --- a/internal/httpapi/phase3.go +++ b/internal/httpapi/phase3.go @@ -230,10 +230,12 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID, emitExecutionEvent(ctx, pool, execID, "failed", map[string]any{"target": targetSlug, "error": err.Error()}) return } - if cfg.VMID == 0 || cfg.Hostname == "" { + // Only hostname is required. vmid is optional — when 0 (or later found + // to collide) the VMID guard below assigns a free cluster id. + if cfg.Hostname == "" { pool.Exec(ctx, `UPDATE executions SET status='failed', result=$2::jsonb WHERE entity_id=$1`, - execID, `{"error":"pct_create: vmid and hostname are required"}`) - emitExecutionEvent(ctx, pool, execID, "failed", map[string]any{"target": targetSlug, "error": "missing vmid or hostname"}) + execID, `{"error":"pct_create: hostname is required"}`) + emitExecutionEvent(ctx, pool, execID, "failed", map[string]any{"target": targetSlug, "error": "missing hostname"}) return } if cfg.Cores == 0 { @@ -329,11 +331,23 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID, nestingFlag = fmt.Sprintf(" --features %s", strings.Join(features, ",")) } + // net0: DHCP when no static IP is given (or ip=="dhcp"). Proxmox + // rejects a gateway alongside ip=dhcp, so only add gw for a static IP. + net0 := "name=eth0,bridge=vmbr0," + if cfg.IP == "" || strings.EqualFold(cfg.IP, "dhcp") { + net0 += "ip=dhcp" + } else { + net0 += "ip=" + cfg.IP + if cfg.GW != "" { + net0 += ",gw=" + cfg.GW + } + } + templatePath := fmt.Sprintf("/var/lib/vz/template/cache/%s", cfg.Template) createCmd := fmt.Sprintf( - "pct create %d %s --hostname %s --cores %d --memory %d --rootfs %s:%d %s --net0 name=eth0,bridge=vmbr0,ip=%s,gw=%s%s --start 1", + "pct create %d %s --hostname %s --cores %d --memory %d --rootfs %s:%d %s --net0 %s%s --start 1", cfg.VMID, templatePath, cfg.Hostname, cfg.Cores, cfg.Memory, - cfg.Storage, cfg.DiskGB, privFlag, cfg.IP, cfg.GW, nestingFlag) + cfg.Storage, cfg.DiskGB, privFlag, net0, nestingFlag) if cfg.Nameserver != "" { createCmd += fmt.Sprintf(" --nameserver %s", cfg.Nameserver) @@ -358,8 +372,9 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID, // with a boot settle; failures are appended to output and mark the // execution failed so the operator sees exactly which step broke. if err == nil && (len(cfg.Services) > 0 || cfg.PostInstall != "") { - // Give the container a moment to finish booting before exec. - steps := []string{fmt.Sprintf("sleep 5")} + // Give the container time to boot and (for DHCP) acquire a lease + // before apt needs the network. + steps := []string{"sleep 10"} if len(cfg.Services) > 0 { pkgs := strings.Join(sanitizePkgs(cfg.Services), " ") steps = append(steps, fmt.Sprintf("pct exec %d -- bash -lc 'apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get install -y -qq %s'", cfg.VMID, pkgs))