Skip to content

feat(kgo): add node selector to deployment - #1264

Open
vitoorgomes wants to merge 1 commit into
Kong:mainfrom
vitoorgomes:kgo-controller-node-selector
Open

feat(kgo): add node selector to deployment#1264
vitoorgomes wants to merge 1 commit into
Kong:mainfrom
vitoorgomes:kgo-controller-node-selector

Conversation

@vitoorgomes

Copy link
Copy Markdown

What this PR does / why we need it:

The idea is to add an option to specify the node on which the KGO controller should be scheduled. This feature can be helpful when clusters have dedicated nodes for specific use cases

Special notes for your reviewer:

I'm not sure if I forgot to run something locally, in any case, just let me know

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • PR is based off the current tip of the main branch.
  • Changes are documented under the "Unreleased" header in CHANGELOG.md
  • New or modified sections of values.yaml are documented in the README.md
  • Commits follow the Kong commit message guidelines

@vitoorgomes
vitoorgomes requested a review from a team as a code owner March 6, 2025 01:25
@CLAassistant

CLAassistant commented Mar 6, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@pmalek pmalek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Apart from the minor nit please rebase on latest main tip and resolve conflicts.

@@ -0,0 +1,16 @@
nodeSelector:
karpenter.sh/nodepool: my-nodepool

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work on our CI.

We run these values.yaml files against an actual kind cluster and this label is not set on its nodes. You can try using something generic that we should always see on nodes like kubernetes.io/os: linux.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the heads up, just committed the requested changes

@vitoorgomes
vitoorgomes force-pushed the kgo-controller-node-selector branch from 4fad38f to e2d9674 Compare March 26, 2025 01:11
@pmalek

pmalek commented Mar 26, 2025

Copy link
Copy Markdown
Member

Please run make test.golden.update to include the change in chart version: https://github.com/Kong/charts/actions/runs/14073289665/job/39429900999?pr=1264

@pmalek

pmalek commented Apr 14, 2025

Copy link
Copy Markdown
Member

@vitoorgomes Just checking in, do you still want to contribute this change?

@vitoorgomes

vitoorgomes commented Apr 16, 2025

Copy link
Copy Markdown
Author

@vitoorgomes Just checking in, do you still want to contribute this change?

hey @pmalek, sorry for the late response, the last couple of weeks have been rough

I will rebase and run the make cmd and push it soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants