[ui] Disconnected Clients: "Unknown" allocations in the UI #12544
Conversation
philrenaud
requested review from
DingoEatingFuzz,
DerekStrickland and
ChaiWithJai
| // from being visually centered | ||
| &:nth-child(2n + 1):last-child { | ||
| margin-right: calc(35% + 0.75em); | ||
| } |
There was a problem hiding this comment.
Without this rule:
With this rule:
| @@ -211,6 +211,7 @@ export default class ClientController extends Controller.extend( | |||
| { key: 'complete', label: 'Complete' }, | |||
| { key: 'failed', label: 'Failed' }, | |||
| { key: 'lost', label: 'Lost' }, | |||
| { key: 'unknown', label: 'Unknown' }, | |||
There was a problem hiding this comment.
This change (and similar changes to jobs/job/allocations controller, etc.) all correspond to these filters:
Ember Asset Size actionAs of 9f8ae31 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
philrenaud
force-pushed
the
12486-disconnected-clients-ui-update-client-statuses
branch
from
d2187e4 to
25c57c6
Compare
There was a problem hiding this comment.
Excellent job! I think there's a few things we should discuss. I like to use a bottoms-up approach starting with views (chatting about components if necessary), then moving up routes/controllers, and then models + services and then tie it all back with tests.
The following views are impacting by the surface area of this change, which sends back a new status on the Allocation and Node models:
- ui/clients
- /ui/clients/:id
- /ui/allocations/:id
- /ui/jobs/:id
- /ui/jobs/:id/allocations
- /ui/jobs/:id/deployments
- /ui/topology
I'm unsure if we're addressing the changes in /clients or allocations.
What should we do about the Topology Visualization?
@DerekStrickland did a great job spotting this out in his RFC. And I think, during the design process we missed talking about impact to the Topology visualization which visualizes Allocations and decorates them based on their status (starting, running, or failed).
We should turn back to @mikenomitch and @juliezzhou regarding this.
How should we handle the new Allocation clientStatus of unknown and the Node being in a state of disconnected instead of down?
This issue is more than a color-swatch but about getting an inventory of derived state computations that will be used throughout the application via related Ember Data models and services (our Stats Tracker Registry). I don't think there will be too much to look into re: Stats Tracker Registry at a cursory glance, but just wanted to highlight all of the possible places we should be looking.
How can I set this environment up myself?
I think we'll want to include some documentation about how to reproduce this without Mirage. @DerekStrickland is there any documentation about setting this environment up?
There are certain actions the user can take against nodes and allocations such as setting node eligibility and draining the node in the UI, and we'll need intense testing around these possible behaviors.
| @@ -74,6 +74,7 @@ module('Unit | Util | JobClientStatus', function () { | |||
| notScheduled: [], | |||
| queued: [], | |||
| starting: [], | |||
| unknown: [], | |||
There was a problem hiding this comment.
thought: should we write a unit test for unknown statuses and how it can impact our computation?
| @@ -18,6 +18,7 @@ export default Factory.extend({ | |||
| Running: faker.random.number(10), | |||
| Starting: faker.random.number(10), | |||
| Lost: faker.random.number(10), | |||
| Unknown: faker.random.number(10), | |||
There was a problem hiding this comment.
question: Should this impact all jobs?
In the edge case, we're developing this feature to enable those users to maintain their up-time requirements. That makes me feel like this situation is only regarding service jobs which are long-standing jobs compared to system and batch jobs.
cc: @DerekStrickland @mikenomitch do I have this to be, correct?
There was a problem hiding this comment.
System, sysbatch, service and batch jobs can all transition to unknown.
There was a problem hiding this comment.
Yeah all of them can be unknown.
Not important for the code, but actually in some ways batch is more vital than service. Since batch jobs are "stateful" more often that service, the cost of restarting them can often be higher. So losing connection and coming back gracefully is more important.
There is! You can follow the steps outlined here for setting up a Vagrant environment. I can also help support you as you work through that process if it would help. Another alternative is to set up an E2E environment using our E2E test terraform. The README is pretty awesome. If you need/want to do it together just let me know. Once you have the environment up and running, you can find some sample jobs and basic instructions in this repo. I sent you an invite. The instructions in the root readme should be G2G for the basic use case. They are geared towards the Vagrant environment, so if you go with E2E you'll have to make some adjustments. Just let me know if you need help. |
Ember Test Audit flaky testsEmber Test Audit detected these flaky tests on main:
Ember Test Audit detected these flaky tests on 9f8ae31:
|
philrenaud
deleted the
12486-disconnected-clients-ui-update-client-statuses
branch
|
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. |
Resolves #12485
Resolves #12486
Adds "Unknown" as a valid status in allocation tables, to the allocation status bar, and the allocation legend.
