NAS-135491 / 25.10 / Allow specifying image OS for virt VMs in UI #11938
Conversation
bugclerk
changed the title
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #11938 +/- ##
=======================================
Coverage 83.67% 83.68%
=======================================
Files 1687 1687
Lines 59678 59712 +34
Branches 6613 6620 +7
=======================================
+ Hits 49938 49968 +30
- Misses 9740 9744 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| [ImageOs.Linux, 'Linux'], | ||
| [ImageOs.FreeBsd, 'FreeBSD'], | ||
| [ImageOs.Windows, 'Windows'], | ||
| [ImageOs.ArchLinux, 'Arch Linux'], |
There was a problem hiding this comment.
Why is Arch Linux included as a separate entry? Isn't it just linux?
| @@ -86,6 +87,7 @@ export interface CreateVirtualizationInstance { | |||
| zvol_path?: string | null; | |||
| storage_pool: string | null; | |||
| volume?: string | null; | |||
| image_os?: ImageOs | null; | |||
| if (!existingOption && this.allowCustomValue()) { | ||
| existingOption = { label: this.value as string, value: this.value }; | ||
| } | ||
| this.selectedOption = { ...existingOption }; |
There was a problem hiding this comment.
Would be good to add a test case for changes in core components.
There was a problem hiding this comment.
sounds reasonable.
| @@ -1,6 +1,20 @@ | |||
| import { marker as T } from '@biesbjerg/ngx-translate-extract-marker'; | |||
| import { iconMarker } from 'app/modules/ix-icon/icon-marker.util'; | |||
|
|
|||
| export enum ImageOs { | |||
| Linux = 'LINUX', | |||
There was a problem hiding this comment.
Values don't have to be uppercase, since this is not a real enum, just a preset for the user.
There was a problem hiding this comment.
@undsoft - I checked the docs for image_os, and there I found these enums:
| @@ -177,6 +186,7 @@ export class InstanceEditFormComponent { | |||
|
|
|||
| if (this.isVm) { | |||
| payload.secure_boot = values.secure_boot; | |||
| payload.image_os = values.image_os; | |||
There was a problem hiding this comment.
Please check with Rehan from MW if we need to send Linux when user selects Use a Linux image (linuxcontainers.org).
There was a problem hiding this comment.
@undsoft - I tried it, and it says that it should be sent only with ISO volume.
|
|
||
| osImage: { | ||
| tooltip: T('Optionally specify the operating system for VM-based instances.\ | ||
| Common options are Windows, Linux, FreeBSD, or Arch Linux, but you can also enter a custom value.\ |
There was a problem hiding this comment.
Same here. Not sure why Arch Linux is special.
Also include this text:
When creating a Windows VM, make sure to set the this field to
Windows.
Doing so will tell us to expect Windows to be running inside of the virtual machine and to tweak behavior accordingly.
| @@ -776,4 +790,27 @@ export class InstanceWizardComponent implements OnInit { | |||
| } | |||
| }); | |||
| } | |||
|
|
|||
| private detectImageOs(value: string | null | undefined): ImageOs | null { | |||
There was a problem hiding this comment.
Would be nice to add a test case for this.
| @if (form.value.volume_type === VolumeContentType.Iso) { | ||
| <ix-combobox | ||
| formControlName="image_os" | ||
| [label]="'OS Image' | translate" |
There was a problem hiding this comment.
Let's call it OS Type here and in edit form.
Testing:
See ticket.
Quick preview:
Screen.Recording.2025-04-29.at.18.22.52.mov
Downstream