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.

Sign up for GitHub

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

Jump to bottom

Proposal: First-generation of kubectl set * commands #21648

Closed
smarterclayton opened this issue Feb 21, 2016 · 42 comments
Closed

Proposal: First-generation of kubectl set * commands #21648

smarterclayton opened this issue Feb 21, 2016 · 42 comments
Labels
area/kubectl area/usability kind/design Categorizes issue or PR as related to design. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/cli Categorizes an issue or PR as relevant to SIG CLI.
Milestone
UX Backlog - Stac...

Comments

@smarterclayton
Copy link
Contributor

smarterclayton commented Feb 21, 2016 •
edited by 0xmichalis
Loading

For 1.3 we'll be considering adding a number of commands developed upstream in OpenShift into kubectl - specifically, commands that assist the user in performing very common mutations to our core objects.

Current implementations are:

  • Adding or removing volumes from an object with a pod template
  • Adding or removing environment variables from an object with a pod template
  • Adding or removing liveness and readiness probes from objects with a pod template.

As proposed in https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/deploy.md#kubectl-set, these commands would live under kubectl set and operate generically on classes of objects (for the commands above, they would operate on objects with mutable pod templates, for other commands, more generic structure should be possible).

The commands would take as input a JSON/YAML file, a real object on the server, or a synthetic resource generated by looking at multiple server resources, transform the object, and create a patch on the original object that results in the desired mutation. The patch would be applied to the object and then the object mutated on the server.

It should be possible to specify -o yaml and generate a file that can be chained to later commands, for instance:

kubectl set volume -f deployment.yaml --add --claim-name=foo -o yaml | kubectl set probe --liveness --get-url=https://foo.bar -o yaml | kubectl apply -f -

We need to ensure the usability of chaining (both apply, patch, replace, and create are all viable terminations) as part of this work.

In practice, we have found it valuable to have the default mode for the set commands to "list" the current state in a pattern that enables users to guess at what they need to do to change the field. Mutation via CLI flags is hard - each flag must be carefully chosen to balance ease of comprehension with ability to express most of the API. It is acceptable to punt some use cases in mutation to the edit command. Most commands should focus on having a "clear" or "remove" flag that can remove a targeted entry, a simple set of flags that set the default values, and where possible try to support the minimum amount of simultaneous change.

Set commands should have a consistent code pattern to make implementing new setters easy. As much as possible, it should be able to take an existing set command, define a business transformation (adding X to Y) and map that to existing code that is tested in a standard way, following https://github.com/kubernetes/kubernetes/blob/master/docs/devel/kubectl-conventions.md#command-implementation-conventions.

OpenShift implementations of the various commands need varying levels of cleanup:

  • oc set env is the oldest of the downstream implementations and does not support downward API flags at the current time. However, it is tuned very well to make adding and removing structured variables easy. https://github.com/openshift/origin/blob/master/pkg/cmd/cli/cmd/set/env.go
  • oc set volumes is the most complex of the commands today - given the number of volumes we have a choice was made to focus on the most common (claims, empty dir, secrets) and force users to patch other commands. Helpers were added to make creating a claim and adding it to a template very easy. https://github.com/openshift/origin/blob/master/pkg/cmd/cli/cmd/set/volume.go
  • oc set probe is brand new and formalizes some of the more recent kubectl behaviors into a set of core library methods (specifically, iteratively patching a set of objects) as well as handling the transformation pattern cleanly. Add 'oc set probe' for setting readiness and liveness openshift/origin#7331
  • oc set triggers represents a command that exposes a generic interface across two similar, but not identical concepts (build and deployment triggers, which result in a mutation of the object based on external stimulus). It is the most complex because the underlying API is dissimilar, but tries to present a common and simple pattern to users. It explicitly does not handle setting multiple triggers of certain types at a time as an 80% use case solution. Setting deployment and build triggers via CLI openshift/origin#7423
Subcommand PR # Assignee
- [x] image #25509 @janetkuo
- [x] selector #38966 @Kargakis
- [x] serviceaccount #48432 @chandanmad
- [x] env #50998 @zjj2wry
- [ ] probe #52243 @zjj2wry
- [ ] volume
- [x] resources #27206 @JacobTanenbaum
- [ ] security
- [ ] port #51947 @zjj2wry
@smarterclayton smarterclayton added kind/design Categorizes issue or PR as related to design. area/usability labels Feb 21, 2016
@smarterclayton smarterclayton added this to the next-candidate milestone Feb 21, 2016
@bgrant0607 bgrant0607 added area/kubectl team/ux priority/backlog Higher priority than priority/awaiting-more-evidence. labels Feb 22, 2016
@bgrant0607
Copy link
Member

cc @janetkuo @pwittrock

@bgrant0607
Copy link
Member

This sounds pretty good to me, though we might want to break some of the volume functionality into multiple commands.

I'd like set image, which would achieve the equivalent of rolling-update --image when used with Deployment.

Can we chain kubectl labels and kubectl annotations in this way today, or could they be extended to do so?

Nit: I might prefer --dry-run to indicate the mutation shouldn't actually be performed rather than -o yaml. I view -o yaml as just indicating that yaml is requested rather than json.

Even if edit supports reading from stdin, we may want a --edit flag for these and other mutators/generators.

@bgrant0607
Copy link
Member

cc @kubernetes/kubectl

@bgrant0607 bgrant0607 modified the milestones: v1.3, next-candidate Mar 19, 2016
@bgrant0607
Copy link
Member

@ncdc I didn't see this on your 1.3 list. Should it be on the list?

