apache
Allow uploading of ISO for creating kubernetes supported versions
Description
This PR fixes https://github.com/apache/cloudstack/issues/8417 by allowing the user to register an ISO by uploading from local.
Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
- [ ] test (unit or integration test code)
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [x] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?
Codecov Report
:x: Patch coverage is 9.77444% with 120 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 17.35%. Comparing base (3d6ec29) to head (1abe54e).
:warning: Report is 129 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #9561 +/- ##
============================================
- Coverage 17.36% 17.35% -0.01%
- Complexity 15237 15238 +1
============================================
Files 5888 5889 +1
Lines 525741 525898 +157
Branches 64164 64174 +10
============================================
+ Hits 91274 91277 +3
- Misses 424167 424320 +153
- Partials 10300 10301 +1
| Flag | Coverage Δ | |
|---|---|---|
| uitests | 3.62% <ø> (-0.01%) |
:arrow_down: |
| unittests | 18.39% <9.77%> (-0.01%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@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 ✔️ debian ✔️ suse15. SL-JID 10728
@vishesh92 will this create access issues? Currently, k8s ISOs are registered as public and are accessible to all. But now will it allow linking any existing ISO which can be seen by root admin. Also, earlier such ISOs were owned by system account to prevent direct deletion. Is there a check to prevent deletion if it is linked to a k8s version?
@vishesh92 will this create access issues? Currently, k8s ISOs are registered as public and are accessible to all. But now will it allow linking any existing ISO which can be seen by root admin. Also, earlier such ISOs were owned by system account to prevent direct deletion. Is there a check to prevent deletion if it is linked to a k8s version?
I can add checks on the backend to ensure that the ISO has the required configuration settings. But it might be confusing for the end user.
Let me revisit the approach and directly allow uploading of ISO for k8s.
@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 ✖️ debian ✖️ suse15. SL-JID 10747
@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.
@vishesh92 To be frank, I think this is too complicated. We could add an API to transform a regular ISO (registered or uploaded from local) to a CKS ISO, it will be simple and cover mose cases (e.g. Update an existing ISO to support CKS)
@vishesh92 To be frank, I think this is too complicated. We could add an API to transform a regular ISO (registered or uploaded from local) to a CKS ISO, it will be simple and cover mose cases (e.g. Update an existing ISO to support CKS)
@weizhouapache My initial PR was for linking an existing ISO with a CKS Supported version (https://github.com/apache/cloudstack/compare/main...shapeblue:cloudstack:k8s-version-add-iso-id-backup). @shwstppr had some concerns regarding the iso's permissions. So, I made the implementation similar to how it is there right now for templates & ISOs.
@vishesh92 To be frank, I think this is too complicated. We could add an API to transform a regular ISO (registered or uploaded from local) to a CKS ISO, it will be simple and cover mose cases (e.g. Update an existing ISO to support CKS)
@weizhouapache My initial PR was for linking an existing ISO with a CKS Supported version (main...shapeblue:cloudstack:k8s-version-add-iso-id-backup). @shwstppr had some concerns regarding the iso's permissions. So, I made the implementation similar to how it is there right now for templates & ISOs.
we can add some restrictions
- only root admin can do (same as add k8s version API)
- the ISO will be changed to the same properties as k8s ISO (e.g. owner is system account, all users can access)
I had a quick look at your initial PR, it looks ok to me
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10772
@vishesh92 To be frank, I think this is too complicated. We could add an API to transform a regular ISO (registered or uploaded from local) to a CKS ISO, it will be simple and cover mose cases (e.g. Update an existing ISO to support CKS)
@weizhouapache My initial PR was for linking an existing ISO with a CKS Supported version (main...shapeblue:cloudstack:k8s-version-add-iso-id-backup). @shwstppr had some concerns regarding the iso's permissions. So, I made the implementation similar to how it is there right now for templates & ISOs.
we can add some restrictions
* only root admin can do (same as add k8s version API) * the ISO will be changed to the same properties as k8s ISO (e.g. owner is system account, all users can access)I had a quick look at your initial PR, it looks ok to me
@weizhouapache I am not sure about changing the properties of the existing ISO. I can maybe create a copy of the ISO with required ownership and properties. Do you think that would be a better approach?
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@vishesh92 could you please fix the merge conflicts. Thank you.
@vishesh92 , please also look at target branch and release, these do not match.
@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 ✔️ debian ✔️ suse15. SL-JID 12444
@Pearl1594 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 12508
@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-12495) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 56479 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9561-t12495-kvm-ol8.zip Smoke tests completed. 140 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|---|---|---|
| test_06_purge_expunged_vm_background_task | Failure |
405.75 | test_purge_expunged_vms.py |