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

[ui] node eligibilty taken into consideration when clients list filtered to "ready" #18607

Merged

Conversation

philrenaud
Copy link
Contributor

@philrenaud philrenaud commented Sep 28, 2023

On our clients list page, we abstract node readiness, eligibility, and draining status into one State filter.

This leads to situations where a user selects "Ready" and expects to see eligible nodes, but a node can technically be "ready" by status and marked ineligible for new tasks.

This seems confusing with the abstraction in place. As such, selecting "Ready" will not filter out any ineligible nodes.

In resolving this, we've made some updates to the way the State filter works, relative to the others:

  • It is an "inclusive" filter, with all options selected by default (others are exclusive until something is clicked)
  • query parameters from it are individually appended to the URL, where in other dropdowns a stringified array of all checked options is appended
  • it includes subsections for Status, Eligibility, and Drain status, where others are single-subject filters.

We realize this creates an inconsistency, but felt like this represents a practical approach to making state filter behaviour here work as expected.

image

Resolves #18593

@philrenaud philrenaud self-assigned this Sep 28, 2023
@philrenaud philrenaud linked an issue Sep 28, 2023 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Sep 28, 2023

Ember Test Audit comparison

main 8495b46 change
passes 1540 1540 0
failures 0 0 0
flaky 0 0 0
duration 10m 48s 517ms 11m 00s 228ms +11s 711ms

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

I agree this likely aligns with user intent, as our placing "ready" next to "ineligible" in the drop-down, with no other explicitly positive option, suggests that "ready" = "eligible" 👍

@philrenaud philrenaud marked this pull request as draft September 29, 2023 19:18
@philrenaud
Copy link
Contributor Author

Note: converting back to draft after realizing how much test logic is tied up in state/eligibility/drain status.
A more holistic approach in the future will look like this:
image

and untie eligibility/drain status as "status" concepts / list them as tags of the node name, rather than under "state" (which should be renamed Status, to match CLI/API)

@philrenaud philrenaud force-pushed the 18593-ui-filtering-clients-on-ready-displays-ineligible-hosts branch from 5e71595 to 8495b46 Compare December 19, 2023 21:13
@philrenaud philrenaud added the backport/1.7.x backport to 1.7.x release line label Dec 19, 2023
@philrenaud philrenaud merged commit e26c2e2 into main Dec 19, 2023
13 checks passed
@philrenaud philrenaud deleted the 18593-ui-filtering-clients-on-ready-displays-ineligible-hosts branch December 19, 2023 21:40
Copy link

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 Jan 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.7.x backport to 1.7.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: filtering clients on "ready" displays ineligible hosts
2 participants