-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Adds url kubernetes iso #10862
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: 4.20
Are you sure you want to change the base?
Adds url kubernetes iso #10862
Conversation
@vits-hugs a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
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 enhances the Kubernetes ISO support by including the URL (isourl) in both the API response and the GUI display. Key changes include:
- Adding the new attribute "isourl" to API response objects and updating its constant in ApiConstants.
- Updating UI configuration and localization files to display the ISO URL.
- Extending tests to verify that the ISO URL is returned only for admin users.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
ui/src/config/section/image.js | Added "url" to the image details fields and adjusted the conditional logic for admin views and upload parameter conditions. |
ui/public/locales/pt_BR.json | Added the localized label for "isourl". |
ui/public/locales/en.json | Added the localized label for "isourl". |
plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/version/KubernetesVersionServiceTest.java | Extended tests to cover scenarios for admin and non-admin users related to ISO URL. |
plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/response/KubernetesSupportedVersionResponse.java | Introduced a new field with getter and setter for the ISO URL. |
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java | Updated response creation to include the ISO URL only for admin users. |
api/src/main/java/org/apache/cloudstack/api/ApiConstants.java | Defined the new constant for "isourl". |
Comments suppressed due to low confidence (1)
ui/src/config/section/image.js:121
- Verify that removing the isZoneCreated() condition is intentional here, as its absence might allow template uploads in zones that are not yet fully initialized.
show: () => { return 'getUploadParamsForTemplate' in store.getters.apis }
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #10862 +/- ##
==========================================
Coverage 16.13% 16.14%
- Complexity 13220 13241 +21
==========================================
Files 5651 5656 +5
Lines 496740 497530 +790
Branches 60183 60322 +139
==========================================
+ Hits 80148 80309 +161
- Misses 407674 408272 +598
- Partials 8918 8949 +31
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:
|
...ernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@DaanHoogland 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 13383 |
@blueorangutan package |
@vits-hugs 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 13385 |
@vits-hugs , these are a bit awkward to search but here are the unit test failures: https://github.com/apache/cloudstack/actions/runs/15027619849/job/42263448047?pr=10862#step:7:14260 |
@blueorangutan package |
@vits-hugs 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 13404 |
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.
While code looks fine, I'm not sure of design for checking if the KubernetesSupportedVersionResponse should have isoUrl.
Right now if the list API returns 10 versions it will query for root admin check 10 times.
@vits-hugs can we use ResponseView from the API class itself? This may require change in createKubernetesSupportedVersionResponse method. add/update API can always have ResponseView.Full while listAPI would need to check the caller and decide on Full/Restricted
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
...ernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java
Outdated
Show resolved
Hide resolved
...ernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java
Outdated
Show resolved
Hide resolved
+1 this PR could be optimised by adding isAdmin or isRootAdmin as the argument of the method, so that the following is run only once
|
@blueorangutan package |
@vits-hugs 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. |
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
Not tested
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13425 |
Description
The URL used to register a kubernetes ISO is stored on the database, however it's not shown by the API
listKubernetesSupportedVersion
. This PR changes the API to return the kubernets ISO URL (isourl
attribute) and the GUI to show it.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
Before modification:
After modification:

How Has This Been Tested?
I registered a kubernetes iso, and checked the API and GUI to see if the kubernetes ISO URL is appearing correctly.