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

CLI Allow querying all namespaces for jobs and allocations #8183

Closed
wants to merge 4 commits into from

Conversation

notnoop
Copy link
Contributor

@notnoop notnoop commented Jun 17, 2020

This allows an Enterprise operator to query for lists in jobs and allocations across all namespaces they have capabilities in.

UX

When -all-namespaces command line argument is set, or if NOMAD_ALL_NAMESPACES env var is set to truthy value (e.g. 1, true), nomad job and nomad alloc commands will lookup relevant jobs/allocations in all namespaces they are authorized to.

nomad job status -all-namespaces will emit all the jobs across namespaces (with a namespace column added). nomad job status -all-namespaces foo will return the job info for foo job regardless of which namespace is in; if there multiple foo jobs, the CLI will emit back an error message, indicating the possible values.

# namespace specific status
$ nomad job status
ID        Type   Priority  Status   Submit Date
example1  batch  50        running  2020-06-17T10:49:26-04:00

# list jobs across all namespaces
$ nomad job status -all-namespaces
ID        Namespace  Type   Priority  Status   Submit Date
example1  default    batch  50        running  2020-06-17T10:49:26-04:00
example1  qa         batch  50        running  2020-06-17T10:49:37-04:00
qa-job    qa         batch  50        running  2020-06-17T10:50:28-04:00

# get job status for a job in a specific namespace
$ nomad job status -short ex
ID            = example1
Name          = example1
Submit Date   = 2020-06-17T10:49:26-04:00
Type          = batch
Priority      = 50
Datacenters   = dc1
Namespace     = default
Status        = running
Periodic      = false
Parameterized = false

# when checking multiple namespaces, report an error message if multiples are found
$ nomad job status -short -all-namespaces ex
Prefix matched multiple jobs

ID        Namespace  Type   Priority  Status   Submit Date
example1  default    batch  50        running  2020-06-17T10:49:26-04:00
example1  qa         batch  50        running  2020-06-17T10:49:37-04:00

# but namespace can always be specified to disambiguate
$ nomad job status -short -namespace qa ex
ID            = example1
Name          = example1
Submit Date   = 2020-06-17T10:49:37-04:00
Type          = batch
Priority      = 50
Datacenters   = dc1
Namespace     = qa
Status        = running
Periodic      = false
Parameterized = false

# querying with a prefix if a uniue job is found works
$ nomad job status -short -all-namespaces qa
ID            = qa-job
Name          = qa-job
Submit Date   = 2020-06-17T10:50:28-04:00
Type          = batch
Priority      = 50
Datacenters   = dc1
Namespace     = qa
Status        = running
Periodic      = false
Parameterized = false

Abandoned ideas

Defaulting to -all-namespaces

I considered avoiding the additional argument and defaulting to all namespaces if no --namespace is specified. Such change would be a significant change in behavior that will surprise users.

Using a sentinel namespace value

We could reserve a sentinel namespace value (e.g. --namespace global) to indicate querying all namespaces. However, operators currently can create any namespace, and introducing a special reserved value might collide with an existing namespace and result into confusion.

Implementation Details

The implementation has three main components:

The RPC Handlers for Job.List and Alloc.List accept AllNamespaces argument to indicate whether one should search all namespaces the request token has access to. This PR implements Allocations.List, as Job.List was already updated in #8001 .

Then the API and CLI components need to pass AllNamespaces appropriately. Due to the sheer number of endpoints this affects, I made the flags a global Client Meta/Config options.

The CLI subcommands typically adhere to the following implementation pattern:

  1. attempt to find all job/allocation matches with given prefix - passing AllNamespaces as appropriate
  2. If a job is found, do a specific lookup while passing the namespace found earlier.

As such, {Job|Alloc}.Get RPC endpoints aren't modified, and as before must require the correct namespace being passed.

Incidentally, we didn't need to add any indexes to the state store to accomodate this. Alloc.List already isn't indexed by namespaces, so checking multiple namespaces is roughly the same computationally intensive operation as the namespaced ones.

Potential follow ups

  • We could implement the same logic for Evals as well but I'm uncertain if it's needed, so waiting for some customer feedback.
  • Add tests in the private Enterprise repository. Hard to test this in OSS where namespaces aren't supported.

Mahmood Ali added 4 commits June 17, 2020 09:06
Expose AllNamespaces parameter so one can query all namespaces an
operator has access to
Expose a `-all-namespaces` | `NOMAD_ALL_NAMESPACES` env var, so that the
job subcommands can query jobs across all namespaces.
This implements the backend handling for querying across namespaces for
allocation list endpoints.
@notnoop notnoop added this to the 0.12.0 milestone Jun 17, 2020
@notnoop notnoop self-assigned this Jun 17, 2020
@notnoop notnoop requested a review from yishan-lin June 17, 2020 15:13
@@ -32,6 +32,7 @@ func (s *HTTPServer) AllocsRequest(resp http.ResponseWriter, req *http.Request)
if s.parse(resp, req, &args.Region, &args.QueryOptions) {
return nil, nil
}
args.AllNamespaces, _ = strconv.ParseBool(req.URL.Query().Get("all_namespaces"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to add this into s.parse?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, looking closer into parse it doesn't make as much sense, nvm

schmichael
schmichael previously approved these changes Jun 18, 2020
@schmichael schmichael dismissed their stale review June 18, 2020 22:51

oops, wrong pr

@notnoop
Copy link
Contributor Author

notnoop commented Jun 19, 2020

Superseded by #8192

@notnoop notnoop closed this Jun 19, 2020
@github-actions
Copy link

github-actions bot commented Jan 2, 2023

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 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants