Add listener based on the machine label and listener label #2155

Shilpa-Gokul · 2025-01-29T09:50:37Z

What this PR does / why we need it:
This PR provides support for assigning additional listeners to specific machines based on label selectors. For more details, please refer the proposal

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1678

Special notes for your reviewer:

/area provider/ibmcloud

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

Add support for assigning additional listeners to specific machines based on label selectors

k8s-ci-robot · 2025-01-29T09:50:47Z

Hi @Shilpa-Gokul. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

netlify · 2025-01-29T09:51:47Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	`8c431e4`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-ibmcloud/deploys/6830041cd7f49d00086aa118
😎 Deploy Preview	https://deploy-preview-2155.cluster-api-ibmcloud.sigs.k8s.io
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

api/v1beta2/ibmvpccluster_types.go

mkumatag · 2025-01-29T14:59:28Z

@Shilpa-Gokul its worth writing a design doc with more examples, lets do that before this PR.

I like the way it is written here - #2139 for one of the work.

Karthik-K-N · 2025-01-30T02:23:29Z

@Shilpa-Gokul its worth writing a design doc with more examples, lets do that before this PR.

I like the way it is written here - #2139 for one of the work.

Yes, We have already planned to do so, With some diagrams and examples, That would become easier for review and to see if there is any better approach as well, This is also one of the reasons for keeping this as WIP and also adding UT with only happy paths.

Amulyam24 · 2025-04-03T05:38:44Z

/ok-to-test

Karthik-K-N

Few small things, Otherwise LGTM

/assign @Amulyam24 @Prajyot-Parab

cloud/scope/powervs_machine.go

internal/webhooks/ibmpowervscluster.go

k8s-ci-robot · 2025-05-07T08:52:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Shilpa-Gokul
Once this PR has been reviewed and has the lgtm label, please ask for approval from prajyot-parab. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

Amulyam24 · 2025-05-07T10:34:59Z

/retest

Amulyam24

/lgtm

Thank you @Shilpa-Gokul for continued effort on this feature!

Hope you have validated the changes by cluster creation.

k8s-ci-robot · 2025-05-07T13:45:17Z

New changes are detected. LGTM label has been removed.

Shilpa-Gokul · 2025-05-07T15:45:47Z

/hold
@Amulyam24 With the latest changes, I'm yet to validate the cluster creation. Will test once again and confirm

Karthik-K-N

Please add the release notes as well.

Thank you for working on this feature, It was a tough one and took more discussion and time than anticipated.

Shilpa-Gokul · 2025-05-16T05:32:25Z

Thanks @Karthik-K-N and @Amulyam24 for helping me with ideas for this feature.
I have validated the cluster creation with the latest changes, please review it once again.

cloud/scope/powervs_machine.go

Karthik-K-N

Most of the things LGTM
/assign @Amulyam24 @Prajyot-Parab

api/v1beta2/ibmvpccluster_types.go

Amulyam24 · 2025-05-23T07:53:00Z

cloud/scope/powervs_machine.go

+			}
+			if additionalListener, ok := lbAdditionalListeners[fmt.Sprintf("%d-%s", *loadBalancerListener.Port, *loadBalancerListener.Protocol)]; ok {
+				if loadBalancerListener.DefaultPool != nil {
+					loadBalancerListeners[*loadBalancerListener.DefaultPool.Name] = additionalListener


@Shilpa-Gokul @Karthik-K-N , one question here, the AdditionalListener in Spec has DefaultPoolName field, is it not possible to populate the field somehow and use the value here?

I'm unable to recall if the pool name follows a unique pattern when it is created

I think currently that field is never considered, also we expects spec to be set by user and not to alter from controller

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/provider/ibmcloud Issues or PRs related to ibmcloud provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 29, 2025

k8s-ci-robot requested review from Amulyam24 and mkumatag January 29, 2025 09:50

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 Jan 29, 2025

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 29, 2025

mkumatag requested changes Jan 29, 2025

View reviewed changes

api/v1beta2/ibmvpccluster_types.go Outdated Show resolved Hide resolved

Shilpa-Gokul force-pushed the add-listener branch from aac912c to e4f009b Compare February 4, 2025 11:38

Shilpa-Gokul force-pushed the add-listener branch 2 times, most recently from dc01447 to ac508e0 Compare March 27, 2025 06:01

Shilpa-Gokul force-pushed the add-listener branch from ffa11f4 to a32fa4f Compare April 2, 2025 08:40

Shilpa-Gokul requested a review from Karthik-K-N April 2, 2025 08:41

Shilpa-Gokul changed the title ~~[WIP]: Add listener based on the machine label and listener label~~ Add listener based on the machine label and listener label Apr 3, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 3, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 3, 2025

Shilpa-Gokul force-pushed the add-listener branch 3 times, most recently from 5f06b18 to 7fbcc75 Compare April 3, 2025 14:31

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2025

Shilpa-Gokul force-pushed the add-listener branch from 7fbcc75 to 1b88ddd Compare April 10, 2025 08:14

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2025

Shilpa-Gokul force-pushed the add-listener branch 2 times, most recently from 48f0f88 to ee91bfe Compare April 11, 2025 12:59

Shilpa-Gokul requested a review from Karthik-K-N May 6, 2025 09:41

Shilpa-Gokul force-pushed the add-listener branch from ef67a45 to d36b296 Compare May 6, 2025 12:38

Karthik-K-N reviewed May 7, 2025

View reviewed changes

cloud/scope/powervs_machine.go Show resolved Hide resolved

internal/webhooks/ibmpowervscluster.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned Amulyam24 and Prajyot-Parab May 7, 2025

Shilpa-Gokul force-pushed the add-listener branch from d36b296 to 8981e0e Compare May 7, 2025 08:52

Amulyam24 reviewed May 7, 2025

View reviewed changes

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

Shilpa-Gokul force-pushed the add-listener branch from 8981e0e to 27d38e1 Compare May 7, 2025 13:45

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 7, 2025

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 7, 2025

Karthik-K-N reviewed May 8, 2025

View reviewed changes

Shilpa-Gokul force-pushed the add-listener branch 2 times, most recently from 106e5c3 to 6b1a4e4 Compare May 16, 2025 05:26

Shilpa-Gokul requested review from Amulyam24, Karthik-K-N and mkumatag May 16, 2025 05:32

Karthik-K-N reviewed May 16, 2025

View reviewed changes

cloud/scope/powervs_machine.go Show resolved Hide resolved

cloud/scope/powervs_machine.go Outdated Show resolved Hide resolved

cloud/scope/powervs_machine.go Outdated Show resolved Hide resolved

cloud/scope/powervs_machine.go Outdated Show resolved Hide resolved

Shilpa-Gokul force-pushed the add-listener branch 2 times, most recently from 4e5d252 to 1f759c2 Compare May 16, 2025 11:28

Shilpa-Gokul requested a review from Karthik-K-N May 22, 2025 05:22

Karthik-K-N reviewed May 22, 2025

View reviewed changes

api/v1beta2/ibmvpccluster_types.go Outdated Show resolved Hide resolved

Add listener based on the machine label and listener label

8c431e4

Shilpa-Gokul force-pushed the add-listener branch from 1f759c2 to 8c431e4 Compare May 23, 2025 05:14

Amulyam24 reviewed May 23, 2025

View reviewed changes

Add listener based on the machine label and listener label #2155

Are you sure you want to change the base?

Add listener based on the machine label and listener label #2155

Uh oh!

Conversation

Shilpa-Gokul commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jan 29, 2025

Uh oh!

netlify bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Uh oh!

Uh oh!

mkumatag commented Jan 29, 2025

Uh oh!

Karthik-K-N commented Jan 30, 2025

Uh oh!

Amulyam24 commented Apr 3, 2025

Uh oh!

Karthik-K-N left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

k8s-ci-robot commented May 7, 2025

Uh oh!

Amulyam24 commented May 7, 2025

Uh oh!

Amulyam24 left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 7, 2025

Uh oh!

Shilpa-Gokul commented May 7, 2025

Uh oh!

Karthik-K-N left a comment

Choose a reason for hiding this comment

Uh oh!

Shilpa-Gokul commented May 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Karthik-K-N left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Amulyam24 May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Shilpa-Gokul commented Jan 29, 2025 •

edited

Loading

netlify bot commented Jan 29, 2025 •

edited

Loading