[ui] Actions implementation in the web UI #18793
Merged
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
Ember Test Audit comparison
|
philrenaud
force-pushed
the
18735-socket-endpoint
branch
from
fb1f31a
to
0a308a1
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
ef434de
to
b89d27a
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
477cfb4
to
14bec47
Compare
philrenaud
changed the title
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
2 times, most recently
from
b14f50e
to
b23a7f2
Compare
Merged
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
b23a7f2
to
10a21d1
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
eb50790
to
c34b2e1
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
c34b2e1
to
44bfe6b
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
f182b8d
to
aec93d4
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
aec93d4
to
c86c7a0
Compare
DingoEatingFuzz
approved these changes
Nov 7, 2023
There was a problem hiding this comment.
Super awesome work!
I think the headline for me (granted I haven't been in this code in a minute) is that there is a lot of exposure to websocket and xterm details in the action adapter.
There's a good chance that code never has to be looked at again, so I don't want to go so far as to say it should be refactored, but since we already have some of those utils for full-blown exec, maybe there is an opportunity to organize some of the common code.
ui/app/adapters/job.js
Outdated
Comment on lines
174
to
191
| const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; | ||
| const region = this.system.activeRegion; | ||
| const applicationAdapter = getOwner(this).lookup('adapter:application'); | ||
| const prefix = `${ | ||
| applicationAdapter.host || window.location.host | ||
| }/${applicationAdapter.urlPrefix()}`; | ||
|
|
||
| const wsUrl = | ||
| `${protocol}//${prefix}/job/${encodeURIComponent(job.get('id'))}/action` + | ||
| `?namespace=${job.get('namespace.id')}&action=${ | ||
| action.name | ||
| }&allocID=${allocID}&task=${action.task.name}&group=${ | ||
| action.task.taskGroup.name | ||
| }&tty=true&ws_handshake=true` + | ||
| (region ? `®ion=${region}` : ''); | ||
|
|
||
| // const socket = new WebSocket(wsUrl); | ||
| let socket; |
There was a problem hiding this comment.
Does it make sense to eventually try to use the sockets service here?
There was a problem hiding this comment.
Ended up starting/stopping on this a couple times today; I think it's a good idea and will be marking it a TODO but I'm worried about drift from the exec implementation enough that I want to give it more consideration.
Comment on lines
+249
to
+251
| JSON.stringify({ | ||
| tty_size: { width: 250, height: 100 }, // Magic numbers, but they pass the eye test. | ||
| }) |
There was a problem hiding this comment.
I believe this is what we use the xterm-addon-fit for over in ExecTerminal.
| } | ||
|
|
||
| if (job.groupAllocCount) { | ||
| groupProps.count = job.groupAllocCount; |
There was a problem hiding this comment.
I suppose it was only a matter of time before this needed to be disambiguated.
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
c86c7a0
to
7bf345a
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
56f1cfa
to
e828a3c
Compare
philrenaud
force-pushed
the
18782-implementation-on-job-index
branch
from
e828a3c
to
036d61e
Compare
nvanthao
pushed a commit
to nvanthao/nomad
that referenced
this pull request
Mar 1, 2024
* runAction model and adapter funcs * Hacky but functional action running from job index * remove proxy hack * runAction added to taskSubRow * Added tty and ws_handshake to job action endpoint call * delog * Bunch of streaming work * action started, running, and finished notification titles, neutral color, and ansi escape * Handle random alloc selection in the web ui * Run on All implementation in web ui * [ui] Helios two-step button and uniform title bar for Actions (hashicorp#18912) * Initial pass at title bar button uniformity * Vertical align on actions dropdown toggle and small edits to prevent keynav overflow issue * We represent loading state w text and disable now * Pageheader component to align buttons * Buttons standardized * Actions dropdown reveal for multi-alloc job * Notification code styles * An action-having single alloc job * Mirageed * Actions-laden jobs in mirage * Separating allocCount and taskCount in mirage mocks * Unbreak stop job tests * Permissions for actions dropdown * tests for running actions from the job index page * running from a task row actions tests * some todocleanup * PR feedback addressed, including page helper for actions
nvanthao
pushed a commit
to nvanthao/nomad
that referenced
this pull request
Mar 1, 2024
* runAction model and adapter funcs * Hacky but functional action running from job index * remove proxy hack * runAction added to taskSubRow * Added tty and ws_handshake to job action endpoint call * delog * Bunch of streaming work * action started, running, and finished notification titles, neutral color, and ansi escape * Handle random alloc selection in the web ui * Run on All implementation in web ui * [ui] Helios two-step button and uniform title bar for Actions (hashicorp#18912) * Initial pass at title bar button uniformity * Vertical align on actions dropdown toggle and small edits to prevent keynav overflow issue * We represent loading state w text and disable now * Pageheader component to align buttons * Buttons standardized * Actions dropdown reveal for multi-alloc job * Notification code styles * An action-having single alloc job * Mirageed * Actions-laden jobs in mirage * Separating allocCount and taskCount in mirage mocks * Unbreak stop job tests * Permissions for actions dropdown * tests for running actions from the job index page * running from a task row actions tests * some todocleanup * PR feedback addressed, including page helper for actions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The Web UI implementation of Nomad Actions
This adds Actions dropdowns to job pages and task rows in tables and jobs where applicable, then runs a websocket query to execute them against a new job/:id/action endpoint.
PR review note: there is quite a bit of noise here from a
groupTaskCounttogroupAllocCountfor in many mirage mocked jobs. This name better reflects what is actually being done in the job factory, as I've added a lever to help separate out task and alloc counts when creating one (this became relevant in order to mock a job with multiple groups with differing numbers of tasks and allocs)Another note on design: the "streaming messages in a code block in a notification" is probably not the end state of the design for this feature, but since it is still in flux, I've opted to move this PR forward and address design changes in the near future.