-
Notifications
You must be signed in to change notification settings - Fork 318
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
NAS-133481 / 25.04 / Changes for VMs in new virtualization #11323
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #11323 +/- ##
=======================================
Coverage 82.86% 82.86%
=======================================
Files 1660 1660
Lines 59423 59440 +17
Branches 6244 6252 +8
=======================================
+ Hits 49241 49257 +16
- Misses 10182 10183 +1 ☔ View full report in Codecov by Sentry. |
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.
The CPU and memory field are not required on the instance wizard. You can leave them empty.
...instances/instance-details/instance-disks/instance-disk-form/instance-disk-form.component.ts
Outdated
Show resolved
Hide resolved
...instances/instance-details/instance-disks/instance-disk-form/instance-disk-form.component.ts
Outdated
Show resolved
Hide resolved
@RehanY147 did you mean to reset value in those fields when user switches from VM to Container? |
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.
I am unclear when we want to choose zvols, can we also select directories? Or does that user has to select a zvol.
/dev/zvol
as a value should not be selectable. That value is meaningless when requiring to select zvols.
Middleware does not allow directories to be used and raises validation error. The |
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.
It's fine the way it works at the moment, we can improve on it later.
This PR has been merged and conversations have been locked. |
Changes:
Testing:
Checks Instance Wizard on Virtualization page.
Downstream