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

Deprecate --export flag from get command #73787

Merged
merged 1 commit into from Feb 14, 2019

Conversation

@soltysh
Copy link
Contributor

soltysh commented Feb 6, 2019

What type of PR is this?
/kind api-change
/sig cli
/sig architecture
/priority important-longterm

What this PR does / why we need it:
kubectl get --export was broken almost from its inception, multiple bugs (eg. #28551) were reported wrt its functionality but not much was addressed.
Additionally, server-side export (#24855) was brought for discussion and most of the time sig-apimachinery decided not to do it.
Given all of the above I think it's the time to start the process of deprecating this functionality.

Special notes for your reviewer:
/assign @deads2k @liggitt
/cc @kubernetes/api-approvers

Does this PR introduce a user-facing change?:

Deprecate --export flag from kubectl get command.
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 6, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@juanvallejo

This comment has been minimized.

Copy link
Member

juanvallejo commented Feb 6, 2019

/lgtm

@soltysh

This comment has been minimized.

Copy link
Contributor Author

soltysh commented Feb 6, 2019

/hold
I'd like to hear from others too, before we merge this

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Feb 6, 2019

I'm in favor of deprecation for the reasons stated, will leave open for comment

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Feb 6, 2019

@bgrant0607

This comment has been minimized.

Copy link
Member

bgrant0607 commented Feb 6, 2019

I am in favor also.

We need to totally rethink how to support this use case.

@sebgoa

This comment has been minimized.

Copy link
Member

sebgoa commented Feb 7, 2019

This is a super useful/handy command to help folks move from imperative to declarative.

The nodeport issue is not that problematic and the other issue mentioned dates back April 2016.

I am not in favor, fwiw.

@prabhakar-pal

This comment has been minimized.

Copy link

prabhakar-pal commented Feb 7, 2019

This is a useful feature, is it possible to consider fixing the bugs?

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Feb 7, 2019

This is a useful feature, is it possible to consider fixing the bugs?

The bugs are secondary. The primary issue is that there are multiple conflicting goals and use cases for this feature that require opposite behavior. Some require dropping cluster-specific info, some require preserving it.

I don't think the current approach to export is consistent or general enough to promote.

@sebgoa

This comment has been minimized.

Copy link
Member

sebgoa commented Feb 7, 2019

Before deprecating could we have a documented alternative.

End users are already facing the deprecation of kubectl run as a wrapper to create a deployment before kubectl create deployment is ready. (different story I am aware but another CLI deprecation).

Any object can certainly be cleaned up by hand without too much hassle but --export was a quick way to get that done.

Since the issues mentioned as justification go back 2016, might as well solve it first once and for all before deprecating this.

my 2cts.

@soltysh

This comment has been minimized.

Copy link
Contributor Author

soltysh commented Feb 7, 2019

Before deprecating could we have a documented alternative.

Like mentioned in the email kubectl get -oyaml is the alternative. You can post-process the resource as you wish.

@sebgoa

This comment has been minimized.

Copy link
Member

sebgoa commented Feb 7, 2019

Like mentioned in the email kubectl get -oyaml is the alternative. You can post-process the resource as you wish.

I read the email, thank you.

@soltysh

This comment has been minimized.

Copy link
Contributor Author

soltysh commented Feb 14, 2019

I think this had sufficient soak time.
/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 120bcd7 into kubernetes:master Feb 14, 2019

16 checks passed

cla/linuxfoundation soltysh authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details

@soltysh soltysh deleted the soltysh:deprecate_export branch Feb 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.