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

Manual cherry pick of part of #44053 to release-1.5 #44365

Merged

Conversation

MrHohn
Copy link
Member

@MrHohn MrHohn commented Apr 11, 2017

This cherry pick has the same purpose as #44246 and #44315.

The first commit is a manual application of git commit 7664b97 to
release-1.5 branch. The unit test part is dropped as it has too
many dependencies that shouldn't be cherry picked.

The second commit is a surgical fix that does not exist on 1.6+, as
we overhauled healthcheck package completely. It fixes the health
check deletion logic in OnServiceUpdate for the same bug.

@thockin @freehan

Release note:

Fix for kube-proxy healthcheck handling an update that simultaneously removes one port and adds another.

This is a manual application of git commit 7664b97 to
release-1.5 branch. The unit test part is dropped as it has too
many dependencies that shouldn't be cherry picked.

7664b97:

Add tests for kube-proxy healthcheck, fix bug

Adding test cases for HC updates found a bug with an update that
simultaneously removes one port and adds another.  Map iteration is
randomized, so sometimes no HC would be created.
Service update that simultaneously removes one port and
adds another will trigger OnServiceUpdate to mistakenly
remove health check on nodes. This commit fixes the
health check deletion logic.

This is a surgical fix only in 1.5 for the health check
bug (kubernetes#44053). It does not exist on 1.6+ as we overhauled
healthcheck package completely.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 11, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 11, 2017
@MrHohn
Copy link
Member Author

MrHohn commented Apr 11, 2017

/assign @thockin @freehan

/unassign @smarterclayton @bprashanth

@thockin
Copy link
Member

thockin commented Apr 11, 2017

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 11, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrHohn, thockin

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 Apr 11, 2017
@thockin thockin added this to the v1.5 milestone Apr 11, 2017
@MrHohn
Copy link
Member Author

MrHohn commented Apr 12, 2017

@saad-ali Not sure who is handling 1.5 patch release?

@MrHohn
Copy link
Member Author

MrHohn commented Apr 13, 2017

@mwielgus We'd like to check the fix in for 1.5.7 :)

@MrHohn
Copy link
Member Author

MrHohn commented Apr 17, 2017

Ping @@mwielgus :)

@mwielgus mwielgus added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Apr 18, 2017
@MrHohn
Copy link
Member Author

MrHohn commented Apr 18, 2017

@mwielgus Thanks!

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit d29f97b into kubernetes:release-1.5 Apr 18, 2017
@k8s-cherrypick-bot
Copy link

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

@MrHohn MrHohn deleted the release-1.5-healthcheck branch May 16, 2017 23:57
This was referenced Sep 11, 2020
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. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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 Denotes a PR that will be considered when it comes time to generate release notes. 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

10 participants