fix for duplicate entries in front-loadbalancer #11308

sachintiptur · 2022-11-07T17:00:53Z

Signed-off-by: Sachin Tiptur sachin@kubermatic.com

What this PR does / why we need it:
Fixes the duplicate entry issue in front-loadbalancer service.

Which issue(s) this PR fixes:

Fixes # 11307

What type of PR is this?
/kind bug

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Fix duplicate SourceRange entries for front-loadbalancer Service

Documentation:

NONE

xrstf · 2022-11-07T18:50:19Z

pkg/resources/nodeportproxy/nodeportproxy.go

+				for _, cidr := range data.Cluster().Spec.APIServerAllowedIPRanges.CIDRBlocks {
+					if !slices.Contains(s.Spec.LoadBalancerSourceRanges, cidr) {
+						s.Spec.LoadBalancerSourceRanges = append(s.Spec.LoadBalancerSourceRanges, cidr)
+					}
+				}


This can be solved by using apimaachinery's sets.String type to just create a string set from the cluster spec and then assining .List() to the service's spec. And for the other one ([]CIDRBlock) it might also be nice to use the string sets, just so we don't have one Go dependency just for slice.Contains.

Thanks for the suggestion, taken care.

rastislavs · 2022-11-08T11:37:38Z

/lgtm

kubermatic-bot · 2022-11-08T11:37:42Z

LGTM label has been added.

Git tree hash: 1c7d1f57a5391de5dcc635a3281fb92bda34e2ad

sachintiptur · 2022-11-08T13:08:06Z

/retest

sachintiptur · 2022-11-08T15:05:45Z

/assign @xrstf

Signed-off-by: Sachin Tiptur <sachin@kubermatic.com>

xrstf

/approve

kubermatic-bot · 2022-11-09T09:26:49Z

LGTM label has been added.

Git tree hash: 53bd8bf9a0445c78cffbb430094f5a5fa8460998

kubermatic-bot · 2022-11-09T09:26:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sachintiptur, xrstf

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:

~~pkg/resources/OWNERS~~ [xrstf]

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

embik · 2022-11-14T10:07:55Z

/cherry-pick release/v2.21

embik · 2022-11-14T10:07:59Z

/cherry-pick release/v2.20

embik · 2022-11-14T10:08:04Z

/cherry-pick release/v2.19

kubermatic-bot · 2022-11-14T10:08:31Z

@embik: #11308 failed to apply on top of branch "release/v2.21":

Applying: fix for duplicate entries in front-loadbalancer
Using index info to reconstruct a base tree...
M	pkg/resources/nodeportproxy/nodeportproxy.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/resources/nodeportproxy/nodeportproxy.go
CONFLICT (content): Merge conflict in pkg/resources/nodeportproxy/nodeportproxy.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix for duplicate entries in front-loadbalancer
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release/v2.21

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.

kubermatic-bot · 2022-11-14T10:08:35Z

@embik: #11308 failed to apply on top of branch "release/v2.20":

Applying: fix for duplicate entries in front-loadbalancer
Using index info to reconstruct a base tree...
M	pkg/resources/nodeportproxy/nodeportproxy.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/resources/nodeportproxy/nodeportproxy.go
CONFLICT (content): Merge conflict in pkg/resources/nodeportproxy/nodeportproxy.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix for duplicate entries in front-loadbalancer
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release/v2.20

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.

kubermatic-bot · 2022-11-14T10:08:40Z

@embik: #11308 failed to apply on top of branch "release/v2.19":

Applying: fix for duplicate entries in front-loadbalancer
Using index info to reconstruct a base tree...
M	pkg/resources/nodeportproxy/nodeportproxy.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/resources/nodeportproxy/nodeportproxy.go
CONFLICT (content): Merge conflict in pkg/resources/nodeportproxy/nodeportproxy.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix for duplicate entries in front-loadbalancer
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release/v2.19

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.

embik · 2022-11-14T10:42:49Z

@sachintiptur it looks like this cannot be automatically backported. Since the original issue said it's present in 2.21, can you please take a look and backport this to all relevant and active release branches? Thanks!

embik · 2022-12-08T08:30:19Z

