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.

Sign up for GitHub

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

[backport] Disable session affinity for internal kubernetes service #65177

Merged
merged 1 commit into from
Jun 22, 2018
Merged

[backport] Disable session affinity for internal kubernetes service #65177

merged 1 commit into from
Jun 22, 2018

Conversation

afritzler
Copy link

@afritzler afritzler commented Jun 18, 2018 •
edited
Loading

Under following conditions session affinity leads to a deadlock:

  • Self hosted controller-manager, where it talks to API servers
    via kubernetes service ClusterIP
  • default master-count reconcilier is used
  • --apiserver-count is set to >1 according to the help message
  • number of responsive APIServers goes below apiserver-count
  • all controller-managers happen to be hashed to APIServers which
    are down.

What then happens is that controller managers never be able to
contact APIServer, despite correctly working APIServer available.

Less serious outages also possible for other consumers of kubernetes
service, such as operators, kube-dns, flannel & calico, etc. There is
always non zero chance, that given consumer is hashed to an apiserver
which is down.

Revert "give the kubernetes service client ip session affinity"
This reverts commit e21ebbc.

What this PR does / why we need it:
Backporting #56690 to 1.10 release branch.

Which issue(s) this PR fixes
Fixes #23129

Release note:

Disable session affinity for internal kubernetes service - Backport of #56690 to 1.10 release branch

@redbaron @afritzler
Disable session affinity for internal kuberntes service
6e7f53e 
Under following conditions session affinity leads to a deadlock:
  - Self hosted controller-manager, where it talks to API servers
    via kubernetes service ClusterIP
  - default master-count reconcilier is used
  - --apiserver-count is set to >1 according to the help message
  - number of responsive APIServers goes below `apiserver-count`
  - all controller-managers happen to be hashed to APIServers which
    are down.

What then happens is that controller managers never be able to
contact APIServer, despite correctly working APIServer available.

Less serious outages also possible for other consumers of kubernetes
service, such as operators, kube-dns, flannel & calico, etc.  There is
always non zero chance, that given consumer is hashed  to an apiserver
which is down.

Revert "give the kubernetes service client ip session affinity"
This reverts commit e21ebbc.
@k8s-ci-robot k8s-ci-robot added 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 18, 2018
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 18, 2018
@k8s-github-robot k8s-github-robot added the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label Jun 18, 2018
@afritzler
Copy link
Author

/assign @derekwaynecarr

@afritzler afritzler changed the title [backport] Disable session affinity for internal kuberntes service [backport] Disable session affinity for internal kubernetes service Jun 18, 2018
@afritzler afritzler mentioned this pull request Jun 18, 2018
Closed
@MaciekPytel
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 18, 2018
@thockin
Copy link
Member

thockin commented Jun 18, 2018

/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 Jun 18, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 18, 2018
@MaciekPytel MaciekPytel added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. status/approved-for-milestone and removed do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. labels Jun 19, 2018
@MaciekPytel MaciekPytel added this to the v1.10 milestone Jun 19, 2018
@MaciekPytel MaciekPytel added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jun 19, 2018
@MaciekPytel
Copy link
Contributor

/retest

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jun 19, 2018 •
edited
Loading

@afritzler: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-kubemark-e2e-gce 6e7f53e link /test pull-kubernetes-kubemark-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@MaciekPytel
Copy link
Contributor

/hold
Freezing branch for 1.10.5

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 19, 2018
@MaciekPytel
Copy link
Contributor

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 22, 2018
@MaciekPytel
Copy link
Contributor

/retest

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request Labels Incomplete

@afritzler @derekwaynecarr @thockin

Action required: This pull request requires label changes.

kind: Must specify exactly one of kind/bug, kind/cleanup or kind/feature.
sig owner: Must specify at least one label prefixed with sig/.

Help

@k8s-github-robot
Copy link

Automatic merge from submit-queue.

@k8s-github-robot k8s-github-robot merged commit d416d1c into kubernetes:release-1.10 Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@thockin thockin

@derekwaynecarr derekwaynecarr

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. milestone/incomplete-labels priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. 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
Milestone
v1.10
Development

Successfully merging this pull request may close these issues.
7 participants
@afritzler @MaciekPytel @thockin @k8s-ci-robot @k8s-github-robot @derekwaynecarr @redbaron