-
Notifications
You must be signed in to change notification settings - Fork 750
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
improvement: add podmonitor for vpc metric collection #3061
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: adam_buran <[email protected]>
Signed-off-by: adam_buran <[email protected]>
Signed-off-by: adam_buran <[email protected]>
Signed-off-by: adam_buran <[email protected]>
This pull request is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days |
Can someone review this PR? It's amazing fairly simple change here |
@@ -128,6 +128,9 @@ spec: | |||
- name: aws-eks-nodeagent | |||
image: {{ include "aws-vpc-cni.nodeAgentImage" . }} | |||
imagePullPolicy: {{ .Values.nodeAgent.image.pullPolicy }} | |||
ports: | |||
- containerPort: {{ .Values.nodeAgent.metricsBindAddr}} | |||
name: agentmetrics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could call the port as node-agent-metrics
to be explicit. I assume -
is allowed in the term.
A minor comment on the term for the port for PodMonitor. Rest looks good to me. |
What type of PR is this?
Which issue does this PR fix?:
There exists no way to collect metrics from the nodeagent at this time as there is no port opened up.
What does this PR do / Why do we need it?:
This adds a
PodMonitor
template in order to faciliatate automated prometheus metrics collection of the VPC CNI and EKS node agent. Additionally this PR adds a port to the eks node agent in order to allow for prometheus metrics to be collected.Testing done on this change:
Run helm template locally to generate the rendered templates
Will this PR introduce any new dependencies?:
no
Will this break upgrades or downgrades? Has updating a running cluster been tested?:
no
Does this change require updates to the CNI daemonset config files to work?:
Does this PR introduce any user-facing change?:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.