Allow uploading of ISO for creating kubernetes supported versions #9561

vishesh92 · 2024-08-21T08:57:03Z

Description

This PR fixes #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)
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
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

codecov · 2024-08-21T09:07:41Z

Codecov Report

Attention: Patch coverage is 9.77444% with 120 lines in your changes missing coverage. Please review.

Project coverage is 16.00%. Comparing base (42a77c7) to head (cdcf240).
Report is 55 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...bernetes/version/KubernetesVersionManagerImpl.java	23.63%	41 Missing and 1 partial ⚠️
...tUploadParamsForKubernetesSupportedVersionCmd.java	0.00%	36 Missing ⚠️
...che/cloudstack/api/AbstractGetUploadParamsCmd.java	0.00%	21 Missing ⚠️
...api/command/user/iso/GetUploadParamsForIsoCmd.java	0.00%	18 Missing ⚠️
...oudstack/api/response/GetUploadParamsResponse.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20    #9561      +/-   ##
============================================
+ Coverage     15.98%   16.00%   +0.01%     
- Complexity    13077    13107      +30     
============================================
  Files          5649     5652       +3     
  Lines        495617   495998     +381     
  Branches      59997    60055      +58     
============================================
+ Hits          79247    79376     +129     
- Misses       407520   407759     +239     
- Partials       8850     8863      +13

Flag	Coverage Δ
uitests	`4.00% <ø> (-0.02%)`	⬇️
unittests	`16.84% <9.77%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

vishesh92 · 2024-08-21T09:58:39Z

@blueorangutan package

blueorangutan · 2024-08-21T10:00:03Z

@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.

blueorangutan · 2024-08-21T11:01:03Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10728

FelipeM525 · 2024-08-21T11:43:22Z

@vishesh92 would you mind adding some images to the description?

shwstppr · 2024-08-21T12:06:48Z

@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 · 2024-08-22T06:52:45Z

@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 · 2024-08-22T10:48:17Z

@blueorangutan package

blueorangutan · 2024-08-22T10:50:04Z

@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.

blueorangutan · 2024-08-22T11:15:22Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10747

vishesh92 · 2024-08-23T06:34:30Z

@blueorangutan package

blueorangutan · 2024-08-23T06:36:03Z

@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.

weizhouapache · 2024-08-23T06:44:42Z

@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 · 2024-08-23T06:53:26Z

@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.

weizhouapache · 2024-08-23T07:05:32Z

@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

blueorangutan · 2024-08-23T07:54:37Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10772

vishesh92 · 2024-08-23T08:05:50Z

@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?

github-actions · 2024-09-06T06:47:24Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

blueorangutan · 2025-02-13T12:22:03Z

@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.

blueorangutan · 2025-02-13T13:45:33Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12444

Pearl1594 · 2025-02-14T14:33:49Z

...ernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java

+        uploadIso.setName(isoName);
+        uploadIso.setPublicIso(true);
+        if (zoneId != null) {
+            uploadIso.setZoneId(zoneId);


zoneId - seems to be a mandatory param for upload iso, so I wonder if this could cause some failure if upload iso is done by a user / dom admin
considering https://github.com/apache/cloudstack/blob/main/server/src/main/java/com/cloud/template/TemplateAdapterBase.java#L194-L197

I followed the existing checks in AddKubernetesSupportedVersionCmd for the upload command.

I also need to check if we allow domain admin or a user to add kubernetes versions.

It seems like by default we do not allow registering kubernetes iso for dom admins. Corner case is when we add custom roles. But I think that can be overlooked.

...ck/api/command/admin/kubernetes/version/GetUploadParamsForKubernetesSupportedVersionCmd.java

ui/src/config/section/image.js

...ck/api/command/admin/kubernetes/version/GetUploadParamsForKubernetesSupportedVersionCmd.java

ui/src/config/section/image.js

Pearl1594 · 2025-02-18T15:35:42Z

@blueorangutan package

blueorangutan · 2025-02-18T15:38:04Z

@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.

blueorangutan · 2025-02-18T16:42:03Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12508

vishesh92 · 2025-02-26T10:19:20Z

@blueorangutan test

blueorangutan · 2025-02-26T10:20:03Z

@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-02-27T02:42:20Z

[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

kiranchavala

@vishesh92 Found the following issues when testing

The text contains "label.kubernetes.version.add.from local " it should be

Upload kubenetes ISO from local

After clicking ok in the kubernetes iso upload, there is no action happening in the UI or upload progress shown

For a Normal iso upload, there is upload progress.

Also I see no options in devtools section when uploading a kubernetes iso

vishesh92 · 2025-03-12T08:46:27Z

@blueorangutan package

blueorangutan · 2025-03-12T08:48:04Z

@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.

blueorangutan · 2025-03-12T09:56:31Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12751

Pearl1594

code lgtm

kiranchavala

@vishesh92

Failed iso upload remains in allocated state

Step to reproduce the issue

Navaigte to images > kubetnetes iso > Upload a k8s iso
For some reason the job fails
The kubernetes iso state will be in Allocated state

Navaigte to images >iso the k8s iso will be in "Not ready" state

Navaigte to images > kubetnetes iso

Later on it goes into a failed state after "upload.operation.timeout)"

While images > Iso > the state will be still in Not ready

In case of failures

When end user executed the API call list kubernetessupportedversions

can you change the behaviour of isostate from

Allocated >>failed

to

Notready to failed

(localcloud) 🐱 > list kubernetessupportedversions filter=isoname,isostate
{
"count": 4,
"kubernetessupportedversion": [
{
"isoname": "v1.31.1-Kubernetes-Binaries-ISO",
"isostate": "Failed"
},

boring-cyborg bot added component:kubernetes component:UI labels Aug 21, 2024

vishesh92 force-pushed the k8s-version-add-iso-id branch from 158acb9 to 364f4ce Compare August 21, 2024 09:58

vishesh92 force-pushed the k8s-version-add-iso-id branch from 364f4ce to be1539f Compare August 22, 2024 10:47

boring-cyborg bot added the component:api label Aug 22, 2024

vishesh92 changed the title ~~Provide option to use existing ISOs for kubernetes supported version~~ Allow uploading of ISO for creating kubernetes supported versions Aug 22, 2024

vishesh92 force-pushed the k8s-version-add-iso-id branch from be1539f to 6d378b5 Compare August 23, 2024 06:34

rohityadavcloud added this to the 4.20.0.0 milestone Sep 4, 2024

rohityadavcloud requested review from shwstppr and Pearl1594 September 5, 2024 04:12

github-actions bot added the status:has-conflicts label Sep 6, 2024

JoaoJandre removed this from the 4.20.0.0 milestone Sep 10, 2024

Pearl1594 reviewed Feb 14, 2025

View reviewed changes

...ck/api/command/admin/kubernetes/version/GetUploadParamsForKubernetesSupportedVersionCmd.java Outdated Show resolved Hide resolved

Pearl1594 reviewed Feb 14, 2025

View reviewed changes

ui/src/config/section/image.js Outdated Show resolved Hide resolved

vishesh92 commented Feb 14, 2025

View reviewed changes

...ck/api/command/admin/kubernetes/version/GetUploadParamsForKubernetesSupportedVersionCmd.java Outdated Show resolved Hide resolved

vishesh92 commented Feb 14, 2025

View reviewed changes

ui/src/config/section/image.js Show resolved Hide resolved

vishesh92 commented Feb 14, 2025

View reviewed changes

ui/src/config/section/image.js Outdated Show resolved Hide resolved

Apply suggestions from code review

19f4f64

kiranchavala reviewed Feb 28, 2025

View reviewed changes

Fix k8s iso upload

cdcf240

Pearl1594 added this to ACS 4.20.1 Mar 17, 2025

Pearl1594 moved this to In Progress in ACS 4.20.1 Mar 17, 2025

vishesh92 requested review from Pearl1594 and kiranchavala March 17, 2025 20:44

Pearl1594 approved these changes Mar 17, 2025

View reviewed changes

kiranchavala reviewed Mar 18, 2025

View reviewed changes

rohityadavcloud requested a review from nvazquez May 1, 2025 06:01

Pearl1594 removed this from ACS 4.20.1 Jun 3, 2025

Pearl1594 modified the milestones: 4.20.1, 4.20.2 Jun 3, 2025

Allow uploading of ISO for creating kubernetes supported versions #9561

Are you sure you want to change the base?

Allow uploading of ISO for creating kubernetes supported versions #9561

Uh oh!

Conversation

vishesh92 commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

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?

Uh oh!

codecov bot commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vishesh92 commented Aug 21, 2024

Uh oh!

blueorangutan commented Aug 21, 2024

Uh oh!

blueorangutan commented Aug 21, 2024

Uh oh!

FelipeM525 commented Aug 21, 2024

Uh oh!

shwstppr commented Aug 21, 2024

Uh oh!

vishesh92 commented Aug 22, 2024

Uh oh!

vishesh92 commented Aug 22, 2024

Uh oh!

blueorangutan commented Aug 22, 2024

Uh oh!

blueorangutan commented Aug 22, 2024

Uh oh!

vishesh92 commented Aug 23, 2024

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

weizhouapache commented Aug 23, 2024

Uh oh!

vishesh92 commented Aug 23, 2024

Uh oh!

weizhouapache commented Aug 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

vishesh92 commented Aug 23, 2024

Uh oh!

github-actions bot commented Sep 6, 2024

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

Pearl1594 Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

vishesh92 Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

vishesh92 Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

Pearl1594 Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pearl1594 commented Feb 18, 2025

Uh oh!

blueorangutan commented Feb 18, 2025

Uh oh!

vishesh92 commented Aug 21, 2024 •

edited

Loading

codecov bot commented Aug 21, 2024 •

edited

Loading

weizhouapache commented Aug 23, 2024 •

edited

Loading