Skip to content

SystemVM offering declaration - #327

Open
bddvlpr wants to merge 2 commits into
apache:mainfrom
bddvlpr:feat/system-offerings
Open

SystemVM offering declaration#327
bddvlpr wants to merge 2 commits into
apache:mainfrom
bddvlpr:feat/system-offerings

Conversation

@bddvlpr

@bddvlpr bddvlpr commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Adds a cloudstack_system_service_offering resource that does what it says on the tin, creates SystemVM offerings. I was first considering merging this logic with cloudstack_service_offering_fixed as it performs the exact same API call with an exception of a few parameters, but to avoid confusion I kept it separate (I also don't want to mix it with the shared commons between constrained and fixed offerings).

Copilot AI lite review requested due to automatic review settings August 28, 2026 06:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 28, 2026 06:11
@bddvlpr
bddvlpr force-pushed the feat/system-offerings branch from 274d962 to 5ed8725 Compare August 28, 2026 06:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sureshanaparti

Copy link
Copy Markdown
Contributor

Hi @bddvlpr Can you check the failed tests here. Thanks.

@bddvlpr

bddvlpr commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

These failing tests are not of my doing, one of which is the panic where the acceptance test times out (happens upstream too periodically). The other is a racecon on the initialization of the simulator which also happens upstream. Could you please re-run these tests?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

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

Comment thread cloudstack/resource_cloudstack_system_service_offering.go
Comment thread cloudstack/resource_cloudstack_system_service_offering.go
Comment thread cloudstack/resource_cloudstack_system_service_offering.go
Comment thread cloudstack/resource_cloudstack_system_service_offering.go
Comment thread cloudstack/resource_cloudstack_system_service_offering.go
@sureshanaparti

Copy link
Copy Markdown
Contributor

Hi @bddvlpr Can you check & address the copilot comments if relevant. Thanks.

@bddvlpr

bddvlpr commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

I've adressed the comments. Only one was kind-of relevant and I've added tests for this one.

@sudo87

sudo87 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@bddvlpr validateSystemServiceOfferingConfiguration compares system_vm_type/storage_type against lowercase literals, but both fields accept mixed case via StringInSlice(..., true) — the lowercasing StateFunc only applies to stored state, not to the raw value CustomizeDiff sees while planning. So system_vm_type = "DomainRouter" (valid per schema, and CloudStack's own casing convention) with network_rate set wrongly fails with "network_rate can only be set when system_vm_type is domainrouter". Same issue in reverse for storage_type = "Local" + offer_ha — it silently skips the check instead. Could this be a problem?

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.

4 participants