UI: CSI namespace bugs #7896
Merged
UI: CSI namespace bugs #7896
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
backspace
approved these changes
May 8, 2020
There was a problem hiding this comment.
Pending the rebase, I reviewed using this comparison URL. This looks good to me, a clean and well-tested intervention.
Changing namespaces can be done anywhere in the app even though many Nomad resources aren't namespace-sensitive (e.g., clients, plugins). A user changing namespaces is an intent to reset context, "now I want to begin a task that relates to Namespace X". Where that task begins used to always be the Jobs list, since it was the only namespace sensitive resource. Now with CSI Volumes, "square 1" is Volumes if the namespace is changed from a storage page.
DingoEatingFuzz
force-pushed
the
f-ui/csi-namespace-issue
branch
from
6427dcb
to
804029c
Compare
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This builds off of #7895. Only the last two commits are original. This should be rebased and merged second.
There were a couple namespace quirks that the UI wasn't handling well. The combination of the two meant it was impossible to view CSI volumes for any namespace other than Default unless you put the namespace query param in the URL 😳
Bug 1: Switching namespaces shouldn't always redirect to Jobs
This made sense when Jobs were the only namespace-sensitive resource, but volumes are too. Now the UI will reset to the Jobs list for the Volumes list depending on where you are in the app. If you are on a storage page, go to volumes, otherwise, go to jobs.
Bug 2: Setting a namespace on the Volumes or Jobs page didn't carry over to the other page
Switching to, say, Namespace 1 when looking at jobs successfully sets the active namespace to Namespace 1, but if you then click on the Storage nav link, the namespace is reset to Default since they have independent query params. Now the namespace query param is provided in the gutter menu link-tos
Since switching namespaces means being forced to the jobs page, and then revisiting storage means resetting the namespace back to Default, you can see how the two bugs work in concert to lock people out of seeing non-default namespaced volumes.