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

Fix winkernel proxier setting the wrong HNS loadbalancer ID for ingresss IP #112837

Merged
merged 1 commit into from Oct 4, 2022
Merged

Fix winkernel proxier setting the wrong HNS loadbalancer ID for ingresss IP #112837

merged 1 commit into from Oct 4, 2022
+0 −2

Conversation

daschott
Copy link
Contributor

@daschott daschott commented Oct 3, 2022 •
edited by liggitt

What type of PR is this?

/kind bug
/kind regression

What this PR does / why we need it:

We need this as proxier winkernel load balancing has regressed on >= v1.24.0 such that users will see intermittent connectivity failures.

See issue #112836 for more details and workarounds.

Which issue(s) this PR fixes:

Fixes #112836

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixes a 1.24 regression in winkernel proxier that causes proxy rules to leak anytime service backends are modified.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 3, 2022
@k8s-ci-robot k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 3, 2022
@daschott
Copy link
Contributor Author

daschott commented Oct 3, 2022

cc @sbangari @jsturtevant @marosset

@jsturtevant
Copy link
Contributor

/priority important-soon
/triage accepted

Is there a unit test that could be added to validate this?

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 3, 2022
This was referenced Oct 3, 2022
Merged
Merged
@sbangari
Copy link
Contributor

sbangari commented Oct 4, 2022

/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 Oct 4, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: daschott, sbangari

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 Oct 4, 2022
@marosset
Copy link
Contributor

marosset commented Oct 4, 2022

/milestone v1.26
/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Oct 4, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.26 milestone Oct 4, 2022
@marosset
Copy link
Contributor

marosset commented Oct 4, 2022

/hold
This needs a release note

@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 Oct 4, 2022
@marosset
Copy link
Contributor

marosset commented Oct 4, 2022

/sig windows

@k8s-ci-robot k8s-ci-robot added sig/windows Categorizes an issue or PR as relevant to SIG Windows. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 4, 2022
@daschott
Copy link
Contributor Author

daschott commented Oct 4, 2022

/hold This needs a release note

@marosset added this now.

@marosset
Copy link
Contributor

marosset commented Oct 4, 2022

/hold This needs a release note

@marosset added this now.

Thanks @daschott

@marosset
Copy link
Contributor

marosset commented Oct 4, 2022

/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 Oct 4, 2022
@k8s-triage-robot
Copy link

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

  • The PR does have any do-not-merge/* labels
  • The PR does not have the needs-ok-to-test label
  • The PR is mergeable (does not have a needs-rebase label)
  • The PR is approved (has cncf-cla: yes, lgtm, approved labels)
  • The PR is failing tests required for merge

You can:

/retest

@k8s-ci-robot k8s-ci-robot merged commit 3ef7784 into kubernetes:master Oct 4, 2022
k8s-ci-robot added a commit that referenced this pull request Oct 6, 2022
@k8s-ci-robot
Merge pull request #112839 from daschott/automated-cherry-pick-of-#11…
b251791 
…2837-upstream-release-1.24

Automated cherry pick of #112837: Fix winkernel proxier setting the wrong HNS loadbalancer ID
k8s-ci-robot added a commit that referenced this pull request Oct 6, 2022
@k8s-ci-robot
Merge pull request #112840 from daschott/automated-cherry-pick-of-#11…
af1935b 
…2837-upstream-release-1.25

Automated cherry pick of #112837: Fix winkernel proxier setting the wrong HNS loadbalancer ID
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feiskyer feiskyer Awaiting requested review from feiskyer

@sbangari sbangari Awaiting requested review from sbangari

Assignees

@sbangari sbangari

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. kind/regression Categorizes issue or PR as related to a regression from a prior release. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.26
Development

Successfully merging this pull request may close these issues.

Regression in winkernel proxier that causes stale load balancing proxy rules
6 participants
@daschott @jsturtevant @sbangari @k8s-ci-robot @marosset @k8s-triage-robot