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

feat: limit functionalities for storage pool with driver that's not fully supported in the UI [WD-9345] #708

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

mas-who
Copy link
Collaborator

@mas-who mas-who commented Mar 22, 2024

Done

  • remove volumes tab in storage detail page when pool driver is not supported
  • remove main configuration form menu tab and default to show yaml configs editor when pool driver is not supported
  • filter out storage pool selector options with unsupported drivers (applies to creating custom volumes, selecting root storage pool for instance and profile, creating custom volume while creating instance, uploading custom iso)

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 @mas-who or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • Setup a ceph cluster for the lxd host server
    • Then run lxc storage create cephfs-pool cephfs source=my-filesystem cephfs.create_missing=true cephfs.data_pool=my-data cephfs.meta_pool=my-metadata to create a cephfs storage pool.
    • Test visiting storage pool detail configurations page for the cephfs-pool. Ensure the volumes tab is gone, ensure main configurations form menu item is gone, try updating the storage pool config using the yaml editor (cephfs.create_missing config can be changed without negative consequences)
    • Try create a custom volume, the storage pool selection should not contain cephfs-pool.
    • Try upload a custom iso, the storage pool selection should not contain cephfs-pool.
    • Try creating an instance or a profile, go to disk devices section, the root storage pool selector should not contain cephfs-pool as an option

@webteam-app
Copy link

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

@@ -219,7 +223,7 @@ const StoragePoolForm: FC<Props> = ({ formik, section, setSection }) => {
readOnly={formik.values.readOnly}
>
<Notification severity="information" title="YAML Configuration">
This is the YAML representation of the storage pool.
{`${!supportedStorageDrivers.has(formik.values.driver) ? `The ${formik.values.driver} driver is not fully supported in the web interface. ` : ""}This is the YAML representation of the storage pool.`}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought it would be good to add some info here indicating that the driver for the storage pool is not fully supported in the UI. Open to suggestions for better copy 🙂

@mas-who mas-who changed the title feat: limit functionalities for storage pool with driver that's not fully supported in the UI [WD-9761] feat: limit functionalities for storage pool with driver that's not fully supported in the UI [WD-9345] Mar 22, 2024
@mas-who
Copy link
Collaborator Author

mas-who commented Mar 22, 2024

@piperdeck I think the easiest way to do a design review for this PR is to jump on a call and go through the changes. Let me know when you have some time?

src/util/storageOptions.tsx Outdated Show resolved Hide resolved
src/pages/storage/StoragePoolSelector.tsx Show resolved Hide resolved
src/pages/storage/StoragePoolDetail.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StoragePoolFormMenu.tsx Outdated Show resolved Hide resolved
@mas-who mas-who force-pushed the lxd-unsupported-storage-driver branch from fbad9f9 to a8ad838 Compare March 22, 2024 17:31
@mas-who mas-who requested a review from edlerd March 22, 2024 17:32
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more ideas to simplify flow and clarify on naming.

src/pages/storage/EditStoragePool.tsx Outdated Show resolved Hide resolved
src/pages/storage/StoragePoolSelector.tsx Outdated Show resolved Hide resolved
src/components/forms/DiskDeviceFormRoot.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StoragePoolForm.tsx Outdated Show resolved Hide resolved
src/util/storageOptions.tsx Outdated Show resolved Hide resolved
@mas-who mas-who force-pushed the lxd-unsupported-storage-driver branch from a8ad838 to 9763c81 Compare March 25, 2024 15:30
@mas-who
Copy link
Collaborator Author

mas-who commented Mar 25, 2024

Had a design review session with @piperdeck, everything is okay on that front.

…ully supported in the UI

- remove volumes tab in storage detail page
- remove main configuration form menu tab and default to show yaml configs editor
- filter out storage pool options with unsupported drivers (applies to creating custom volumes, selecting root storage pool for instance and profile, creating custom volume while creating instance, uploading custom iso)

Signed-off-by: Mason Hu <mason.hu@canonical.com>
@mas-who mas-who force-pushed the lxd-unsupported-storage-driver branch from 9763c81 to 87cb136 Compare March 25, 2024 15:32
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fixes :)

@mas-who mas-who merged commit f598acc into canonical:main Mar 25, 2024
10 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 25, 2024
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.

4 participants