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

Remove kubectl run generators #87077

Merged
merged 1 commit into from Jan 29, 2020
Merged

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Jan 10, 2020

What type of PR is this?
/kind api-change
/kind cleanup
/kind deprecation
/priority backlog
/sig cli

What this PR does / why we need it:
Over a year ago we've deprecated all the generators from under kubectl run command except for the one responsible for generating a pod (see #68132).
This PR removes all the generators and deprecates all the flags towards the goal to make kubectl run simpler.

Special notes for your reviewer:
/assign @liggitt @seans3 @pwittrock

Does this PR introduce a user-facing change?:

Remove all the generators from kubectl run. It will now only create pods. Additionally, deprecates all the flags that are not relevant anymore.

/cc @kubernetes/sig-cli-api-reviews @kubernetes/sig-cli-pr-reviews

@k8s-ci-robot k8s-ci-robot added the release-note label Jan 10, 2020
@k8s-ci-robot k8s-ci-robot added kind/api-change size/XXL kind/cleanup sig/cli kind/deprecation cncf-cla: yes priority/backlog approved area/kubectl area/test sig/testing labels Jan 10, 2020
@soltysh
Copy link
Contributor Author

@soltysh soltysh commented Jan 10, 2020

/hold
for visibility

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold label Jan 10, 2020
@soltysh soltysh force-pushed the remove_run_generators branch from 0d480d2 to caee7ab Compare Jan 10, 2020
@soltysh
Copy link
Contributor Author

@soltysh soltysh commented Jan 10, 2020

/sig architecture
@kubernetes/sig-architecture-api-reviews since this is dropping a few of conformance tests

@k8s-ci-robot k8s-ci-robot added sig/architecture area/conformance and removed approved labels Jan 10, 2020
Testname: Kubectl, run deployment
Description: Command ‘kubectl run’ MUST create a deployment, with --generator=deployment, when a image name is specified in the run command. After the run command there SHOULD be a deployment that should exist with one container running the specified image. Also there SHOULD be a Pod that is controlled by this deployment, with a container running the specified image.
*/
framework.ConformanceIt("should create a deployment from an image ", func() {
Copy link
Member

@liggitt liggitt Jan 10, 2020

Choose a reason for hiding this comment

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

I'm surprised we had a conformance test using deprecated function... that shouldn't really have happened

Copy link
Contributor Author

@soltysh soltysh Jan 10, 2020

Choose a reason for hiding this comment

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

yeah, my bad I didn't catch it earlier 😞

@soltysh soltysh force-pushed the remove_run_generators branch 2 times, most recently from dbc8c0a to e1da66b Compare Jan 10, 2020
@soltysh
Copy link
Contributor Author

@soltysh soltysh commented Jan 10, 2020

/retest

@sftim
Copy link
Contributor

@sftim sftim commented Jan 11, 2020

Does this need an issue to remove / revise documentation about generators from the website?
(there might be one already)

@sftim
Copy link
Contributor

@sftim sftim commented Jan 11, 2020

Thinking about this, any website page that recommends using kubectl run will need to change. There might be some left.

@soltysh
Copy link
Contributor Author

@soltysh soltysh commented Jan 14, 2020

Does this need an issue to remove / revise documentation about generators from the website?
(there might be one already)

Yes, docs pr will follow.

@k8s-ci-robot k8s-ci-robot added the needs-rebase label Jan 14, 2020
@alexellis
Copy link

@alexellis alexellis commented May 12, 2020

Now that kubectl run has effectively been deprecated for creating deployments, I'm working through lots of docs to try and convert previously-working examples to the new syntax.

I am running into a bit of a stumbling-block here as there doesn't seem to be an equivalent to:

kubectl run nginx-1 --image=nginx --port=80 --restart=Always

This is how far I got before hitting a bit of a wall:

space-mini:docs alex$ kubectl create deployment nginx-1 --image=nginx --port=80 --restart=Always
Error: unknown flag: --port
See 'kubectl create deployment --help' for usage.
space-mini:docs alex$ kubectl create deployment nginx-1 --image=nginx  --restart=Always
Error: unknown flag: --restart
See 'kubectl create deployment --help' for usage.
space-mini:docs alex$ 

The only alternative I can think of that makes sense is to have a YAML file which is honestly going to be much more repetitive and harder to maintain.

Does anyone have any suggestions on how to convert samples like this?

@alexellis
Copy link

@alexellis alexellis commented May 12, 2020

Here's an example of what we are going to need to put pretty much everywhere kubectl run was used for deployments. It seems rather unfortunate that "improving" the UX has made Kubernetes harder to use for project maintainers.

First create a deployment for Nginx.

For Kubernetes 1.17 and lower:

kubectl run nginx-1 --image=nginx --port=80 --restart=Always

For 1.18 and higher:

export DEPLOYMENT=nginx-1

(cat<<EOF
apiVersion: apps/v1
kind: Deployment
metadata:
  name: $DEPLOYMENT
  labels:
    app: nginx
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.14.2
        ports:
        - containerPort: 80
EOF
) | kubectl apply -f -

alexellis added a commit to inlets/inlets-operator that referenced this issue May 12, 2020
In 1.18 the "run" command has effectively been removed
and can no longer generate deployments as before. We now have to
include large snippets of YAML in documentation.

kubernetes/kubernetes#87077

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@caruccio
Copy link

@caruccio caruccio commented May 13, 2020

You guys are lucky this is not a Linus Torvalds' project.

@NinjaPanda91
Copy link

@NinjaPanda91 NinjaPanda91 commented May 13, 2020

Can we revert this PR ?
--port, --replicas flags and a bunch of flags are missing and I'm studying for CKA exam and i just found out it uses 1.18.

We can raise this back once I'm done with the exam.

@sftim
Copy link
Contributor

@sftim sftim commented May 13, 2020

To be clear, the deprecation occurred in #68132 based on SIG CLI's consensus-based decision making.
If you'd like to enhance kubectl create, or to suggest enhancements, I feel that those suggestions are much more likely to be accepted.

@soltysh
Copy link
Contributor Author

@soltysh soltysh commented May 14, 2020

#91113 should solve the issue @alexellis hope this helps your case 😺

@alexellis
Copy link

@alexellis alexellis commented May 14, 2020

Nice to have port back, what about the other key flags?

@sftim
Copy link
Contributor

@sftim sftim commented May 14, 2020

To suggest an enhancement for kubectl, you can visit https://github.com/kubernetes/kubectl/issues/new (and I hope that people will make these suggestions)

@supirman
Copy link

@supirman supirman commented May 16, 2020

Can we revert this PR ?
--port, --replicas flags and a bunch of flags are missing and I'm studying for CKA exam and i just found out it uses 1.18.

We can raise this back once I'm done with the exam.

I was taking some Qwiklabs labs, some of kubertes lab rely on kubectl run. It seem that kubectl in Google Cloud Shell got update recently and the lab not updated to this issue.

For the workaround, you can just use the old version where kubectl run still working in old ways.

wking pushed a commit to wking/kubernetes that referenced this issue Jul 21, 2020
Remove kubectl run generators

Kubernetes-commit: dba8d60
@tm1810
Copy link

@tm1810 tm1810 commented Aug 29, 2020

kubectl run for deployments is being missed, can we get the features back in create please, this will make a lot of difference

@armujahid
Copy link

@armujahid armujahid commented Aug 29, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/conformance area/kubectl area/test cncf-cla: yes kind/api-change kind/cleanup kind/deprecation lgtm priority/backlog release-note sig/architecture sig/cli sig/testing size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet