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.

Sign up for GitHub

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

Improve "cni-metrics-helper" setup experience #2874

Merged
merged 2 commits into from
Apr 19, 2024
Merged

Improve "cni-metrics-helper" setup experience #2874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d33c4f9
Improve "cni-metrics-helper" setup experience
guessi Apr 11, 2024
40aa4ab
Merge branch 'master' into improve-setup-flow
orsenthil Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 42 additions & 22 deletions charts/cni-metrics-helper/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,22 @@ This chart provides a Kubernetes deployment for the Amazon VPC CNI Metrics Helpe
First add the EKS repository to Helm:

```shell
helm repo add eks https://aws.github.io/eks-charts
$ helm repo add eks https://aws.github.io/eks-charts
```

To install the chart with the release name `cni-metrics-helper` and default configuration:
Ensure helm repository up to date

```shell
$ helm repo update eks
```
Comment on lines +20 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing step to ensure helm repository is up to date.


To identify the version you are going to apply

```shell
$ helm search repo eks/cni-metrics-helper --versions
```
Comment on lines +24 to +28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

User might not what to use latest version.


To install the latest chart with the release name `cni-metrics-helper` and default configuration:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Address the sample command is for latest version (depends on the last update time of helm repo update).


```shell
$ helm install cni-metrics-helper --namespace kube-system eks/cni-metrics-helper
Expand Down Expand Up @@ -43,26 +55,34 @@ $ helm uninstall cni-metrics-helper --namespace kube-system

The following table lists the configurable parameters for this chart and their default values.

| Parameter | Description | Default |
|------------------------------|---------------------------------------------------------------|--------------------|
| fullnameOverride | Override the fullname of the chart | cni-metrics-helper |
| image.region | ECR repository region to use. Should match your cluster | us-west-2 |
| image.tag | Image tag | v1.18.0 |
| image.account | ECR repository account number | 602401143452 |
| image.domain | ECR repository domain | amazonaws.com |
| env.USE_CLOUDWATCH | Whether to export CNI metrics to CloudWatch | true |
| env.USE_PROMETHEUS | Whether to export CNI metrics to Prometheus | false |
| env.AWS_CLUSTER_ID | ID of the cluster to use when exporting metrics to CloudWatch | default |
| env.AWS_VPC_K8S_CNI_LOGLEVEL | Log verbosity level (ie. FATAL, ERROR, WARN, INFO, DEBUG) | INFO |
| env.METRIC_UPDATE_INTERVAL | Interval at which to update CloudWatch metrics, in seconds. | |
| | Metrics are published to CloudWatch at 2x the interval | 30 |
| serviceAccount.name | The name of the ServiceAccount to use | nil |
| serviceAccount.create | Specifies whether a ServiceAccount should be created | true |
| serviceAccount.annotations | Specifies the annotations for ServiceAccount | {} |
| podAnnotations | Specifies the annotations for pods | {} |
| revisionHistoryLimit | The number of revisions to keep | 10 |
| podSecurityContext | SecurityContext to set on the pod | {} |
| containerSecurityContext | SecurityContext to set on the container | {} |

| Parameter | Description | Default |
| -------------------------------|---------------------------------------------------------------|-------------------------------------|
| `affinity` | Map of node/pod affinities | `{}` |
| `fullnameOverride` | Override the fullname of the chart | `cni-metrics-helper` |
| `image.tag` | Image tag | `v1.17.1` |
| `image.domain` | ECR repository domain | `amazonaws.com` |
| `image.region` | ECR repository region to use. Should match your cluster | `us-west-2` |
| `image.account` | ECR repository account number | `602401143452` |
| `env.USE_CLOUDWATCH` | Whether to export CNI metrics to CloudWatch | `true` |
| `env.USE_PROMETHEUS` | Whether to export CNI metrics to Prometheus | `false` |
| `env.AWS_CLUSTER_ID` | ID of the cluster to use when exporting metrics to CloudWatch | `default` |
| `env.AWS_VPC_K8S_CNI_LOGLEVEL` | Log verbosity level (ie. FATAL, ERROR, WARN, INFO, DEBUG) | `INFO` |
| `env.METRIC_UPDATE_INTERVAL` | Interval at which to update CloudWatch metrics, in seconds. | |
| | Metrics are published to CloudWatch at 2x the interval | `30` |
| `serviceAccount.name` | The name of the ServiceAccount to use | `nil` |
| `serviceAccount.create` | Specifies whether a ServiceAccount should be created | `true` |
| `serviceAccount.annotations` | Specifies the annotations for ServiceAccount | `{}` |
| `podAnnotations` | Specifies the annotations for pods | `{}` |
| `revisionHistoryLimit` | The number of revisions to keep | `10` |
| `podSecurityContext` | SecurityContext to set on the pod | `{}` |
| `containerSecurityContext` | SecurityContext to set on the container | `{}` |
| `tolerations` | Optional deployment tolerations | `[]` |
| `updateStrategy` | Optional update strategy | `{}` |
| `imagePullSecrets` | Docker registry pull secret | `[]` |
| `nodeSelector` | Node labels for pod assignment | `{}` |
| `tolerations` | Optional deployment tolerations | `[]` |
Comment on lines +80 to +84
Copy link
Contributor Author

@guessi guessi Apr 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Keep new values as empty to avoid existing setup break.



Specify each parameter using the `--set key=value[,key=value]` argument to `helm install` or provide a YAML file containing the values for the above parameters:

Expand Down
2 changes: 2 additions & 0 deletions charts/cni-metrics-helper/templates/clusterrole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "cni-metrics-helper.fullname" . }}
labels:
{{ include "cni-metrics-helper.labels" . | indent 4 }}
rules:
- apiGroups: [""]
resources:
Expand Down
24 changes: 22 additions & 2 deletions charts/cni-metrics-helper/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ metadata:
namespace: {{ .Release.Namespace }}
labels:
k8s-app: cni-metrics-helper
{{ include "cni-metrics-helper.labels" . | indent 4 }}
Copy link
Contributor Author

@guessi guessi Apr 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, the motivation of the PR is to fix this issue, but I think it could be better.

Without this change, command from NOTES.txt would not work.

Copy link
Member

Choose a reason for hiding this comment

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

This will add label to the deployment, which is fine.

I think, there is an additional change required in order to meet the NOTES.txt description.

diff --git a/charts/cni-metrics-helper/templates/deployment.yaml b/charts/cni-metrics-helper/templates/deployment.yaml
index 5cb2fb13..a33ccc44 100644
--- a/charts/cni-metrics-helper/templates/deployment.yaml
+++ b/charts/cni-metrics-helper/templates/deployment.yaml
@@ -23,6 +23,8 @@ spec:
       {{- end }}
       {{- end }}
       labels:
+        app.kubernetes.io/name: {{ include "cni-metrics-helper.name" . }}
+        app.kubernetes.io/instance: {{ .Release.Name }}
         k8s-app: cni-metrics-helper
     spec:
       containers:

You can bring this in separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops... my mistake, thanks for fixing that.

spec:
{{- if .Values.updateStrategy }}
strategy: {{ toYaml .Values.updateStrategy | nindent 4 }}
{{- end }}
revisionHistoryLimit: {{ .Values.revisionHistoryLimit }}
selector:
matchLabels:
Expand Down Expand Up @@ -36,6 +40,22 @@ spec:
name: cni-metrics-helper
image: "{{- if .Values.image.override }}{{- .Values.image.override }}{{- else }}{{- .Values.image.account }}.dkr.ecr.{{- .Values.image.region }}.{{- .Values.image.domain }}/cni-metrics-helper:{{- .Values.image.tag }}{{- end}}"
serviceAccountName: {{ template "cni-metrics-helper.serviceAccountName" . }}
{{- if .Values.podSecurityContext }}
{{- with .Values.tolerations }}
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.podSecurityContext }}
securityContext: {{ toYaml .Values.podSecurityContext | nindent 8 }}
{{- end }}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.affinity }}
affinity:
{{- toYaml . | nindent 8 }}
{{- end }}
31 changes: 31 additions & 0 deletions charts/cni-metrics-helper/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,34 @@ podSecurityContext: {}
containerSecurityContext: {}

podAnnotations: {}

imagePullSecrets: []

updateStrategy: {}
# type: RollingUpdate
# rollingUpdate:
# maxUnavailable: "10%"

nodeSelector: {}

tolerations: []
# - operator: Exists

affinity: {}
# nodeAffinity:
# requiredDuringSchedulingIgnoredDuringExecution:
# nodeSelectorTerms:
# - matchExpressions:
# - key: "kubernetes.io/os"
# operator: In
# values:
# - linux
# - key: "kubernetes.io/arch"
# operator: In
# values:
# - amd64
# - arm64
# - key: "eks.amazonaws.com/compute-type"
# operator: NotIn
# values:
# - fargate
Loading