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

Kubectl Diff path to GA #68526

Open
apelisse opened this Issue Sep 11, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@apelisse
Copy link
Member

apelisse commented Sep 11, 2018

This is describing the steps necessary to move kubectl diff to Beta and GA.

These are the list of features and bugs that need to be addressed before kubectl diff can move to beta and GA.

Please cross-out (do not remove, please) whichever does not seem appropriate for moving to Beta/GA and put your name behind it. If you want to add any notes specific to a task, please list it as a bullet point and again, put your name behind it.

Features

  • Simplify and improve existing interface to be more focused #68525 @apelisse
  • Use server-side dry-run to get the merged version, rather than using Phil's code #69162 @apelisse
  • Move command out of alpha sub-commands #69167 @apelisse
  • Add proper documentation for the command @apelisse
  • Find a few places in the documentation where apply is used, and suggest running diff before, to see what will happen @apelisse

Bugs

  • Check server version or availability of the dry-run parameter to prevent unwanted changes to the server (old servers might ignore the dryRun parameter). #69449 @apelisse
  • Checking dry-run availability isn't done on "create" path #69745 @apelisse
  • Return errors from "diff" are ignored, including if diff(1) doesn't exist #69742 @apelisse
  • KUBERNETES_EXTERNAL_DIFF should be renamed KUBECTL_EXTERNAL_DIFF #69742 @apelise
  • When using dry-run patch, we need to patch against the same specific resourceVersion of the object, or it might diff two different version #71156 @apelisse

Tech Debt

  • Change the way options are created #72204 (suggested by @soltysh)
  • Add more tests in test-diff.sh (suggested by @soltysh)
  • Test that it works fine against CRDs
@apelisse

This comment has been minimized.

Copy link
Member

apelisse commented Sep 11, 2018

/sig cli

@k8s-ci-robot k8s-ci-robot added sig/cli and removed needs-sig labels Sep 11, 2018

@soltysh soltysh assigned soltysh and apelisse and unassigned soltysh Oct 19, 2018

@clux clux referenced this issue Oct 25, 2018

Open

remove reliance on tiller #11

4 of 7 tasks complete
@guineveresaenger

This comment has been minimized.

Copy link
Contributor

guineveresaenger commented Nov 5, 2018

Hi @apelisse - I'm Enhancements Shadow for 1.13. Could you please update the release team with the likelihood of this enhancement making the 1.13 release? It looks close but when we don't see any updates in a few days, we would appreciate an update.

Code slush begins on 11/9 and code freeze is 11/15.

Thank you!

@apelisse

This comment has been minimized.

Copy link
Member

apelisse commented Nov 5, 2018

Hi @guineveresaenger, this feature is pretty much ready, which is why I don't look super active here :-)

@zx8

This comment has been minimized.

Copy link

zx8 commented Nov 21, 2018

@apelisse I've found what I think is a bug:

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"12", GitVersion:"v1.12.2", GitCommit:"17c77c7898218073f14c8d573582e8d2313dc740", GitTreeState:"clean", BuildDate:"2018-10-30T21:39:16Z", GoVersion:"go1.11.1", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"11+", GitVersion:"v1.11.2-gke.18", GitCommit:"5796233393d7bc034428de15191ad3d2eaff95fb", GitTreeState:"clean", BuildDate:"2018-11-08T20:49:08Z", GoVersion:"go1.10.3b4", Compiler:"gc", Platform:"linux/amd64"}

$ helm template . | kubectl alpha diff -f - MERGED
error: missmatching non-nil types for the same field: float64 int64

Not sure how to diagnose this. Anything I can do to help figure out the issue? This was the result of doing a helm template . | kubectl apply -f- followed by the above diff command.

@apelisse

This comment has been minimized.

Copy link
Member

apelisse commented Nov 21, 2018

Hey @zx8, thanks for the feedback! kubectl diff beta that is being released right now is significantly different from that version, and so this bug is not very relevant anymore. Please try the new diff version when 1.13 is released and tell me if you like it!

@zx8

This comment has been minimized.

Copy link

zx8 commented Nov 21, 2018

Oops, my mistake! I just rebuilt from master @ a19bf33 and am getting:

$ helm template . | kubectl diff -f-
error: /v1, Kind=ConfigMap doesn't support dry-run

When commenting out the ConfigMap YAML and trying again, I get:

$ helm template . | kubectl diff -f-
error: apps/v1, Kind=Deployment doesn't support dry-run

I assume this requires the server to be running v1.13+ as well? I don't want to spam this issue with feedback so let me know if this is a legitimate bug and I'll raise it through the appropriate channel.

@apelisse

This comment has been minimized.

Copy link
Member

apelisse commented Nov 21, 2018

Right, you need server-side dry-run (kubernetes/enhancements#576) on the server (it's currently alpha so you might be able to try by enabling the feature-gate)

@apelisse apelisse changed the title Kubectl Diff path to Beta/GA Kubectl Diff path to GA Dec 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment