Skip to content

Fixes - Skip compute orchestration for unmanaged hosts#456

Open
jakduch wants to merge 1 commit intotheforeman:masterfrom
jakduch:fix/skip-compute-orchestration-unmanaged-hosts
Open

Fixes - Skip compute orchestration for unmanaged hosts#456
jakduch wants to merge 1 commit intotheforeman:masterfrom
jakduch:fix/skip-compute-orchestration-unmanaged-hosts

Conversation

@jakduch
Copy link
Copy Markdown

@jakduch jakduch commented Mar 4, 2026

Summary

When registering existing hosts via Foreman Global Registration using a hostgroup
that includes a Proxmox Compute Resource, the plugin's compute orchestration
methods (setComputeUpdate, delComputeUpdate, setComputeDetails) are triggered
even though the host is unmanaged (managed: false).

This causes errors such as:

NoMethodError: protected method 'setCompute' called for #<Host::Managed>

The root cause is that core Foreman's queue_compute runs whenever compute_resource_id
is present (inherited from the hostgroup), but the plugin's orchestration overrides
do not check whether the host is actually managed before attempting Proxmox API operations.

Changes

Added managed? guards to the three orchestration methods in
app/models/concerns/orchestration/proxmox/compute.rb:

  • setComputeUpdate: returns true early if host is not managed
  • delComputeUpdate: returns true early if host is not managed
  • setComputeDetails: returns true early if host is not managed

Impact

  • Registration of existing hosts: Now works correctly when the hostgroup has a Proxmox Compute Resource
  • Provisioning of new VMs: Unaffected — new VMs are managed: true and orchestration runs normally
  • Updating existing managed VMs: Unaffected — same managed: true logic

Environment

  • Foreman 3.19 (also reproducible on 3.x series)
  • foreman_fog_proxmox 0.21.0
  • Proxmox VE 8.x

Test plan

  • Register an existing VM using Global Registration with a hostgroup that has a Proxmox Compute Resource → should succeed without compute orchestration errors
  • Provision a new VM through Foreman with Proxmox → should work as before
  • Update a managed host's compute attributes → should work as before

@jakduch
Copy link
Copy Markdown
Author

jakduch commented Mar 4, 2026

PS. Tested and worked on our production servers.

@jakduch jakduch force-pushed the fix/skip-compute-orchestration-unmanaged-hosts branch from 9af5ca3 to a71d373 Compare March 4, 2026 17:19
@jakduch jakduch changed the title Fixes #455 - Skip compute orchestration for unmanaged hosts Fixes - Skip compute orchestration for unmanaged hosts Mar 4, 2026
@jakduch
Copy link
Copy Markdown
Author

jakduch commented Mar 10, 2026

Additional finding: core Foreman also needs managed? guards

While testing this fix, I discovered that the plugin-side fix alone is not sufficient. Core Foreman's app/models/concerns/orchestration/compute.rb also lacks managed? checks in several methods:

Affected methods in core Foreman:

  • setCompute — attempts to create a compute instance even for unmanaged hosts
    • delCompute — attempts to destroy a compute instance during rollback, even for unmanaged hosts
      The queue_compute method adds orchestration tasks whenever compute_resource_id is present (inherited from hostgroup), regardless of the managed? flag. The plugin overrides setComputeUpdate, delComputeUpdate, and setComputeDetails (which this PR fixes), but setCompute and delCompute are defined in core and are not overridden by the plugin.

Reproduction:

  1. Create a hostgroup with a Proxmox Compute Resource
    1. Register an existing host via Global Registration using that hostgroup
    1. The host is created as managed: false, but setCompute and delCompute still run and fail
      Suggested core fix:
      Add return true unless managed? at the beginning of setCompute and delCompute in app/models/concerns/orchestration/compute.rb.

I will file a separate issue on the core Foreman repository for this.

@nadjaheitmann
Copy link
Copy Markdown
Contributor

@jakduch Can you please add a log message and some test coverage analogue to the PR in Foreman core? (theforeman/foreman#10905)

@jakduch jakduch force-pushed the fix/skip-compute-orchestration-unmanaged-hosts branch from a71d373 to 2e21c8d Compare April 7, 2026 11:17
…ed hosts

When a host is registered via Global Registration with a hostgroup that
has a Compute Resource (Proxmox), the compute orchestration methods
(setComputeUpdate, delComputeUpdate, setComputeDetails) are triggered
even though the host is unmanaged (managed: false). This causes errors
like "protected method setCompute called" because the orchestration
tries to interact with the Proxmox API for a host that was not
provisioned through Foreman.

This patch adds managed? guards with log messages to skip
Proxmox-specific compute orchestration for unmanaged hosts,
analogous to foreman core PR #10905.

Unit tests verify all three methods skip orchestration and log
appropriately for unmanaged hosts.

Assisted-By: Claude (Anthropic)
@jakduch jakduch force-pushed the fix/skip-compute-orchestration-unmanaged-hosts branch from 2e21c8d to b30393f Compare April 7, 2026 13:48
@jakduch
Copy link
Copy Markdown
Author

jakduch commented Apr 7, 2026

Added log messages and unit tests as requested, following the same pattern as foreman#10905.

Changes:

  • All three managed? guards now log an info message when skipping orchestration (e.g. Skipping Proxmox compute update for <host> - host is not managed)
  • Added test/unit/foreman_fog_proxmox/proxmox_orchestration_compute_test.rb with 6 tests covering setComputeUpdate, delComputeUpdate, and setComputeDetails - verifying both the return value and the log output for unmanaged hosts

Squashed into a single commit with Assisted-By: Claude (Anthropic) trailer.

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