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.

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

NDEV-21780 feat: added option to include cluster labels in kube-controller #528

Merged
merged 5 commits into from
Feb 28, 2025

Conversation

VedRatan
Copy link
Contributor

Added option to include cluster labels via helm chart in kube-controller.

Related Jira Ticket :- JIRA

@puneet-nirmata puneet-nirmata merged commit 4857fb5 into main Feb 28, 2025
1 check passed
@puneet-nirmata puneet-nirmata deleted the NDEV-21780 branch February 28, 2025 17:48
@patelrit
Copy link
Contributor

patelrit commented Mar 1, 2025

@puneet-nirmata @VedRatan
I see multiple issues with this commit

  1. According to the kubecontroller code the labels should be of the format. --cluster-labels=foo1:bar1,foo2:bar2 but the change in this checkin will create something like
  • --cluster-labels
  • foo1:bar1
  • --cluster-labels
  • foo2:bar2
    Which means only the last label will be taken
  1. Why do we need separate registeries for the kube-controller image and otel image? Both these images will always be pulled from the same registry e.g. ghcr.io/nirmata or customer-reg/nirmata
    So we can keep things simple and use the same registry

Please fix and test this thoroghly before merging:
@dtoledo67 @JimBugwadia @anusha94

@VedRatan
Copy link
Contributor Author

VedRatan commented Mar 1, 2025

Hi @patelrit,

Thank you for your feedback. I’d like to clarify why the issues mentioned in points 1 and 2 will not occur, based on our implementation and testing:

Cluster Labels Flexibility:
The cluster-labels field in kube-controller is designed as a map[string]string, which allows for key-value pairs to be passed in multiple ways. We have thoroughly tested both formats:

Comma-separated key-value pairs (e.g., key1=value1,key2=value2).

Individual --cluster-labels flags (e.g., --cluster-labels key1=value1 --cluster-labels key2=value2).
This ensures compatibility and flexibility for users, and our testing confirms that both approaches work seamlessly.

Image Registry Granularity:
To streamline image management, we’ve defined global.imageRegistry and global.imageRepository in the values.yaml file. By default, all components in the kube-controller chart use these global fields. However, to provide additional flexibility, we’ve also enabled the ability to specify separate registries or repositories for individual components.
For example:

A customer might want to pull the kube-controller image from ghcr.io/nirmata.

Simultaneously, they might want to use a custom or alternative registry for the otel-agent image (e.g., docker.io/ or nirmata).
This granularity has been implemented and tested across all possible scenarios, ensuring it meets diverse use cases without issues.

We’ve rigorously tested these features to ensure they work as intended, and they are designed to provide both simplicity and flexibility for our users. Please let us know if you have any further questions or need additional clarification!

cc: @puneet-nirmata @suhasgummanirmata @dtoledo67 @JimBugwadia

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