Skip to content

fix(container): honor PROXMOX_NODE across role tasks#303

Open
j551n-ncloud wants to merge 3 commits intostevius10:developfrom
j551n-ncloud:fix/proxmox-node-resolution
Open

fix(container): honor PROXMOX_NODE across role tasks#303
j551n-ncloud wants to merge 3 commits intostevius10:developfrom
j551n-ncloud:fix/proxmox-node-resolution

Conversation

@j551n-ncloud
Copy link
Copy Markdown

Summary

  • Fix PROXMOX_NODE handling in the container role so configured node values are consistently used.
  • Remove hardcoded node behavior that could silently target pve.
  • Keep fallback behavior safe when node values are empty.

Problem

PROXMOX_NODE was defined in configuration, but not fully propagated through the role execution path:

  • Node value was not set in env resolution for container tasks.
  • Module defaults used a hardcoded node: "pve".
  • URL templates used a weaker default filter in several places.

This could cause API calls to hit the wrong Proxmox node.

Changes

  • Added PROXMOX_NODE to env fact resolution in base/roles/container/tasks/env.yml.
  • Updated proxmox module defaults to use PROXMOX_NODE | default('pve', true) in base/roles/container/defaults/main.yml.
  • Updated container API URL node expressions to use default('pve', true) in:
    • base/roles/container/tasks/main.yml
    • base/roles/container/tasks/start.yml
    • base/roles/container/tasks/remove.yml
    • base/roles/container/tasks/os.yml

Why This Fix

Using default('pve', true) ensures empty values are treated as missing and fall back safely.
Resolving PROXMOX_NODE from config/env in one place avoids drift between task paths.

Scope and Safety

  • No behavior changes outside node selection.
  • Auth flow and existing credential handling remain unchanged.
  • Local/private config files were intentionally excluded from commit:
    • container.env
    • globals.json
    • local/config.json

Validation

  • Static verification of changed files completed.
  • Local runtime execution started successfully and progressed through pipeline setup without reintroducing the original variable issue.

Commit

  • 0fa7aa8 - fix(container): honor PROXMOX_NODE across role tasks

Copilot AI review requested due to automatic review settings April 17, 2026 13:03
@j551n-ncloud
Copy link
Copy Markdown
Author

sorry for writing description with ai slop im working currently :C

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Proxmox node selection in the container Ansible role so the configured PROXMOX_NODE is propagated consistently through both module defaults and direct API uri calls, avoiding accidental fallback to a hardcoded pve node.

Changes:

  • Resolves and sets PROXMOX_NODE during environment/config fact setup in the container role.
  • Updates Proxmox module defaults to use the resolved node (with a safe fallback for empty values).
  • Updates multiple Proxmox API URL templates to use default('pve', true) so empty node values don’t produce empty URL path segments.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
base/roles/container/tasks/env.yml Adds PROXMOX_NODE to the role’s env/config resolution so downstream tasks share a single resolved node value.
base/roles/container/defaults/main.yml Removes hardcoded node: "pve" and defers to PROXMOX_NODE with a safe fallback.
base/roles/container/tasks/main.yml Uses the stronger default filter for node in the node-scoped /status API call.
base/roles/container/tasks/start.yml Uses the stronger default filter for node in the container config polling URL.
base/roles/container/tasks/remove.yml Uses the stronger default filter for node in stop/removal polling URLs.
base/roles/container/tasks/os.yml Uses the stronger default filter for node in OS download and content polling URLs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants