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: standardize kubernetes resource parsing #792

Closed
siggy opened this issue Apr 18, 2018 · 4 comments
Closed

cli: standardize kubernetes resource parsing #792

siggy opened this issue Apr 18, 2018 · 4 comments
Assignees
Labels
Milestone

Comments

@siggy
Copy link
Member

siggy commented Apr 18, 2018

Background

We're working towards new Kubernetes-aware cli commands, specificallly to stat (#627) and tap (#778). We need common patterns to describe Kubernetes objects in cli flags.

Requirements

Both of these commands require input of the following information

  • primary resource
    • namespace
    • type
    • name
  • source / from / destination resource
    • namespace
    • type
    • name

Current state

The APIs currently consume this input via inconsistent flags. Some examples:

stat

# Get all deployments in the test namespace.
conduit stat deployments -n test

# Get the hello1 deployment in the test namespace.
conduit stat deployments hello1 -n test

# Get the test namespace.
conduit stat namespaces test

# Get all deployments.
conduit stat --all-namespaces=true deployments

# Get the hello1 deployment in the test namespace, filtered by requests to ns2 / pod-123
conduit stat deployment hello1 -n test --to-namespace ns2 --to-resource pod --to pod-123

tap

# tap the web deployment in the default namespace
conduit tap deploy default/web

# tap the web-dlbvj pod in the default namespace
conduit tap pod default/web-dlbvj

Proposal

  • All primary/target resource strings take the form (type) [name] or (type)/[name], where name is optional:
    • stat deploy
    • stat deploy/foo
    • stat deploy foo
  • To/from/destination resources always use the compound form:
    • --to deploy
    • --to deploy/foo
    • --from deploy/foo
  • When type != namespace, namespaces are specified by their own flags:
    • --namespace
    • --to-namepace
    • --from-namespace
  • --all-namespaces behaves as it does today

Some examples:

# Get all deployments in the test namespace.
conduit stat -n test deploy

# Get the hello1 deployment in the test namespace.
conduit stat -n test deploy/hello1 

# Get the test namespace.
conduit stat ns/test

# Get all deployments.
conduit stat --all-namespaces=true deployments

# Get the hello1 deployment in the test namespace, filtered by requests to ns2 / pod-123
conduit stat -n test deploy/hello1 --to-namespace ns2 --to pod/pod-123

# tap the web deployment in the default namespace
conduit tap -n default deploy/web

# tap the web-dlbvj pod in the default namespace
conduit tap -n default pod/web-dlbvj
@siggy siggy added the area/cli label Apr 18, 2018
@siggy siggy added this to the 0.4.1 milestone Apr 18, 2018
@klingerf
Copy link
Contributor

Relates to #683 (comment)

@briansmith
Copy link
Contributor

conduit stat -n test deploy/hello1 --to-namespace ns2 --to pod/pod-123

Instead of this, I would prefer (thinking as a user) to use deploy/ns2.hello1 instread of deploy/hello1 --to-namespace ns2, i.e. a syntax of the form <type>/[<namespace>.]<name> where <namespace> defaults to the default namespace that might be overridden with -n.

@klingerf
Copy link
Contributor

I have a strong preference for using an established syntax for referencing these resources, and as far as I know <type>/[<namespace>.]<name> is not something that's used in other kubernetes tooling. If anything, I think <type>/<name>[.<namespace>] would be more consistent with kubernetes naming conventions. But that said, it should also be possible to specify the namespace without specifying a resource name or type, and I don't know how we do that easily without a separate namespace flag.

@briansmith
Copy link
Contributor

I think /[.] would be more consistent with kubernetes naming conventions.

Yes, my mistake.

But that said, it should also be possible to specify the namespace without specifying a resource name or type, and I don't know how we do that easily without a separate namespace flag.

True.

@siggy siggy self-assigned this Apr 19, 2018
siggy added a commit that referenced this issue Apr 23, 2018
The Tap command leveraged new cli parsing code, enabling Kubernetes
resources specified as `(TYPE [NAME | -l label] | TYPE/NAME ...)`. The
Stat command did not use this.

Modify the Stat command to use the same cli flag parsing code as Tap.
Remove the to/from-resource flags.

Fixes #792

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
siggy added a commit that referenced this issue Apr 23, 2018
The Tap command leveraged new cli parsing code, enabling Kubernetes
resources specified as `(TYPE [NAME] | TYPE/NAME)`. The Stat command
did not use this.

Modify the Stat command to use the same cli flag parsing code as Tap.
Remove the to/from-resource flags from Stat.

Fixes #792

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
@siggy siggy added the review label Apr 23, 2018
siggy added a commit that referenced this issue Apr 23, 2018
The Tap command leveraged new cli parsing code, enabling Kubernetes
resources specified as `(TYPE [NAME] | TYPE/NAME)`. The Stat command
did not use this.

Modify the Stat command to use the same cli flag parsing code as Tap.
Remove the to/from-resource flags from Stat.

Fixes #792

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
@siggy siggy removed the review label Apr 23, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants