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

csi/ui: show Node Only for volumes when controllers aren't required #9416

Merged
merged 4 commits into from
Nov 25, 2020

Conversation

tgross
Copy link
Member

@tgross tgross commented Nov 20, 2020

Fixes #9252

Plugin health for controllers should show "Node Only" in the UI only when both
conditions are true: controllers are not required, and no controllers have
registered themselves (0 expected controllers). This accounts for "monolith"
plugins which might register as both controllers and nodes but not necessarily have
ControllerRequired = true because they don't implement the Controller RPC
endpoints we need (this requirement was added in #7844)

This changeset includes the following fixes:

  • Update the Plugins tab of the UI so that monolith plugins don't show "Node
    Only" once they've registered.
  • Add the missing "Node Only" logic to the Volumes tab of the UI.

@github-actions
Copy link

github-actions bot commented Nov 20, 2020

Ember Asset Size action

As of 1b5c81c

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +1.08 kB +14 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
nomad-ui.css 0 B 0 B
vendor.css 0 B 0 B

@tgross tgross added this to the 1.0 milestone Nov 20, 2020
@github-actions
Copy link

github-actions bot commented Nov 20, 2020

Ember Test Audit comparison

master 1b5c81c change
passes 1536 1536 0
failures 0 0 0
flaky 0 0 0
duration 8m 04s 284ms 7m 24s 678ms -39s 606ms

Plugin health for controllers should show "Node Only" in the UI only when both
conditions are true: controllers are not required, and no controllers have
registered themselves (0 expected controllers). This accounts for "monolith"
plugins which might register as both controllers and nodes but not necessarily
have `ControllerRequired = true` because they don't implement the Controller
RPC endpoints we need (this requirement was added in #7844)

This changeset includes the following fixes:

* Update the Plugins tab of the UI so that monolith plugins don't show "Node
  Only" once they've registered.
* Add the missing "Node Only" logic to the Volumes tab of the UI.
@tgross tgross marked this pull request as ready for review November 20, 2020 20:34
@tgross tgross marked this pull request as draft November 20, 2020 21:39
@tgross
Copy link
Member Author

tgross commented Nov 20, 2020

Whoops, looks like I need to update the unit test to cover the no-controllers case here. Will hit that first thing Monday. Edit: fixed!

@tgross tgross marked this pull request as ready for review November 23, 2020 15:05
@tgross tgross mentioned this pull request Nov 25, 2020
3 tasks
Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz left a comment

Choose a reason for hiding this comment

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

Nice work on this!

Most of my comments are stylistic things, but there is one genuine bug I spotted in the tests.

ui/app/templates/csi/plugins/index.hbs Outdated Show resolved Hide resolved
ui/app/templates/csi/volumes/index.hbs Outdated Show resolved Hide resolved
ui/tests/acceptance/volumes-list-test.js Outdated Show resolved Hide resolved
ui/tests/acceptance/volumes-list-test.js Outdated Show resolved Hide resolved
ui/tests/acceptance/volumes-list-test.js Outdated Show resolved Hide resolved
Co-authored-by: Michael Lange <dingoeatingfuzz@gmail.com>
@tgross tgross merged commit 101ae73 into master Nov 25, 2020
@tgross tgross deleted the b-ui-csi-volume-node-only branch November 25, 2020 19:50
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSI volume shows controller health as unhealthy even when no controller is needed
2 participants