@janetkuo
Copy link
Member

Can we chain kubectl labels and kubectl annotations in this way today, or could they be extended to do so?

Yes we can chain kubectl label and kubectl annotate with -o yaml.

@ncdc
Copy link
Member

ncdc commented Mar 22, 2016

@bgrant0607 it wasn't originally, but I will discuss it with my team and see if we can fit it in.

@bgrant0607
Copy link
Member

We need at least kubectl set image. cc @janetkuo @pwittrock

@smarterclayton
Copy link
Contributor Author

So here's a concrete proposal before I start moving code upstream.

Implement the two most valuable primitives - set image and one other (env?) as a first step, setting up the proper pattern for these commands. Then fan out to the others.

For each one, do an issue that describes the basic syntax - agree on that, then move the impl upstream.

@0xmichalis
Copy link
Contributor

sgtm

@smarterclayton smarterclayton mentioned this issue Apr 21, 2016
Merged
This was referenced May 4, 2016
Merged
Closed
@janetkuo
Copy link
Member

janetkuo commented May 9, 2016

Is anyone planning to work / already working on this?

@smarterclayton
Copy link
Contributor Author

I've got probe and env in a branch. Volumes is more involved, because
it's not as simple.

Some points raised by folks:

  • users want to see the current env or probes. The commands today
    under set handle that (oc set env rc/bar prints the env vars in bash
    form). There was discussion about having another command show that,
    but all the possible locations had trade offs
  • some folks felt using "set X" to remove something was unintuitive
    and suggested "unset X". My concern there is that it scatters
    commands across two hierarchies and reduces discoverability (and some
    commands may not have an unset). Label and annotate today combine set
    and unset
  • for complex commands, like volume, there are enough flags that
    splitting the command was reasonable - except that led to "oc set
    volume add|remove" which could be redundant (or the previous
    discussion about unset)

Will post some examples.

@janetkuo
Copy link
Member

I'll start working on kubectl set image.

@janetkuo janetkuo mentioned this issue May 11, 2016
Merged
@smarterclayton
Copy link
Contributor Author

I'll do probe - probably the least controversial, and the most recent

On May 10, 2016, at 7:52 PM, Janet Kuo notifications@github.com wrote:

I'll start working on kubectl set image.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#21648 (comment)

@smarterclayton
Copy link
Contributor Author

PR for probe is #25451

k8s-github-robot pushed a commit that referenced this issue May 11, 2016
Merge pull request #25444 from janetkuo/kubectl-set
c042156 
Automatic merge from submit-queue

Add 'kubectl set'

## Pull Request Guidelines

1. Please read our [contributor guidelines](https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md).
1. See our [developer guide](https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md).
1. Follow the instructions for [labeling and writing a release note for this PR](https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes) in the block below.

```release-note
```

Ref #21648, the parent `kubectl set` command; will add more sub-command in subsequent PRs. @kubernetes/kubectl 


[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()
@deads2k
Copy link
Contributor

deads2k commented May 11, 2016

Any chance of having kubectl set <foo> take -f - so I could do things like kubectl convert -f foo | kubectl set probe -f - --dry-run > new-file.yaml?

k8s-github-robot pushed a commit that referenced this issue Aug 30, 2017
Merge pull request #50998 from zjj2wry/set-env
4d5fc46 
Automatic merge from submit-queue (batch tested with PRs 51377, 46580, 50998, 51466, 49749)

feat(#21648 )Add kubectl set env command.

**What this PR does / why we need it**:
#21648
Moved from OpenShift to Kubenetes.
@Kargakis @smarterclayton 

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
@bgrant0607
Copy link
Member

configmap set command: #24049

zjj2wry pushed a commit to zjj2wry/kubernetes that referenced this issue Sep 5, 2017
(kubernetes#21648)add kubectl set port command
06f26c7
@zjj2wry zjj2wry mentioned this issue Sep 5, 2017
Closed
@zjj2wry
Copy link
Contributor

zjj2wry commented Sep 5, 2017

@smarterclayton i am working with set probe and lifecycle. it is helpful for user.

@zjj2wry zjj2wry mentioned this issue Sep 10, 2017
Closed
@zjj2wry
Copy link
Contributor

zjj2wry commented Sep 11, 2017

@mengqiy can you help add my name in port , i add kubectl set port command in pr #51947 ,thank you very much

@0xmichalis
Copy link
Contributor

@zjj2wry done

@mengqiy
Copy link
Member

mengqiy commented Sep 12, 2017

@mengqiy can you help add my name in port , i add kubectl set port command in pr #51947 ,thank you very much

Done.

zjj2wry pushed a commit to zjj2wry/kubernetes that referenced this issue Sep 24, 2017
feat(kubernetes#21648) add kubectl set probe command
8094069
@WanLinghao
Copy link
Contributor

@smarterclayton i am working with set volume

@shiywang
Copy link
Contributor

@mengqiy @Kargakis I would like to pick up the last security if no one already did.

@WanLinghao WanLinghao mentioned this issue Oct 20, 2017
Closed
@ericchiang
Copy link
Contributor

PR for set volume is here: #54284

I was assigned but don't have much background. Maybe someone on this thread would be interested in reviewing?

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 25, 2018
@bgrant0607
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 26, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 14, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 14, 2018
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/kubectl area/usability kind/design Categorizes issue or PR as related to design. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/cli Categorizes an issue or PR as relevant to SIG CLI.
Projects
None yet
Milestone
UX Backlog - Stack Ranked
Development

No branches or pull requests