[occm] add a node selector support for loadbalancer services #2601

kayrus · 2024-05-24T16:02:06Z

What this PR does / why we need it:

This PR adds an ability to specify loadbalancer service with a node selector.

Big thanks to @ririko-nakamura for the initial code.

A new OCCM node-selector option is added into the [LoadBalancer] config section.
And a new loadbalancer.openstack.org/node-selector service annotation support is added.

Which issue this PR fixes(if applicable):
fixes #1770

Special notes for reviewers:

Release note:

added a node selector support for loadbalancer services

linux-foundation-easycla · 2024-05-24T16:02:13Z

✅login: ririko-nakamura / (5b31796)
✅login: ririko-nakamura / (5b31796, c8442aa)
✅login: ririko-nakamura / (5b31796, c8442aa, ec863e1)
✅login: kayrus / (604fdc0)
✅login: kayrus / (604fdc0, 13a09b9)

The committers listed above are authorized under a signed CLA.

zetaab

otherwise looks good, but perhaps worth of adding test for util.go stuff

also @ririko-nakamura is missing cla

pkg/util/util.go

zetaab

/approve

k8s-ci-robot · 2024-05-25T06:36:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zetaab

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:

~~OWNERS~~ [zetaab]

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

jichenjc · 2024-05-27T02:17:45Z

pkg/openstack/loadbalancer.go

+	}
+
+	for k, v := range filterLabels {
+		if nodeLabelValue, ok := node.Labels[k]; !ok || (v != "" && nodeLabelValue != v) {


not sure add a info.v(6) for the not match will be helpful for debug?

jichenjc · 2024-05-27T02:19:58Z

/lgtm

kayrus · 2024-05-27T07:34:47Z

@jichenjc @zetaab @dulek this PR is clearly a feature, and this improvement affects only selected environments. Keeping in mind that the default behavior wasn't changed, is there a chance we can cherry-pick the fix to earlier OCCM versions?

jichenjc · 2024-05-27T07:40:34Z

generally only with bugs to backport..
but I think this unblock some use case so should be good backport if everyone agree

dulek · 2024-05-27T11:29:05Z

Yeah, I'm not opposed to backporting this, seems like it was highly anticipated. @stephenfin, @mdbooth, how do you look at this?

kayrus · 2024-05-27T17:32:03Z

/cherry-pick release-1.30

kayrus · 2024-05-27T17:32:11Z

/cherry-pick release-1.29

kayrus · 2024-05-27T17:32:17Z

/cherry-pick release-1.28

k8s-infra-cherrypick-robot · 2024-05-27T17:32:45Z

@kayrus: new pull request created: #2603

In response to this:

/cherry-pick release-1.30

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-sigs/prow repository.

k8s-infra-cherrypick-robot · 2024-05-27T17:32:46Z

@kayrus: new pull request created: #2604

In response to this:

/cherry-pick release-1.29

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-sigs/prow repository.

k8s-infra-cherrypick-robot · 2024-05-27T17:32:51Z

@kayrus: #2601 failed to apply on top of branch "release-1.28":

Applying: POC of TargetNodeLabels selector on OpenStack LB
Applying: Fix type errors
Applying: Update implementation of getKeyValuePropertiesFromServiceAnnotation
Applying: gofmt -w -s ./pkg
Applying: Polish the code and add documentation
Using index info to reconstruct a base tree...
M	docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md
M	docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md
M	pkg/openstack/loadbalancer.go
M	pkg/openstack/loadbalancer_test.go
M	pkg/openstack/openstack.go
M	pkg/util/util.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/util/util.go
Auto-merging pkg/openstack/openstack.go
Auto-merging pkg/openstack/loadbalancer_test.go
CONFLICT (content): Merge conflict in pkg/openstack/loadbalancer_test.go
Auto-merging pkg/openstack/loadbalancer.go
Auto-merging docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md
CONFLICT (content): Merge conflict in docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md
Auto-merging docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0005 Polish the code and add documentation
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-1.28

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-sigs/prow repository.

…tes#2601) * POC of TargetNodeLabels selector on OpenStack LB * Fix type errors * Update implementation of getKeyValuePropertiesFromServiceAnnotation * gofmt -w -s ./pkg * Polish the code and add documentation --------- Co-authored-by: Ririko Nakamura <ririko.nakamura@outlook.com>

…2605) * POC of TargetNodeLabels selector on OpenStack LB * Fix type errors * Update implementation of getKeyValuePropertiesFromServiceAnnotation * gofmt -w -s ./pkg * Polish the code and add documentation --------- Co-authored-by: Ririko Nakamura <ririko.nakamura@outlook.com>

ririko-nakamura and others added 4 commits May 21, 2024 13:55

POC of TargetNodeLabels selector on OpenStack LB

5b31796

Fix type errors

c8442aa

Update implementation of getKeyValuePropertiesFromServiceAnnotation

ec863e1

gofmt -w -s ./pkg

604fdc0

kayrus requested review from dulek and jichenjc May 24, 2024 16:02

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 24, 2024

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 24, 2024

k8s-ci-robot requested review from FengyunPan2 and mdbooth May 24, 2024 16:02

kayrus force-pushed the target-node-labels-selector branch 4 times, most recently from 87ca5fe to 5f58546 Compare May 24, 2024 17:04

zetaab requested changes May 24, 2024

View reviewed changes

pkg/util/util.go Show resolved Hide resolved

kayrus force-pushed the target-node-labels-selector branch 2 times, most recently from fc1657b to dc5dfd4 Compare May 24, 2024 20:44

Polish the code and add documentation

13a09b9

kayrus force-pushed the target-node-labels-selector branch from dc5dfd4 to 13a09b9 Compare May 24, 2024 20:47

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 24, 2024

zetaab approved these changes May 25, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 25, 2024

jichenjc approved these changes May 27, 2024

View reviewed changes

k8s-ci-robot assigned jichenjc May 27, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 27, 2024

k8s-ci-robot merged commit 59efe1c into kubernetes:master May 27, 2024
11 checks passed

kayrus deleted the target-node-labels-selector branch May 27, 2024 07:29

k8s-infra-cherrypick-robot mentioned this pull request May 27, 2024

[release-1.30] [occm] add a node selector support for loadbalancer services #2603

Merged

k8s-infra-cherrypick-robot mentioned this pull request May 27, 2024

[release-1.29] [occm] add a node selector support for loadbalancer services #2604

Merged

kayrus mentioned this pull request May 27, 2024

[release-1.28] [occm] add a node selector support for loadbalancer services #2605

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[occm] add a node selector support for loadbalancer services #2601

[occm] add a node selector support for loadbalancer services #2601

kayrus commented May 24, 2024

linux-foundation-easycla bot commented May 24, 2024 •

edited

Loading

zetaab left a comment •

edited

Loading

zetaab left a comment

k8s-ci-robot commented May 25, 2024

jichenjc May 27, 2024

jichenjc commented May 27, 2024

kayrus commented May 27, 2024

jichenjc commented May 27, 2024

dulek commented May 27, 2024

kayrus commented May 27, 2024

kayrus commented May 27, 2024

kayrus commented May 27, 2024

k8s-infra-cherrypick-robot commented May 27, 2024

k8s-infra-cherrypick-robot commented May 27, 2024

k8s-infra-cherrypick-robot commented May 27, 2024

[occm] add a node selector support for loadbalancer services #2601

[occm] add a node selector support for loadbalancer services #2601

Conversation

kayrus commented May 24, 2024

linux-foundation-easycla bot commented May 24, 2024 • edited Loading

zetaab left a comment • edited Loading

Choose a reason for hiding this comment

zetaab left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 25, 2024

jichenjc May 27, 2024

Choose a reason for hiding this comment

jichenjc commented May 27, 2024

kayrus commented May 27, 2024

jichenjc commented May 27, 2024

dulek commented May 27, 2024

kayrus commented May 27, 2024

kayrus commented May 27, 2024

kayrus commented May 27, 2024

k8s-infra-cherrypick-robot commented May 27, 2024

k8s-infra-cherrypick-robot commented May 27, 2024

k8s-infra-cherrypick-robot commented May 27, 2024

linux-foundation-easycla bot commented May 24, 2024 •

edited

Loading

zetaab left a comment •

edited

Loading