Skip to content
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

Add validation on instance create #507

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Oct 24, 2023

Done

  • added instance creation validation:
  • for missing root storage
  • missing network device name and
  • missing storage volume path

Fixes #485

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @lorumic or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • Create an instance without root storage
    • Create an instance with a network device that has no name
    • Create an instance with a custom storage volume that has no mount point

@webteam-app
Copy link

Demo starting at https://lxd-ui-507.demos.haus

@edlerd
Copy link
Collaborator Author

edlerd commented Nov 17, 2023

Hey @piperdeck
I played a bit with your ideas for this and have another iteration ready.

See updated demo link above or the screencast link below for how it is now:

Screencast.from.17.11.2023.13.39.19.webm

Edit: I avoided moving the root storage to the main page of the form, because it would be hidden all the way on the bottom and look a little odd there with the table like view next to the regular form elements.

@edlerd edlerd force-pushed the validate-instance-create branch 8 times, most recently from 3f20b80 to d5874de Compare November 24, 2023 16:20
@piperdeck
Copy link

Discovered another area where we should catch missing information in instance config.

When you create a network device for an instance, it enforces giving the network a name but doesn't actually enforce choosing a network for that network device. It lets you click "save changes" but then throws an error. We should make the "Network" field mandatory as well.

…k device name and missing storage volume path canonical#485

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
Copy link
Collaborator Author

edlerd commented Nov 29, 2023

When you create a network device for an instance, it enforces giving the network a name but doesn't actually enforce choosing a network for that network device. It lets you click "save changes" but then throws an error. We should make the "Network" field mandatory as well.

Good point! I think the solution is to auto-select the 1st network when a network is added. That way, the Network selector can't be in an invalid state. I just pushed a change to do that. Please give the PR another pass @piperdeck

@piperdeck
Copy link

LGTM

@mas-who
Copy link
Contributor

mas-who commented Dec 1, 2023

QA and code both looks good to me

@edlerd edlerd merged commit 97259e8 into canonical:main Dec 1, 2023
6 checks passed
@edlerd edlerd deleted the validate-instance-create branch December 1, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When adding a new network device, device name should be a required field
4 participants