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

New endpoint: job/:id/actions #18690

Merged
merged 9 commits into from
Oct 12, 2023

Conversation

philrenaud
Copy link
Contributor

@philrenaud philrenaud commented Oct 6, 2023

Adds a new endpoint: /job/:id/actions

It parses through a job's taskgroups' tasks, and extracts their actions up to a single flat array with the name of task and taskGroup included as fields.

$ nomad operator api '/v1/job/actions-demo/actions' | jq
[
  {
    "Args": [
      "parrot.live"
    ],
    "Command": "/usr/bin/curl",
    "Name": "party",
    "TaskGroupName": "group",
    "TaskName": "task"
  },
  {
    "Args": [
      "ipinfo.io"
    ],
    "Command": "/usr/bin/curl",
    "Name": "ipinfo",
    "TaskGroupName": "group",
    "TaskName": "task"
  },
  {
    "Args": [
      "parrot.live"
    ],
    "Command": "/usr/bin/curl",
    "Name": "party",
    "TaskGroupName": "group",
    "TaskName": "task2"
  },
...
]

Resolves #18680

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Overall this looks pretty good @philrenaud. I've left some comments on a few areas we can clean things up or improve validation.

command/agent/job_endpoint.go Outdated Show resolved Hide resolved
nomad/mock/job.go Outdated Show resolved Hide resolved
nomad/mock/job.go Outdated Show resolved Hide resolved
nomad/job_endpoint.go Outdated Show resolved Hide resolved
command/agent/job_endpoint_test.go Outdated Show resolved Hide resolved
command/agent/job_endpoint_test.go Outdated Show resolved Hide resolved
command/agent/job_endpoint_test.go Outdated Show resolved Hide resolved
command/agent/job_endpoint_test.go Outdated Show resolved Hide resolved
command/agent/job_endpoint_test.go Outdated Show resolved Hide resolved
t.Fatalf("err: %v", err)
}

// Check the output
Copy link
Member

Choose a reason for hiding this comment

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

Something to consider for this set of assertions is that the slice of actions will always get returned in the same order. So you should be able to compare against a fix slice of outputs. Something like:

must.Len(t, 10, actionsResp3)
must.Eq(t,
	[]string{"date test", "echo test",....},
	helper.ConvertSlice(actionsResp, func(a *Action) string { return a.Name }))
must.Eq(t,
	[]string{"", "g", "g", "",....},
	helper.ConvertSlice(actionsResp, func(a *Action) string { return a.TaskGroupName }))

@philrenaud philrenaud changed the base branch from 18627-scaffolding-task-actions to 18627-task-actions October 11, 2023 20:49
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM!

There's some outstanding comments still but they're basically about style.

command/agent/job_endpoint.go Outdated Show resolved Hide resolved
@philrenaud philrenaud merged commit ae4e343 into 18627-task-actions Oct 12, 2023
7 of 8 checks passed
@philrenaud philrenaud deleted the 18680-action-at-job-level-go branch October 12, 2023 22:41
philrenaud added a commit that referenced this pull request Oct 18, 2023
* 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
philrenaud added a commit that referenced this pull request Oct 20, 2023
* 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>
nvanthao pushed a commit to nvanthao/nomad that referenced this pull request Mar 1, 2024
* 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>
nvanthao pushed a commit to nvanthao/nomad that referenced this pull request Mar 1, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants