newrelic / helm-charts

Helm charts for New Relic applications
Apache License 2.0
97 stars 205 forks source link

[pixie-chart] - nodeSelector not working #945

Open kpiroto opened 1 year ago

kpiroto commented 1 year ago

Bug description

The nodeSelector configuration is applied to pixie-chart, but it seems to be ignored

Version of Helm and Kubernetes

Helm 3, Kubernetes 1.20

Which chart?

pixie-chart from nri-bundle chart

What happened?

I have configured nodeSelector on pixie-chart, but the NODE SELECTOR for the vizier-pem DaemonSet is none:

NAME                                         DESIRED   CURRENT   READY   UP-TO-DATE   AVAILABLE   NODE SELECTOR   AGE
daemonset.apps/newrelic-nrk8s-controlplane   0         0         0       0            0           <none>          84m
daemonset.apps/newrelic-nrk8s-kubelet        4         4         4       4            4           <none>          84m
daemonset.apps/vizier-pem                    4         4         4       4            4           <none>          20h

If I configure using the global nodeSelector option of nri-bundle chart it works:

NAME                                         DESIRED   CURRENT   READY   UP-TO-DATE   AVAILABLE   NODE SELECTOR   AGE
daemonset.apps/newrelic-nrk8s-controlplane   0         0         0       0            0           pixie=allowed   143m
daemonset.apps/newrelic-nrk8s-kubelet        0         0         0       0            0           pixie=allowed   143m
daemonset.apps/vizier-pem                    4         4         4       4            4           <none>          21h

What you expected to happen?

Run vizier-pem DaemonSet only in a set of nodes using nodeSelector

How to reproduce it?

Add the label pixie=allowed to a node, then install the helm chart:

helm install:

helm upgrade --install newrelic-bundle newrelic/nri-bundle \
 --namespace=newrelic \
 --set prometheus.enabled=true \
 --set newrelic-pixie.enabled=true \
 --set newrelic-pixie.apiKey=API_KEY \
 --set pixie-chart.enabled=true \
 --set pixie-chart.deployKey=DEPLOY_KEY \
 --set pixie-chart.pathces.vizier-pem=

I also tried to use the approach from this doc:

helm upgrade --install newrelic-bundle newrelic/nri-bundle \
 --namespace=newrelic \
 --set prometheus.enabled=true \
 --set newrelic-pixie.enabled=true \
 --set newrelic-pixie.apiKey=API_KEY \
 --set pixie-chart.enabled=true \
 --set pixie-chart.deployKey=DEPLOY_KEY \
 --set pixie-chart.patches.vizier-pem='\{\"spec\"\: \{\"template\"\: \{\"spec\"\: \{ \"nodeSelector\"\: \{\"pixie\"\: \"allowed\" \}\}\}\}\}'

Anything else we need to know?

I used terraform to apply those helm configurations, follows the relevant part of code:

resource "helm_release" "newrelic" {

  name             = "newrelic"
  repository       = "https://helm-charts.newrelic.com"
  chart            = "nri-bundle"
  namespace        = "newrelic"
  create_namespace = true
  version          = "4.8.7"
  timeout          = 900

  set {
    name  = "pixie-chart.enabled"
    value = true
  }
  set {
    name  = "newrelic-pixie.enabled"
    value = true
  }
  set {
    name  = "pixie-chart.nodeSelector.pixie"
    value = "allowed"
  }
  # set {
  #   name  = "pixie-chart.patches.vizier-pem"
  #   value = "'{\"spec\": {\"template\": {\"spec\": {\"nodeSelector\": {\"pixie\": \"allowed\" }}}}}'"
  # }
}

The latest px-dev-viziers.yaml and olm-crd.yaml CRDs are also applied.

Values from helm, after terraform apply:

helm --namespace newrelic get values newrelic
pixie-chart:
  enabled: true
  nodeSelector:
    pixie: allowed

Or this, when using the patch model:

pixie-chart:
  enabled: true
  patches:
    vizier-pem: '''{"spec": {"template": {"spec": {"nodeSelector": {"pixie": "allowed"
      }}}}}'''
maxlemieux commented 1 year ago

Could you please assign me this issue?

cdenneen commented 7 months ago

@maxlemieux any updates?

maxlemieux commented 7 months ago

Hi @cdenneen I took another look at this issue, and based on the upstream subchart values it seems like the setting needs to be under pod:

pixie-chart:
  pod:
    nodeSelector:
      pixie: allowed

Let me know if this gives the results you need. I've tested it successfully in the past. The documented patch method should also work.

cdenneen commented 7 months ago

Sorry I saw this but was hoping this would help with affinity but doesn’t. Thanks for follow up.

cdenneen commented 7 months ago

@maxlemieux in order to do this you'd require setting a label on all nodes like pixie: allowed which I'm not sure is best solution since really should be able to use affinity rules. You have a global.fargate: true setting which applies affinity rules to all child charts of nri-bundle so this should be no different.

cdenneen commented 7 months ago

The documented patch method should also work.

@maxlemieux since you have global.fargate: true can that be translated into a patch method to apply the affinity resulting in same as the rest of the charts? So while most you can apply the affinity to child charts directly, in this case if global.fargate: true it should apply that similar affinity as a patch. Basically the chart would convert the _helper fargate affinity into a json patch and set here. Gives you the same result without customers having to create one-off workarounds like this.