@sachintiptur ping, can you please confirm this doesn't need a backport to 2.19?

sachintiptur · 2022-12-08T09:01:17Z

@sachintiptur ping, can you please confirm this doesn't need a backport to 2.19?

Yes, it is not required in v2.19.

embik · 2022-12-08T11:31:14Z

Thanks!

xrstf reviewed Nov 7, 2022

View reviewed changes

sachintiptur force-pushed the fix_ipw branch from 64596c4 to 3c3d431 Compare November 8, 2022 10:35

sachintiptur changed the title ~~WIP: fix for duplicate entries in front-loadbalancer~~ fix for duplicate entries in front-loadbalancer Nov 8, 2022

kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 8, 2022

sachintiptur requested a review from rastislavs November 8, 2022 11:29

kubermatic-bot assigned rastislavs Nov 8, 2022

kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2022

sachintiptur requested a review from xrstf November 8, 2022 11:44

kubermatic-bot assigned xrstf Nov 8, 2022

fix for duplicate entries in front-loadbalancer

28d45cb

Signed-off-by: Sachin Tiptur <sachin@kubermatic.com>

sachintiptur force-pushed the fix_ipw branch from 3c3d431 to 28d45cb Compare November 9, 2022 04:50

kubermatic-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 9, 2022

xrstf approved these changes Nov 9, 2022

View reviewed changes

kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2022

kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 9, 2022

kubermatic-bot merged commit 8f024ba into kubermatic:main Nov 9, 2022

embik added the backport-needed Denotes a PR or issue that has not been fully backported. label Nov 14, 2022

This was referenced Nov 16, 2022

[release/v2.21] fix for duplicate entries in front-loadbalancer #11371

Merged

[release/v2.20] fix for duplicate entries in front-loadbalancer #11369

Merged

kubermatic-bot added 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 Nov 16, 2022

embik added backport-complete Denotes a PR or issue which has been fully backported to all required release branches. and removed backport-needed Denotes a PR or issue that has not been fully backported. labels Dec 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix for duplicate entries in front-loadbalancer #11308

fix for duplicate entries in front-loadbalancer #11308

sachintiptur commented Nov 7, 2022 •

edited

Loading

xrstf Nov 7, 2022

sachintiptur Nov 8, 2022

rastislavs commented Nov 8, 2022

kubermatic-bot commented Nov 8, 2022

sachintiptur commented Nov 8, 2022

sachintiptur commented Nov 8, 2022

xrstf left a comment

kubermatic-bot commented Nov 9, 2022

kubermatic-bot commented Nov 9, 2022

embik commented Nov 14, 2022

embik commented Nov 14, 2022

embik commented Nov 14, 2022

kubermatic-bot commented Nov 14, 2022

kubermatic-bot commented Nov 14, 2022

kubermatic-bot commented Nov 14, 2022

embik commented Nov 14, 2022

embik commented Dec 8, 2022

sachintiptur commented Dec 8, 2022

embik commented Dec 8, 2022

fix for duplicate entries in front-loadbalancer #11308

fix for duplicate entries in front-loadbalancer #11308

Conversation

sachintiptur commented Nov 7, 2022 • edited Loading

xrstf Nov 7, 2022

Choose a reason for hiding this comment

sachintiptur Nov 8, 2022

Choose a reason for hiding this comment

rastislavs commented Nov 8, 2022

kubermatic-bot commented Nov 8, 2022

sachintiptur commented Nov 8, 2022

sachintiptur commented Nov 8, 2022

xrstf left a comment

Choose a reason for hiding this comment

kubermatic-bot commented Nov 9, 2022

kubermatic-bot commented Nov 9, 2022

embik commented Nov 14, 2022

embik commented Nov 14, 2022

embik commented Nov 14, 2022

kubermatic-bot commented Nov 14, 2022

kubermatic-bot commented Nov 14, 2022

kubermatic-bot commented Nov 14, 2022

embik commented Nov 14, 2022

embik commented Dec 8, 2022

sachintiptur commented Dec 8, 2022

embik commented Dec 8, 2022

sachintiptur commented Nov 7, 2022 •

edited

Loading