Scaffolding actions #18639

philrenaud · 2023-10-02T20:55:02Z

First steps in setting up Nomad Actions (#18627)

Establishes the Action struct at task level and allows them to be included in a jobspec
Returns them under tasks on the GET job endpoint

Also includes a (temporary) workaround to get exec working on our local Ember development environment: direct forwarding to :4646. This is marked with FIXMEs and will be removed prior to merge with main.

vercel · 2023-10-02T20:55:13Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nomad-storybook-and-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 2, 2023 8:57pm

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
nomad	⬜️ Ignored (Inspect)			Oct 2, 2023 8:57pm

…ugh to 4646

github-actions · 2023-10-06T14:17:32Z

Ember Test Audit comparison

	18627-task-actions	`817667b`	change
passes	1533	1533	0
failures	1	1	0
flaky	0	0	0
duration	000ms	000ms	-000ms

nomad/structs/structs.go

api/tasks.go

command/agent/job_endpoint.go

Co-authored-by: Tim Gross <tgross@hashicorp.com>

nomad/structs/structs.go

nomad/structs/diff.go

…her diffs

tgross

Looks like we're getting close!

api/tasks.go

command/agent/job_endpoint_test.go

nomad/structs/diff.go

lgfa29

Submitting some early comments. I will try to give a more in-depth review later today.

command/agent/job_endpoint.go

lgfa29 · 2023-10-10T19:05:52Z

nomad/structs/actions.go

+
+import "slices"
+
+type Action struct {


Hum...I feel like this name may be too generic 🤔

The structs package deal with all sorts of stuff and a struct called Action may not be obvious what it relates to. Perhaps TaskAction would be a better name?

I'm not opposed to this (they started out with that name!) but lately we've been talking about Node Actions and similar concepts, which I think would borrow from this a bit. I'm torn!

Are they similar or the same?

Reusing structs for similar concepts may result in weird interfaces, like attributes that are nil in some cases but not in others. And sometimes that's OK. So there's not right answer, just feels at this point.

My vote would be for TaskAction, but I will leave it up to you 😄

nomad/structs/diff.go

…n mind

tgross

LGTM! Just a bit of now-dead code to clean up and this can be merged. Nice work!

nomad/structs/diff.go

lgfa29

Some additional cleanup, but good one!

command/agent/job_endpoint.go

lgfa29 · 2023-10-11T14:07:32Z

nomad/structs/actions.go

+
+import "slices"
+
+type Action struct {


Are they similar or the same?

Reusing structs for similar concepts may result in weird interfaces, like attributes that are nil in some cases but not in others. And sometimes that's OK. So there's not right answer, just feels at this point.

My vote would be for TaskAction, but I will leave it up to you 😄

* Task-level actions for job submissions and retrieval * FIXME: Temporary workaround to get ember dev server to pass exec through to 4646 * Update api/tasks.go Co-authored-by: Tim Gross <tgross@hashicorp.com> * Update command/agent/job_endpoint.go Co-authored-by: Tim Gross <tgross@hashicorp.com> * Diff and copy implementations * Action structs get their own file, diff updates to behave like our other diffs * Test to observe actions changes in a version update * Tests migrated into structs/diff_test and modified with PR comments in mind * APIActionToSTructsAction now returns a new value * de-comment some plain parts, remove unused action lookup * unused param in action converter --------- Co-authored-by: Tim Gross <tgross@hashicorp.com>

* Scaffolding actions (#18639) * Task-level actions for job submissions and retrieval * FIXME: Temporary workaround to get ember dev server to pass exec through to 4646 * Update api/tasks.go Co-authored-by: Tim Gross <tgross@hashicorp.com> * Update command/agent/job_endpoint.go Co-authored-by: Tim Gross <tgross@hashicorp.com> * Diff and copy implementations * Action structs get their own file, diff updates to behave like our other diffs * Test to observe actions changes in a version update * Tests migrated into structs/diff_test and modified with PR comments in mind * APIActionToSTructsAction now returns a new value * de-comment some plain parts, remove unused action lookup * unused param in action converter --------- Co-authored-by: Tim Gross <tgross@hashicorp.com> * New endpoint: job/:id/actions (#18690) * unused param in action converter * backing out of parse_job level and moved toward new endpoint level * Adds taskName and taskGroupName to actions at job level * Unmodified job mock actions tests * actionless job test * actionless job test * Multi group multi task actions test * HTTP method check for GET, cleaner errors in job_endpoint_test * decomment * Actions aggregated at job model level (#18733) * Removal of temporary fix to proxy to 4646 * Run Action websocket endpoint (#18760) * Working demo for review purposes * removal of cors passthru for websockets * Remove job_endpoint-specific ws handlers and aimed at existing alloc exec handlers instead * PR comments adressed, no need for taskGroup pass, better group and task lookups from alloc * early return in action validate and removed jobid from req args per PR comments * todo removal, we're checking later in the rpc * boolean style change on tty * Action CLI command (#18778) * Action command init and stuck-notes * Conditional reqpath to aim at Job action endpoint * De-logged * General CLI command cleanup, observe namespace, pass action as string, get random alloc w group adherence * tab and varname cleanup * Remove action param from Allocations().Exec calls * changelog * dont nil-check acl --------- Co-authored-by: Tim Gross <tgross@hashicorp.com>

* Scaffolding actions (hashicorp#18639) * Task-level actions for job submissions and retrieval * FIXME: Temporary workaround to get ember dev server to pass exec through to 4646 * Update api/tasks.go Co-authored-by: Tim Gross <tgross@hashicorp.com> * Update command/agent/job_endpoint.go Co-authored-by: Tim Gross <tgross@hashicorp.com> * Diff and copy implementations * Action structs get their own file, diff updates to behave like our other diffs * Test to observe actions changes in a version update * Tests migrated into structs/diff_test and modified with PR comments in mind * APIActionToSTructsAction now returns a new value * de-comment some plain parts, remove unused action lookup * unused param in action converter --------- Co-authored-by: Tim Gross <tgross@hashicorp.com> * New endpoint: job/:id/actions (hashicorp#18690) * unused param in action converter * backing out of parse_job level and moved toward new endpoint level * Adds taskName and taskGroupName to actions at job level * Unmodified job mock actions tests * actionless job test * actionless job test * Multi group multi task actions test * HTTP method check for GET, cleaner errors in job_endpoint_test * decomment * Actions aggregated at job model level (hashicorp#18733) * Removal of temporary fix to proxy to 4646 * Run Action websocket endpoint (hashicorp#18760) * Working demo for review purposes * removal of cors passthru for websockets * Remove job_endpoint-specific ws handlers and aimed at existing alloc exec handlers instead * PR comments adressed, no need for taskGroup pass, better group and task lookups from alloc * early return in action validate and removed jobid from req args per PR comments * todo removal, we're checking later in the rpc * boolean style change on tty * Action CLI command (hashicorp#18778) * Action command init and stuck-notes * Conditional reqpath to aim at Job action endpoint * De-logged * General CLI command cleanup, observe namespace, pass action as string, get random alloc w group adherence * tab and varname cleanup * Remove action param from Allocations().Exec calls * changelog * dont nil-check acl --------- Co-authored-by: Tim Gross <tgross@hashicorp.com>

Task-level actions for job submissions and retrieval

9952078

philrenaud self-assigned this Oct 2, 2023

vercel bot deployed to Preview – nomad-storybook-and-ui October 2, 2023 20:57 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui October 6, 2023 14:01 View deployment

philrenaud force-pushed the 18627-scaffolding-task-actions branch from d8291f2 to b4d3e3e Compare October 6, 2023 14:03

philrenaud changed the base branch from main to 18627-task-actions October 6, 2023 14:04

philrenaud force-pushed the 18627-scaffolding-task-actions branch 2 times, most recently from b0d0043 to 2ca821e Compare October 6, 2023 14:04

FIXME: Temporary workaround to get ember dev server to pass exec thro…

a1e439e

…ugh to 4646

philrenaud force-pushed the 18627-scaffolding-task-actions branch from 2ca821e to a1e439e Compare October 6, 2023 14:05

philrenaud requested review from lgfa29 and tgross October 6, 2023 14:05

philrenaud changed the title ~~Scaffolding task actions~~ Scaffolding actions Oct 6, 2023

philrenaud marked this pull request as ready for review October 6, 2023 14:07

vercel bot deployed to Preview – nomad-storybook-and-ui October 6, 2023 14:09 View deployment

tgross reviewed Oct 6, 2023

View reviewed changes

nomad/structs/structs.go Outdated Show resolved Hide resolved

api/tasks.go Outdated Show resolved Hide resolved

command/agent/job_endpoint.go Outdated Show resolved Hide resolved

philrenaud and others added 2 commits October 7, 2023 00:19

Update api/tasks.go

2e77eb9

Co-authored-by: Tim Gross <tgross@hashicorp.com>

Update command/agent/job_endpoint.go

dd4bb5d

Co-authored-by: Tim Gross <tgross@hashicorp.com>

vercel bot deployed to Preview – nomad-storybook-and-ui October 7, 2023 04:25 View deployment

Diff and copy implementations

2fc800f

vercel bot deployed to Preview – nomad-storybook-and-ui October 8, 2023 00:57 View deployment

tgross reviewed Oct 9, 2023

View reviewed changes

nomad/structs/structs.go Outdated Show resolved Hide resolved

nomad/structs/structs.go Outdated Show resolved Hide resolved

nomad/structs/diff.go Outdated Show resolved Hide resolved

nomad/structs/diff.go Outdated Show resolved Hide resolved

Action structs get their own file, diff updates to behave like our ot…

5b68347

…her diffs

vercel bot deployed to Preview – nomad-storybook-and-ui October 10, 2023 15:02 View deployment

Test to observe actions changes in a version update

01ed71e

vercel bot deployed to Preview – nomad-storybook-and-ui October 10, 2023 16:15 View deployment

philrenaud requested a review from tgross October 10, 2023 16:33

tgross reviewed Oct 10, 2023

View reviewed changes

api/tasks.go Outdated Show resolved Hide resolved

command/agent/job_endpoint_test.go Outdated Show resolved Hide resolved

nomad/structs/diff.go Outdated Show resolved Hide resolved

lgfa29 reviewed Oct 10, 2023

View reviewed changes

Tests migrated into structs/diff_test and modified with PR comments i…

2215148

…n mind

vercel bot deployed to Preview – nomad-storybook-and-ui October 10, 2023 21:01 View deployment

philrenaud requested review from lgfa29 and tgross October 10, 2023 21:03

vercel bot deployed to Preview – nomad-storybook-and-ui October 10, 2023 21:06 View deployment

APIActionToSTructsAction now returns a new value

ae20d4c

philrenaud force-pushed the 18627-scaffolding-task-actions branch from c3eed68 to ae20d4c Compare October 11, 2023 13:12

vercel bot deployed to Preview – nomad-storybook-and-ui October 11, 2023 13:14 View deployment

tgross approved these changes Oct 11, 2023

View reviewed changes

nomad/structs/diff.go Outdated Show resolved Hide resolved

nomad/structs/diff.go Outdated Show resolved Hide resolved

de-comment some plain parts, remove unused action lookup

1131dfe

philrenaud force-pushed the 18627-scaffolding-task-actions branch from bef38d4 to 1131dfe Compare October 11, 2023 13:55

vercel bot deployed to Preview – nomad-storybook-and-ui October 11, 2023 14:00 View deployment

lgfa29 approved these changes Oct 11, 2023

View reviewed changes

vercel bot deployed to Preview – nomad-storybook-and-ui October 11, 2023 14:29 View deployment

unused param in action converter

817667b

philrenaud force-pushed the 18627-scaffolding-task-actions branch from f5de743 to 817667b Compare October 11, 2023 14:37

vercel bot deployed to Preview – nomad-storybook-and-ui October 11, 2023 14:40 View deployment

philrenaud merged commit a0a56c9 into 18627-task-actions Oct 11, 2023
27 checks passed

philrenaud deleted the 18627-scaffolding-task-actions branch October 11, 2023 20:36

philrenaud mentioned this pull request Oct 11, 2023

☂️ Nomad Actions #18627

Closed

32 tasks

philrenaud restored the 18627-scaffolding-task-actions branch October 11, 2023 20:46

philrenaud added this to the 1.7.0 milestone Dec 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scaffolding actions #18639

Scaffolding actions #18639

philrenaud commented Oct 2, 2023 •

edited

Loading

vercel bot commented Oct 2, 2023 •

edited

Loading

github-actions bot commented Oct 6, 2023 •

edited

Loading

tgross left a comment

lgfa29 left a comment

lgfa29 Oct 10, 2023

philrenaud Oct 10, 2023

lgfa29 Oct 11, 2023

tgross left a comment

lgfa29 left a comment

lgfa29 Oct 11, 2023

Scaffolding actions #18639

Scaffolding actions #18639

Conversation

philrenaud commented Oct 2, 2023 • edited Loading

vercel bot commented Oct 2, 2023 • edited Loading

github-actions bot commented Oct 6, 2023 • edited Loading

Ember Test Audit comparison

tgross left a comment

Choose a reason for hiding this comment

lgfa29 left a comment

Choose a reason for hiding this comment

lgfa29 Oct 10, 2023

Choose a reason for hiding this comment

philrenaud Oct 10, 2023

Choose a reason for hiding this comment

lgfa29 Oct 11, 2023

Choose a reason for hiding this comment

tgross left a comment

Choose a reason for hiding this comment

lgfa29 left a comment

Choose a reason for hiding this comment

lgfa29 Oct 11, 2023

Choose a reason for hiding this comment

philrenaud commented Oct 2, 2023 •

edited

Loading

vercel bot commented Oct 2, 2023 •

edited

Loading

github-actions bot commented Oct 6, 2023 •

edited

Loading