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

Jump to bottom

kubectl apply and edit are broken for TPR #40119

Closed
mengqiy opened this issue Jan 19, 2017 · 9 comments
Closed

kubectl apply and edit are broken for TPR #40119

mengqiy opened this issue Jan 19, 2017 · 9 comments
Assignees
@mengqiy
Labels
area/app-lifecycle area/kubectl priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI.

Comments

@mengqiy
Copy link
Member

mengqiy commented Jan 19, 2017

This issue is found when working on #39906. This is a different issue from #39906. It affects all 1.5.x and master.

The error message is

error: unable to decode "../pkiobj.json": no kind "Pki" is registered for version "box-pki.com/v1"

kubectl apply and edit are broken for TPR in branch 1.5 and master.
The reason is the function call of Decode(). It fails to decode the data.

Anything else do we need to know:
1.4.7 is working, but 1.4.8 is not, because of #39396.
Similar patch #38982 has been introduced in 1.5.2, but both 1.5.1 and 1.5.2 are broken for TPR. apply fails without even reaching the code introduced in #38982.

@liggitt @deads2k @pwittrock @kubernetes/sig-cli-misc
@edsiper
Copy link
Contributor

edsiper commented Jan 19, 2017

+1

kubectl apply is an unrecognized command:

$ kubectl apply -f https://git.io/weave-kube
Error: unknown command "apply"

# kubectl version
Client Version: version.Info{Major:"0", Minor:"20.1", GitVersion:"v0.20.1", GitCommit:"", GitTreeState:"not a git tree"}
Server Version: version.Info{Major:"1", Minor:"5", GitVersion:"v1.5.2", GitCommit:"08e099554f3c31f6e6f07b448ab3ed78d0520507", GitTreeState:"clean"}

@liggitt
Copy link
Member

liggitt commented Jan 19, 2017 •
edited

@edsiper You seem to have a custom built version. Where did you obtain your kubectl?

@edsiper
Copy link
Contributor

edsiper commented Jan 19, 2017

@liggitt

deb package:

ii  kubectl      1.5.1-00     amd64        Kubernetes Command Line Tool

repository:

deb http://apt.kubernetes.io/ kubernetes-xenial main

per instructions from: https://kubernetes.io/docs/getting-started-guides/kubeadm/

@edsiper
Copy link
Contributor

edsiper commented Jan 19, 2017

my "kubectl apply" issue is solved (my bad), I had an old-local/custom kubectl in /usr/local/bin/ messing up the things.

@ahakanbaba ahakanbaba mentioned this issue Jan 19, 2017
Closed
@ahakanbaba
Copy link
Contributor

Thanks @ymqytw for creating this issue.
As a first step, I would be interested in adding tests that reproduce these behavior.
We could add those tests in a disabled way and make sure they run at the end of the fix.

Would you be interested in that approach ?

  1. Add tests that reproduce the behavior
  2. Merge those tests, keep them in a disabled state. Annotate them with this issue.
  3. Work on the fixes.
  4. enable the tests

I am playing around with adding some tests that reproduce these behavior.

Please let me know if you agree/ disagree.

@ahakanbaba
Copy link
Contributor

ahakanbaba commented Jan 19, 2017 •
edited

@huggsboson @kunalparmar

@mengqiy
Copy link
Member Author

mengqiy commented Jan 19, 2017

@ahakanbaba That sounds reasonable to me. But I'm not sure how to add some disabled tests.
Another way is add the test with fix in the same PR.

I am playing around with adding some tests that reproduce these behavior.

Please feel free to create a PR for the tests, if you already have some. cc me in that PR. Thanks.

@bgrant0607 bgrant0607 added area/app-lifecycle area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jan 20, 2017
@liggitt liggitt mentioned this issue Jan 21, 2017
Merged
19 tasks
k8s-github-robot pushed a commit that referenced this issue Jan 27, 2017
Merge pull request #40260 from liggitt/kubectl-tpr
1625150 
Automatic merge from submit-queue (batch tested with PRs 39223, 40260, 40082, 40389)

make kubectl generic commands work with unstructured objects

part of making apply, edit, label, annotate, and patch work with third party resources

fixes #35149
fixes #34413

prereq of:
#35496
#40096

related to:
#39906
#40119

kubectl is currently decoding any resource it doesn't have compiled-in to a ThirdPartyResourceData struct, which means it computes patches using that struct, and would try to send a ThirdPartyResourceData object to the API server when running `apply`

This PR removes the behavior that decodes unknown objects into ThirdPartyResourceData structs internally, and fixes up the following generic commands to work with unstructured objects

- [x] apply
  - [x] decode into runtime.Unstructured objects
  - [x] successfully use `--record` with unregistered objects 
- [x] patch
  - [x] decode into runtime.Unstructured objects
  - [x] successfully use `--record` with unregistered objects 
- [x] describe
  - [x] decode into runtime.Unstructured objects
  - [x] implement generic describer
- [x] fix other generic kubectl commands to work with unstructured objects
  - [x] label
  - [x] annotate

follow-ups for pre-existing issues:
- [ ] `explain` doesn't work with unregistered resources
- [ ] remove special casing of federation group in clientset lookups, etc
- [ ] `patch`
  - [ ] doesn't honor output formats when persisting to server (`kubectl patch -f svc.json --type merge -p '{}' -o json` doesn't output json)
  - [ ] --local throws exception (`kubectl patch -f svc.json --type merge -p '{}' --local`)
- [ ] `apply`
  - [ ] fall back to generic JSON patch computation if no go struct is registered for the target GVK (e.g. #40096)
  - [ ] ensure subkey deletion works in CreateThreeWayJSONMergePatch
  - [ ] ensure type stomping works in CreateThreeWayJSONMergePatch
  - [ ] lots of tests for generic json patch computation
  - [ ] prevent generic apply patch computation among different versions
  - [ ] reconcile treatment of nulls with #35496
- [ ] `edit`
  - [ ] decode into runtime.Unstructured objects
  - [ ] fall back to generic JSON patch computation if no go struct is registered for the target GVK
@ash2k ash2k mentioned this issue Jan 30, 2017
Closed
@bgrant0607 bgrant0607 added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Feb 7, 2017
@bgrant0607
Copy link
Member

@ymqytw Please dedupe this. There are several other issues about apply and edit for TPR.

Apply: #29542, #39906
Edit: #35993

@liggitt
Copy link
Member

liggitt commented Feb 8, 2017 •
edited

apply is fixed in #40666. #35993 covers edit

@liggitt liggitt closed this as completed Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mengqiy mengqiy

Labels
area/app-lifecycle area/kubectl priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@edsiper @liggitt @mengqiy @ahakanbaba @bgrant0607