(Fix) Updating helm charts to support version 3 windows integration #754
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ rules: | |
| - "services" | ||
| - "nodes" | ||
| - "namespaces" | ||
| - "pods" # Needed for our Windows integration to be able to access KSM on a Linux node. | ||
|
There was a problem hiding this comment. @RagalahariP From v2 to v3, the pods resource is removed for linux support for some reason as you mentioned during the meeting. The reason may be reducing the security surface. What I mean during the meeting is if we can adding it back for windows only, that will be better . There was a problem hiding this comment. @xqi-nr after you left the meeting we agreed that this comment is a good compromise. @RagalahariP will contact @paologallinaharbur to request his review of this line change since he was involved in the project to upgrade V2 to V3 and may have historical context to validate this change There was a problem hiding this comment. Hello! Yes, I'll take a look! In general, we have never added support for Windows due to the lack of a canary environment, tooling, and proper pipelines to build the images. There was a problem hiding this comment. By the way, answering this particular point, it was a permission not needed anymore and therefore it was dropped. |
||
| verbs: [ "get", "list", "watch" ] | ||
| - nonResourceURLs: ["/metrics"] | ||
| verbs: ["get"] | ||
|
|
||
|
There was a problem hiding this comment. I may not understand the approach. However, this seems a V2 component being merged into the V3 chart. The image used is |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| {{- if and .Values.kubelet.enabled .Values.kubelet.enableWindows }} | ||
| {{- range .Values.kubelet.windowsOsList }} | ||
| apiVersion: apps/v1 | ||
| kind: DaemonSet | ||
| metadata: | ||
| namespace: {{ $.Release.Namespace }} | ||
| name: {{ include "nriKubernetes.kubelet.windows.fullname" $ }}-{{ .version }} | ||
| {{- $legacyAnnotation:= fromYaml (include "newrelic.compatibility.annotations" $) -}} | ||
| {{- with include "newrelic.compatibility.valueWithFallback" (dict "legacy" $legacyAnnotation "supported" $.Values.kubelet.annotations )}} | ||
| annotations: {{ . | nindent 4 }} | ||
| {{- end }} | ||
| spec: | ||
| {{- with $.Values.updateStrategy }} | ||
| updateStrategy: {{ toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| selector: | ||
| matchLabels: | ||
| {{- include "newrelic.common.labels.selectorLabels" $ | nindent 6 }} | ||
| app.kubernetes.io/component: kubelet | ||
| template: | ||
| metadata: | ||
| annotations: | ||
| checksum/nri-kubernetes: {{ include (print $.Template.BasePath "/kubelet/scraper-configmap.yaml") $ | sha256sum }} | ||
|
There was a problem hiding this comment. Nri-kubernetes in V2 branch does not read any configuration, that was introduced in V3. I wonder if you actually compiled the V3 branch and pushed it to the old repo There are some discrepancies:
There was a problem hiding this comment. how a user could change the config mounted? or set a proxy? |
||
| checksum/agent-config: {{ include (print $.Template.BasePath "/kubelet/agent-configmap.yaml") $ | sha256sum }} | ||
| {{- if include "newrelic.common.license.secret" $ }}{{- /* If the is secret to template */}} | ||
| checksum/license-secret: {{ include (print $.Template.BasePath "/secret.yaml") $ | sha256sum }} | ||
| {{- end }} | ||
| labels: | ||
| {{- include "nriKubernetes.labels.podLabels" $ | nindent 8 }} | ||
| app.kubernetes.io/component: kubelet | ||
| spec: | ||
| {{- with include "newrelic.common.dnsConfig" $ }} | ||
| dnsConfig: | ||
| {{- . | nindent 8 }} | ||
| {{- end }} | ||
| serviceAccountName: {{ include "newrelic.common.serviceAccount.name" $ }} | ||
| containers: | ||
| - name: agent | ||
| image: newrelic/infrastructure-k8s:{{ .imageTag }} | ||
|
There was a problem hiding this comment. This is a V2 image, why not installing the nri-bundle-v2 next to the V3 then? In this way we are mixing them up in the same chart! |
||
| imagePullPolicy: {{ $.Values.images.integration.pullPolicy }} | ||
| env: | ||
| - name: NRIA_LICENSE_KEY | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "newrelic.common.license.secretName" $ }} | ||
| key: {{ include "newrelic.common.license.secretKeyName" $ }} | ||
| - name: "NRI_KUBERNETES_CLUSTERNAME" | ||
| value: {{ include "newrelic.common.cluster" $ }} | ||
| - name: "NRI_KUBERNETES_VERBOSE" | ||
| value: {{ include "newrelic.common.verboseLog.valueAsBoolean" $ | quote }} | ||
| - name: "NRK8S_NODE_NAME" | ||
| valueFrom: | ||
| fieldRef: | ||
| apiVersion: "v1" | ||
| fieldPath: "spec.nodeName" | ||
| - name: "CLUSTER_NAME" | ||
| value: {{ include "newrelic.common.cluster" $ }} | ||
| - name: "NRIA_PASSTHROUGH_ENVIRONMENT" | ||
| value: "KUBERNETES_SERVICE_HOST,KUBERNETES_SERVICE_PORT,CLUSTER_NAME,CADVISOR_PORT,NRK8S_NODE_NAME,KUBE_STATE_METRICS_URL,ETCD_TLS_SECRET_NAME,ETCD_TLS_SECRET_NAMESPACE,API_SERVER_SECURE_PORT,NRIA_CACHE_PATH,TIMEOUT,DISCOVERY_CACHE_TTL" | ||
| nodeSelector: | ||
| node.kubernetes.io/windows-build: {{ .buildNumber }} | ||
| kubernetes.io/os: windows | ||
| tolerations: | ||
| - key: "node.kubernetes.io/os" | ||
| operator: "Equal" | ||
| value: "windows" | ||
| effect: "NoSchedule" | ||
| --- | ||
| {{- end }} | ||
| {{- end }} | ||
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.
make sure this readme is auto-generated by
helm-docs, otherwise, it will be overwritten