-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Instance lease: Allow deployment of instances with lease duration and leaseexpiry action #10560
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
Conversation
config key setup and configured for alert email lease options in create and update vm screen handle delete protection, edit vm, create vm validated stop and detroy, delete protection
@blueorangutan package |
@sudo87 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 ✖️ debian ✖️ suse15. SL-JID 12761 |
...src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10560 +/- ##
============================================
+ Coverage 16.52% 16.56% +0.04%
- Complexity 13791 13854 +63
============================================
Files 5716 5719 +3
Lines 506016 506908 +892
Branches 61380 61537 +157
============================================
+ Hits 83617 83992 +375
- Misses 413037 413509 +472
- Partials 9362 9407 +45
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:
|
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.
@sudo87 build failing with unit tests failure. Please check
server/src/test/java/org/apache/cloudstack/vm/lease/VMLeaseManagerImplTest.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@sudo87 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 ✖️ debian ✖️ suse15. SL-JID 12802 |
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 |
@sudo87 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 ✔️ debian ✔️ suse15. SL-JID 13497 |
@blueorangutan test |
@sudo87 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13397)
|
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 instance lease functionality to allow administrators to provision VMs with a defined lease duration and an expiry action (STOP or DESTROY), improving resource utilization.
- Added new lease-related fields in service offering and VM view objects and responses.
- Introduced new DAO search builders and queries to filter leased and expiring instances.
- Updated API commands and database views to support instance lease parameters.
Reviewed Changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
ServiceOfferingJoinVO.java | Added leaseDuration and leaseExpiryAction fields with corresponding getters. |
UserVmJoinDaoImpl.java | Added search builders to filter leased instance queries and compute lease duration. |
QueryManagerImpl.java | Updated VM ID search logic to include leased instance filtering and error handling. |
SQL Views (cloud.user_vm_view.sql, cloud.service_offering_view.sql) | Extended views with lease-related columns. |
API Modules (UserVmResponse.java, ServiceOfferingResponse.java, UpdateVMCmd.java, etc.) | Updated API responses and commands with lease parameters. |
ApiConstants.java, VmDetailConstants.java, EventTypes.java | Introduced new lease-related constants and event types. |
Tests | Added test cases for lease parameters in CreateServiceOfferingCmdTest.java. |
Comments suppressed due to low confidence (3)
org/apache/cloudstack/api/query/QueryManagerImpl.java:1359
- [nitpick] Extract the error message into a constant to avoid duplication and ensure consistency in error handling.
if (!VMLeaseManager.InstanceLeaseEnabled.value() && cmd.getOnlyLeasedInstances()) {
org/apache/cloudstack/api/query/QueryManagerImpl.java:1517
- [nitpick] Clarify the join condition by verifying that filtering on the 'name' field for lease execution details is the intended approach; consider using a more descriptive field or alias to improve readability.
leasedInstancesSearch.and(leasedInstancesSearch.entity().getName(), SearchCriteria.Op.EQ).values(VmDetailConstants.INSTANCE_LEASE_EXECUTION);
api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java:163
- [nitpick] Consider renaming the local variable 'onlyLeasedInstances' to 'leased' or similar to match the API constant (LEASED) for consistency.
@Parameter(name = ApiConstants.LEASED, type = CommandType.BOOLEAN,
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.
@sudo87 left some minor comments.
...src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManager.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManager.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManagerImpl.java
Show resolved
Hide resolved
…ager.java Co-authored-by: Vishesh <[email protected]>
@blueorangutan package |
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.
clgtm
@blueorangutan package |
@sudo87 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 ✔️ debian ✔️ suse15. SL-JID 13526 |
@blueorangutan test |
@sudo87 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13423)
|
… leaseexpiry action (apache#10560) * FR-248: Instance lease, WIP commit * insert lease expiry into db and use that to filter exiring vms, add asyncjobmanager * Add leaseDuration and leaseExpiryAction in Service offering create flow * Update listVM cmd to allow listing only leased instances * Add methods to fetch instances for which lease is expiring in next days * Changes included: config key setup and configured for alert email lease options in create and update vm screen handle delete protection, edit vm, create vm validated stop and detroy, delete protection * Update UI screens for leased properties coming from config and service offering * use global lock before running scheduler * Unit tests * Flow changes done in UI based on discussion * Include view changes in schema upgrade files and use feature in various UI elements * Added integration test for vm deployment, UI enhancements for user persona, bug fixes * validate integration tests, minor ui changes and log messages * fix build: moving configkey from setup to test itself * Disable testAlert to unblock build and trim whitespaces in integration tests * Address review comments * Minor changes in EditVM screen * Use ExecutorService instead of Timer and TimerTask * Additional review comments * Incorporate following changes: 1. Execute lease action once on the instance 2. Cancel lease on instance when feature is disabled 3. Relevant events when lease gets disabled, cancelled, executed 4. Disable associating lease after deployment 5. UI elements and flow changes 6. Changes based on feedback from demo * Handle pr review comments * address review comments * move instance.lease.enabled config to VMLeaseManager interface * bug fix in edit instance flow and reject api request for invalid values * max allowed lease is for 100 years * log instance ids for expired instance * Fix config validation for value range and code coverage improvement * fix lease expiry request failures in async * dont use forced: true for StopVmCmd * Update server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManager.java Co-authored-by: Vishesh <[email protected]> * handle review comments --------- Co-authored-by: Rohit Yadav <[email protected]> Co-authored-by: Vishesh <[email protected]>
Description
This PR enables Administrator to provide capability for end user to provision Lease based instances, which will be self destroyed or stopped once lease expires. This will improve efficient utilization of cloud infrastructure and reduce unused VMs in the cloud infrastructure.
Doc PR: apache/cloudstack-documentation#492
Feature can be enabled by updating value for following key to true:
instance.lease.enabled: true
default: false
There are additional configuration keys:
instance.lease.scheduler.interval: when scheduler will run to execute expiryaction, default value: 3600 (seconds)
instance.lease.alertscheduler.interval: alert scheduler interval, sends alerts for any instance which are about to expire in next 7 days (default: 86400)
Feature introduces new parameters in following APIs:
leaseduration: duration in days for which instance is provisioned
leaseexpiryaction: action will be executed once expiry is over
Valid values for
leaseduration: positive number, negative value to removing lease from instance (-1)
leaseexpiryaction: STOP | DESTROY
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?