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

Calculate patches for commands using input version #53158

Merged

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Sep 27, 2017

Fixes #53040

the encoder used for encoding these objects while calculating patches does not have sufficient information to select a correct version when the object does not exist in all versions of a target group (like replicasets not existing in apps/v1beta1)

this PR wraps the encoder to first convert to the same version used to read the object (based on the mapping's GroupVersion)

long-term, we should switch UpdatePodSpecForObject to work on versioned objects and v1.PodSpec and avoid conversion altogether

Fixes an issue with `kubectl set` commands encountering conversion errors for ReplicaSet and DaemonSet objects

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 27, 2017
@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 27, 2017
@liggitt
Copy link
Member Author

liggitt commented Sep 27, 2017

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

@k8s-ci-robot k8s-ci-robot added the sig/cli Categorizes an issue or PR as relevant to SIG CLI. label Sep 27, 2017
@liggitt
Copy link
Member Author

liggitt commented Sep 28, 2017

/retest

Copy link
Member

@janetkuo janetkuo left a comment

Choose a reason for hiding this comment

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

Thanks @liggitt!

This fixes a 1.8 known issue, so please add a release note. Would you also add test and TODO (or open an issue for the long-term fix)?

@liggitt liggitt added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 28, 2017
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 29, 2017
@liggitt
Copy link
Member Author

liggitt commented Sep 29, 2017

Would you also add test and TODO

done

@k8s-github-robot k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 29, 2017
@liggitt
Copy link
Member Author

liggitt commented Sep 29, 2017

kernel panic flake in pull-kubernetes-e2e-gce-bazel #50695
/retest

@liggitt liggitt assigned mengqiy and pwittrock and unassigned adohe-zz and rootfs Sep 29, 2017
@pwittrock
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 29, 2017
@pwittrock
Copy link
Member

/approve

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, pwittrock

Associated issue: 53040

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

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

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 29, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 53101, 53158, 52165). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 4425841 into kubernetes:master Sep 29, 2017
k8s-github-robot pushed a commit that referenced this pull request Sep 30, 2017
…8-upstream-release-1.8

Automatic merge from submit-queue.

Automated cherry pick of #53158

Fixes #53040
Cherry pick of #53158 on release-1.8.

#53158: Calculate patches for  commands using input version

```release-note
Fixes an issue with `kubectl set` commands encountering conversion errors for ReplicaSet and DaemonSet objects
```
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.8" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

@liggitt liggitt deleted the update-pod-spec-versioned branch October 1, 2017 01:54
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. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants