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

[collector] add node ip to kubeletstats #1206

Closed
wants to merge 7 commits into from

Conversation

alextricity25
Copy link

In some scenarios, it's desirable to use the node IP instead of the node name to gather kubelet metrics. This commit makes it possible for the user to specify the presets.kubeletMetrics.useNodeIp flag so that the node ip is used as an endpoint instead of the node name.

Somewhat resovles open-telemetry/opentelemetry-collector-contrib#22843

Copy link

linux-foundation-easycla bot commented May 30, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@TylerHelmuth
Copy link
Member

Instead of making this an option I'd be ok with switching to using Node IP in the preset instead of Node Name.

I'm actually surprised we're using node name instead of node ip as I'd expect IP to be more universally usable. We've seen issues like the one linked where IP is the solution, we could just use it all the time. I don't think it would break users.

@dmitryax @pavolloffay @jinja2 what do you think?

@jinja2
Copy link
Contributor

jinja2 commented May 30, 2024

Yeah should be okay to switch to node ip. It might be a breaking change for setups in which kubelet is not serving a certificate with the IP listed in subject alt name. Users will need to add the insecure_skip_verify: true config for such situations.

@TylerHelmuth
Copy link
Member

TylerHelmuth commented May 31, 2024

It might be a breaking change for setups in which kubelet is not serving a certificate with the IP listed in subject alt name

@jinja2 how common would this be?

@alextricity25
Copy link
Author

@TylerHelmuth Done! Latest commit removes useNodeIp and now exclusively uses K8S_NODE_IP for the kubeMetrics endpoint.

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

Please bump the minor version of the chart and run make generate-examples CHARTS=opentelemetry-collector

@TylerHelmuth
Copy link
Member

Error: INSTALLATION FAILED: DaemonSet.apps "opentelemetry-collector-z9u7pzlfno-agent" is invalid: spec.template.spec.containers[0].env[2].valueFrom.fieldRef.fieldPath: Invalid value: "spec.hostIp": error converting fieldPath: field label not supported: spec.hostIp

@alextricity25 looks like something with your use of the downward api is incorrect

@ChrsMark
Copy link
Member

ChrsMark commented Jul 1, 2024

@alextricity25 you will need to rebase as well :).

alextricity25 and others added 6 commits July 1, 2024 09:26
In some scenarios, it's desirable to use the node IP instead of the node name to gather kubelet metrics.
This commit makes it possible for the user to specify the `presets.kubeletMetrics.useNodeIp` flag
so that the node ip is used as an endpoint instead of the node name.
Co-authored-by: Chris Mark <chrismarkou92@gmail.com>
Copy link

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 16, 2024
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants