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 storage volume detail page #472

Merged
merged 1 commit into from
Oct 4, 2023
Merged

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Sep 19, 2023

Done

  • add storage volume detail page
  • add storage volume configuration page, allows editing of volume configuration
  • allow renaming header to have multiple parents

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:
    • check storage volume detail overview
    • check storage volume detail configuration
    • check pages with a rename header

@webteam-app
Copy link

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

@edlerd edlerd force-pushed the volume-detail-page branch 5 times, most recently from e67bea9 to d4811b5 Compare September 26, 2023 06:56
@edlerd edlerd force-pushed the volume-detail-page branch 2 times, most recently from 5c1b82d to 836f0f5 Compare September 27, 2023 14:29
@edlerd edlerd marked this pull request as ready for review September 27, 2023 15:34
@edlerd edlerd force-pushed the volume-detail-page branch 2 times, most recently from d0ca988 to 9c23492 Compare September 27, 2023 16:47
tests/iso-volumes.spec.ts Outdated Show resolved Hide resolved
src/pages/storage/forms/StorageVolumeEdit.tsx Outdated Show resolved Hide resolved
Copy link

@piperdeck piperdeck left a comment

Choose a reason for hiding this comment

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

1

Not sure if this is a result of a change in this particular branch, but it seems like the way that the UI reports that a volume is being used might be broken/misbehaving.

The volume "pool-dir/smol-vol" reports that it is being used by 1 instance:

Pasted image 20230928145118

But when you navigate to that instance, it doesn't report being connected to smol-vol at all:

Pasted image 20230928145228

Pasted image 20230928145242

The volume's name and configurations can't be changed while it's being "used", but I can't figure out how to make it stop being used.

There seem to be two possibilities of what's going on:

  1. The way in which the UI or core determines that something is attached to it is bugged, and that needs attention.
  2. The UI can't communicate certain kinds of connections that block volume customization

2

Unless there's a very specific reason why it's named this way, I would name this configuration tab "Filesystem" rather than "Block", since both of these options are specifically filesystem related.

Pasted image 20230928145848

3

This option here could probably benefit from a little bit of support text, and a link to documentation on mount options such as the Ubuntu manpages. Even for experienced users, it could be nice to have a quick reference here.

Pasted image 20230928145924

@edlerd
Copy link
Collaborator Author

edlerd commented Sep 28, 2023

Hey @piperdeck , thanks for the review!

I addressed 2. and 3.

Regarding 1.

There seem to be two possibilities of what's going on:

  1. The way in which the UI or core determines that something is attached to it is bugged, and that needs attention.
  2. The UI can't communicate certain kinds of connections that block volume customization

This PR does not include the UI for attaching volumes to instances yet. You can see the relation in the YAML view already. So the instance is referencing the volume, just the instance>configuration>storage section can't display it yet. That feature is in this PR and still pending because there are a bunch of issues on it.

@piperdeck
Copy link

Changes look good to me!

@piperdeck
Copy link

It still says I requested changes. I'm not sure how to make my request go away without breaking anything....

Copy link
Member

@aaryanporwal aaryanporwal left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@aaryanporwal
Copy link
Member

@piperdeck if you approve the changes, the request would go away.
Check this doc link on how to do that: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews

@edlerd edlerd merged commit 8940efa into canonical:main Oct 4, 2023
5 checks passed
@edlerd edlerd deleted the volume-detail-page branch October 4, 2023 08:08
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.

None yet

4 participants