Add nodeAffinity support to all scheduled workloads

[smbecker]

Fixes #247.

Motivation

Users running on clusters with multiple node pools (e.g. GKE) need to pin Pulsar
components to specific nodes. Today the chart exposes nodeSelector and pod
anti-affinity, but no nodeAffinity, forcing workarounds like cordoning nodes.

This revives the work from #349 (by @MonicaMagoniCom), whose source branch was
deleted before it could be merged, and extends it to every other scheduled
workload in the chart.

Modifications

Adds an optional affinity.node_affinity setting to each component. When set, it
renders a requiredDuringSchedulingIgnoredDuringExecution node affinity term
built from a label key/operator/values match expression. When unset (the
default), no node affinity is applied, so existing deployments are unaffected.

Example:

broker:
  affinity:
    node_affinity:
      matchExpression:
        key: node-pool
        operator: In
        values:
          - pulsar-pool

[lhotari]

Thanks for reviving #349 — the nodeAffinity rendering itself works for the documented case. Two things need to change before this can land, and one design question is worth settling now rather than after release.

The blocker

helm template test charts/pulsar on this branch vs master with default values differs in exactly two places, and both are the sts-cleanup Jobs. The zookeeper one gains a required pod anti-affinity, which makes the pre-upgrade hook unschedulable on a 3-node cluster. Details inline on zookeeper-statefulset-upgrade.yaml.

That directly contradicts "When unset (the default), no node affinity is applied, so existing deployments are unaffected." Commit 6f1ffb3 says the same thing — "All blocks are guarded, so default behavior is unchanged" — and the blocks are guarded, but the podAntiAffinity guard is and .Values.affinity.anti_affinity .Values.<component>.affinity.anti_affinity, and both default to true. So it guards nothing by default.

Scope

The title says "all scheduled workloads", but 6 of 18 pod-producing templates have no support: bookkeeper-cluster-initialize.yaml, dekaf-deployment.yaml, oxia-coordinator-deployment.yaml, pulsar-cluster-initialize.yaml, pulsar-manager-cluster-initialize.yaml, pulsar-manager-statefulset.yaml. Even setting the init Jobs aside, Dekaf / Oxia coordinator / Pulsar Manager are component workloads people will expect to pin.

Separately: both cleanup Jobs also gained imagePullSecrets, nodeSelector, priorityClassName, tolerations, topologySpreadConstraints and container imagePullPolicy. That's described in 6f1ffb3 but not in the PR body — worth calling out there, and it might be cleaner as its own PR.

Checked and clean

All 12 changed templates read their own component's value path — no copy-paste errors. Node and pod affinity correctly render as siblings under a single affinity: mapping everywhere. And Exists/DoesNotExist with no values renders values: null, which is a nil slice and passes both kubeconform and API validation — that one's fine as-is.

_{Reviewed with Codex gpt-5.6-sol and Claude Opus 5; every finding reproduced locally by rendering the chart and validating with kubeconform -strict.}

[lhotari]

This blocks helm upgrade on a 3-node cluster with default values.

The defaults that make this fire are all on out of the box: affinity.anti_affinity: true (values.yaml:113), zookeeper.affinity.anti_affinity: true, zookeeper.affinity.type: requiredDuringSchedulingIgnoredDuringExecution (values.yaml:594), and zookeeper.replicaCount: 3 (values.yaml:544). This Job carries "helm.sh/hook": pre-upgrade.

Rendered on master vs this branch with no values overridden at all, this Job goes from a bare pod spec to:

      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
          - labelSelector:
              matchExpressions:
              - {key: "app",       operator: In, values: ["pulsar"]}
              - {key: "release",   operator: In, values: [test]}
              - {key: "component", operator: In, values: [zookeeper]}
            topologyKey: kubernetes.io/hostname

On a 3-node cluster running 3 ZK pods — one per node, enforced by that very same required rule — every hostname is excluded. The hook pod stays Pending, the pre-upgrade hook never completes, and helm upgrade blocks until timeout and fails. Our own values.yaml comment spells it out: "requiredDuringSchedulingIgnoredDuringExecution … requires at least one node per replica."

The broker cleanup Job escapes this only by luck — broker.affinity.type happens to default to preferred.

Suggested fix: don't give the cleanup hooks pod anti-affinity at all. They're short-lived single-shot Jobs that just run kubectl; spreading them across nodes buys nothing and costs this. If you do want the scheduling alignment, it needs to be opt-in rather than inherited from the component defaults.

[lhotari]

values entries (and key above) are interpolated unquoted, so any label value YAML reads as a scalar renders with the wrong type. This same block is copied into every changed template.

$ helm template test charts/pulsar \
    --set 'broker.affinity.node_affinity.matchExpression.key=topology.kubernetes.io/zone' \
    --set 'broker.affinity.node_affinity.matchExpression.operator=In' \
    --set 'broker.affinity.node_affinity.matchExpression.values[0]=true' \
    --set 'broker.affinity.node_affinity.matchExpression.values[1]=123' \
    -s templates/broker-statefulset.yaml
...
                values:
                - true
                - 123

kubeconform -kubernetes-version 1.31.0 -strict rejects it:

at '/spec/.../matchExpressions/0/values/0': got number, want null or string

Worth noting --set-string is not a workaround — I checked, and the template's unquoted output re-introduces the type regardless of how Helm stored the value. So values like true, 123, on and null are simply unreachable, and Gt/Lt are unusable entirely since they require numeric-looking strings.

Suggested change

	- {{ . }}
	- {{ . | quote }}

The key: line above needs | quote too.

[lhotari]

Design question worth settling before this ships, because once released the values shape is a compatibility commitment that's painful to replace.

This models a single matchExpression (singular) → one nodeSelectorTerm → always requiredDuringSchedulingIgnoredDuringExecution. That can't express multiple terms (OR), multiple expressions within a term (AND), matchFields, or weighted preferredDuringSchedulingIgnoredDuringExecution — all of which are common for node pinning. Anyone needing a soft preference or "either of two node pools" has to fork or post-render.

It also breaks the chart's own convention: the adjacent scheduling knobs (nodeSelector, tolerations, topologySpreadConstraints) are all raw toYaml passthroughs.

Have you considered just passing through native syntax?

  affinity:
    nodeAffinity: {}

rendered with {{- toYaml . | nindent 8 }} — less template code than the current 12 copies, fully expressive, no new vocabulary to document, and it sidesteps the quoting bug entirely since users write real YAML. The existing anti_affinity toggles can stay as they are.