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

make kubectl generic commands work with unstructured objects #40260

Merged
merged 13 commits into from Jan 27, 2017

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jan 21, 2017

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

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

follow-ups for pre-existing issues:

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 21, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@liggitt liggitt changed the title WIP - make apply work with unstructured objects WIP - make kubectl apply work with unstructured objects Jan 21, 2017
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jan 21, 2017
@liggitt
Copy link
Member Author

liggitt commented Jan 21, 2017

cc @smarterclayton @deads2k @ymqytw @pwittrock

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2017
@liggitt liggitt force-pushed the kubectl-tpr branch 3 times, most recently from 86e6a5b to 15485ad Compare January 22, 2017 19:00
@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 22, 2017
@pwittrock
Copy link
Member

@liggitt What is the behavior w.r.t. saving nulls in the last-applied-config annotation. Are they saved? What happens when they are added / removed from the local configuration file and applied if they are / are not in the last-applied-config annotation?

@liggitt liggitt changed the title WIP - make kubectl apply work with unstructured objects make kubectl apply work with unstructured objects Jan 23, 2017
@liggitt liggitt added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jan 23, 2017
@liggitt liggitt changed the title make kubectl apply work with unstructured objects make kubectl generic commands work with unstructured objects Jan 23, 2017
@liggitt
Copy link
Member Author

liggitt commented Jan 23, 2017

@pwittrock @ymqytw I removed the generic json patch computation from this PR, so this is now limited to giving us unstructured objects to work with (instead of ThirdPartyResourceData objects).

With this PR, apply still encounters the issue #40096 is attempting to fix, but gives us a better foundation for computing a generic patch for those objects.

@mengqiy
Copy link
Member

mengqiy commented Jan 27, 2017

Sorry for the noise. I just want this to be merged.

@liggitt
Copy link
Member Author

liggitt commented Jan 27, 2017

rebased

@liggitt liggitt added approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jan 27, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

Needs approval from an approver in each of these OWNERS Files:

We suggest the following people:
cc @lavalamp @deads2k
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@liggitt
Copy link
Member Author

liggitt commented Jan 27, 2017

Flake #36821
@k8s-bot gce etcd3 e2e test this
@k8s-bot gci gce e2e test this

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 39223, 40260, 40082, 40389)

@k8s-github-robot k8s-github-robot merged commit 1625150 into kubernetes:master Jan 27, 2017
@liggitt liggitt deleted the kubectl-tpr branch January 27, 2017 14:07
k8s-github-robot pushed a commit that referenced this pull request Feb 1, 2017
Automatic merge from submit-queue (batch tested with PRs 40529, 40630)

propagate explicit nulls in apply

Rebase of #35496 on top of #40260

The client-side propagation of the raw value is no longer needed, since the client is preserving the original object in unstructured form (explicit nulls are preserved).

Kept tests and CreateThreeWayMergePatch changes from #35496

```release-note
kubectl apply now supports explicitly clearing values not present in the config by setting them to null
```

- [x] Clean up orphaned objects in test-cmd to preserve pre- and post- conditions
- [x] improve CreateThreeWayMergePatch test to not filter based on string comparison to test name
k8s-github-robot pushed a commit that referenced this pull request Feb 3, 2017
Automatic merge from submit-queue (batch tested with PRs 40864, 40666, 38382, 40874)

apply falls back to generic JSON patch computation if no go struct is registered for the target GVK

This PR is the master version of #40096 which is target 1.4 branch.
This PR is based on #40260 

- [x] ensure subkey deletion works in CreateThreeWayJSONMergePatch
- [x] ensure type stomping works in CreateThreeWayJSONMergePatch
- [x] lots of tests for generic json patch computation
- [x] apply falls back to generic 3-way JSON merge patch if no go struct is registered for the target GVK
  - [x] prevent generic apply patch computation between different apiVersions and/or kinds
  - [x] make pruner generic (apply --prune works with TPR)

```release-note
apply falls back to generic 3-way JSON merge patch if no go struct is registered for the target GVK
```
@yvespp
Copy link
Contributor

yvespp commented Feb 7, 2017

@liggitt Thanks for the fixes! Does this also fix kubectl convert? I'm getting this error with 1.5.2:

kubectl convert -f test_pdb.yaml
running in local mode...
error: unable to decode "test_pdb.yaml": no kind "Testpdb" is registered for version "test.org/v1"

The issue has no milestone, will this be part of 1.6?

@liggitt
Copy link
Member Author

liggitt commented Feb 8, 2017

Does this also fix kubectl convert

No. Third party resources do not support conversion.

The issue has no milestone, will this be part of 1.6?

These commits will be in 1.6, yes

akhilerm pushed a commit to akhilerm/apimachinery that referenced this pull request Sep 20, 2022
Automatic merge from submit-queue (batch tested with PRs 40529, 40630)

propagate explicit nulls in apply

Rebase of kubernetes/kubernetes#35496 on top of kubernetes/kubernetes#40260

The client-side propagation of the raw value is no longer needed, since the client is preserving the original object in unstructured form (explicit nulls are preserved).

Kept tests and CreateThreeWayMergePatch changes from kubernetes/kubernetes#35496

```release-note
kubectl apply now supports explicitly clearing values not present in the config by setting them to null
```

- [x] Clean up orphaned objects in test-cmd to preserve pre- and post- conditions
- [x] improve CreateThreeWayMergePatch test to not filter based on string comparison to test name

Kubernetes-commit: 9807cd7d06712dec1fc48d12644613768699957a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet