-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add support for providing userdata to system VMs #11654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11654 +/- ##
==========================================
Coverage 17.39% 17.40%
+ Complexity 15284 15283 -1
==========================================
Files 5890 5890
Lines 526174 526296 +122
Branches 64233 64245 +12
==========================================
+ Hits 91540 91603 +63
- Misses 424289 424348 +59
Partials 10345 10345
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15055 |
tools/appliance/systemvmtemplate/scripts/configure_systemvm_services.sh
Outdated
Show resolved
Hide resolved
tools/appliance/systemvmtemplate/scripts/configure_systemvm_services.sh
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 adds support for providing user data (cloud-init) to system VMs via Zone Scoped global settings, enabling operators to customize Console Proxy VMs, Secondary Storage VMs, and Virtual Routers with monitoring, logging, or custom commands.
- Added new global settings to enable user data feature and configure user data for each system VM type
- Implemented user data injection logic in system VM managers to encode and append user data to boot arguments
- Updated system VM template cloud-init configuration to use NoCloud datasource and disable automatic cloud-init services
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java | Added global setting to enable user data for system VMs |
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java | Updated config keys array to include the new system VM user data setting |
server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java | Added configuration key for console proxy user data |
server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java | Implemented user data injection logic for console proxy VMs |
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java | Added configuration key for virtual router user data |
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java | Implemented user data injection logic for virtual router VMs |
server/src/main/java/com/cloud/storage/secondary/SecondaryStorageVmManager.java | Added configuration key for secondary storage user data |
services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java | Implemented user data injection logic for secondary storage VMs |
tools/appliance/systemvmtemplate/scripts/configure_systemvm_services.sh | Updated cloud-init configuration to use NoCloud datasource and disabled automatic services |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tools/appliance/systemvmtemplate/scripts/configure_systemvm_services.sh
Outdated
Show resolved
Hide resolved
9cc8655
to
7b6881b
Compare
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
7b6881b
to
f5eeca0
Compare
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 15057 |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15064 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vishesh92 is it a good idea to add some validations to these userdata configs, to avoid any kind of security risks ?
IMO, it would be difficult to do that. Technically the user data could be a script which fetches a binary (malicious) from internet and executes it on the system VM and we won't be able to validate this. I have also added a new global setting, |
2882672
to
565e229
Compare
565e229
to
1dead65
Compare
1dead65
to
53cbc87
Compare
.../elastic-loadbalancer/src/main/java/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM. good to see initially this is allowed only for default root admins. thanks @vishesh92
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15145 |
@blueorangutan test |
@borisstoyanov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR adds support for specifying user data (cloud-init) for system VMs via Zone Scoped global settings. This allows the operators to customize the System VMs and setup monitoring, logging or execute any custom commands.
We set the user data from the global setting in
/var/cache/cloud/cmdline
, and use the NoCloud datasource to process user data. cloud-init service is still disabled in the system VMs and it's executed as part of thecloud-postinit
service which executes thepostinit.sh
script.Added global settings:
systemvm.userdata.enabled
- Disabled by default. Needs to be enabled to utilize the feature.consoleproxy.userdata
- UUID of the User data to be used for Console Proxysecstorage.userdata
- UUID of the User data to be used for Secondary Storage VMrouter.userdata
- UUID of the User data to be used for Virtual RoutersDocs PR: apache/cloudstack-documentation#567
Generated summary
This pull request introduces support for providing user data to system VMs (Console Proxy VM, Secondary Storage VM, and Virtual Router) via global settings, enabling cloud-init configuration through user-supplied data. The changes include new configuration keys, logic to inject user data into VM boot arguments (encoded in base64), and updates to the system VM template to better support cloud-init.
System VM user data support:
systemvm.userdata.enabled
config key to globally enable user data for system VMs, and made it available in theVirtualMachineManager
and related implementations. [1] [2]consoleproxy.userdata
for Console Proxy VMssecstorage.userdata
for Secondary Storage VMsrouter.userdata
for Virtual RoutersgetConfigKeys()
methods in relevant managers to include the new config keys. [1] [2] [3]User data injection logic:
ConsoleProxyManagerImpl
,SecondaryStorageManagerImpl
,VirtualNetworkApplianceManagerImpl
), added logic to:userdata=...
. [1] [2] [3]System VM template / cloud-init configuration:
configure_systemvm_services.sh
to change the default cloud-init configuration, switching the data source toNoCloud
and disabling automatic cloud-init services to allow manual control by CloudStack.These changes collectively enable administrators to inject custom cloud-init user data into system VMs via global settings, improving flexibility and automation for system VM initialization.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?