feat: add optional PodDisruptionBudget for vector aggregators

[stigglor]

Adds an opt-in PodDisruptionBudget for VectorAggregator and ClusterVectorAggregator, configured per aggregator via spec.podDisruptionBudget.

podDisruptionBudget:
  enabled: false
  maxUnavailable: 1              # or minAvailable
  unhealthyPodEvictionPolicy: AlwaysAllow

Disabled by default, so upgrades leave existing aggregators untouched. unhealthyPodEvictionPolicy defaults to AlwaysAllow so unhealthy pods cannot block a node drain. The budget is removed when disabled or scaled to one replica.

Closes #228

[aa1ex]

Hi, and thanks for the contribution!

The HPA autoscaling work (#230) just merged, so this PR now conflicts with main and needs a rebase. Most of it is mechanical (shared RBAC and the aggregator controller wiring).

One real change beyond the textual conflicts: spec.replicas is now a *int32 (it was int32), so the replicas <= 1 check in pdb.go needs a small update for the pointer. One more thing worth handling: when autoscaling is on, the replica count is set by the HPA, not by spec.replicas, so that same check can skip the PDB for an aggregator that is actually running several replicas. Happy to help with that part if useful.

What do you think?

[stigglor]

Hi, and thanks for the contribution!

The HPA autoscaling work (#230) just merged, so this PR now conflicts with main and needs a rebase. Most of it is mechanical (shared RBAC and the aggregator controller wiring).

One real change beyond the textual conflicts: spec.replicas is now a *int32 (it was int32), so the replicas <= 1 check in pdb.go needs a small update for the pointer. One more thing worth handling: when autoscaling is on, the replica count is set by the HPA, not by spec.replicas, so that same check can skip the PDB for an aggregator that is actually running several replicas. Happy to help with that part if useful.

What do you think?

Cool, thanks for the info. I've fixed up the PR regarding those points you mentioned. Lmk if the HPA check is as you expected.

[aa1ex]

The autoscaling check is as I expected: maxReplicas when autoscaling is on, otherwise replicas (unset = 1). The rebase and *int32 are clean.

One thing before merge. With maxUnavailable: 1, one not-Ready pod drives disruptionsAllowed to 0, so the remaining Ready pod can't be evicted and draining its node never completes while the other replica stays unhealthy. Because the PodDisruptionBudget is created by default, every existing aggregator with more than one effective replica (replicas > 1, or autoscaling maxReplicas > 1) gets one on the next operator upgrade. The problem surfaces later, the first time someone drains that node.

The #228 use case is fine; I'd make it safe by default:

• Put it behind a default-off flag (--enable-aggregator-pdb, like --enable-config-optimization) so an upgrade doesn't change existing aggregators.
• Let maxUnavailable be configurable instead of hardcoded 1 (the follow-up you noted in Include PodDisruptionBudget for VectorAggregator and ClusterVectorAggregator #228).
• Set unhealthyPodEvictionPolicy: AlwaysAllow so not-Ready pods stay evictable regardless of the budget. This only helps once the unhealthy replicas already exceed the budget (with two replicas, when both are unhealthy); it doesn't unblock the case above, where the blocked pod is the Ready one.

[stigglor]

The autoscaling check is as I expected: maxReplicas when autoscaling is on, otherwise replicas (unset = 1). The rebase and *int32 are clean.

One thing before merge. With maxUnavailable: 1, one not-Ready pod drives disruptionsAllowed to 0, so the remaining Ready pod can't be evicted and draining its node never completes while the other replica stays unhealthy. Because the PodDisruptionBudget is created by default, every existing aggregator with more than one effective replica (replicas > 1, or autoscaling maxReplicas > 1) gets one on the next operator upgrade. The problem surfaces later, the first time someone drains that node.

The #228 use case is fine; I'd make it safe by default:

• Put it behind a default-off flag (--enable-aggregator-pdb, like --enable-config-optimization) so an upgrade doesn't change existing aggregators.
• Let maxUnavailable be configurable instead of hardcoded 1 (the follow-up you noted in Include PodDisruptionBudget for VectorAggregator and ClusterVectorAggregator #228).
• Set unhealthyPodEvictionPolicy: AlwaysAllow so not-Ready pods stay evictable regardless of the budget. This only helps once the unhealthy replicas already exceed the budget (with two replicas, when both are unhealthy); it doesn't unblock the case above, where the blocked pod is the Ready one.

Thanks, that's a fair concern. Making it default-on was the wrong call for exactly the drain scenario you describe. I'd like to fold all three points into a single podDisruptionBudget block on the aggregator spec rather than a global flag:

podDisruptionBudget:
  enabled: false
  maxUnavailable: 1
  unhealthyPodEvictionPolicy: AlwaysAllow

enabled defaults to false, so upgrades don't touch existing aggregators. maxUnavailable (or minAvailable) becomes configurable, and unhealthyPodEvictionPolicy defaults to AlwaysAllow. It keeps the PDB config in one place and gives per-aggregator control, which felt cleaner than a global flag that still couldn't carry maxUnavailable.

Agreed that none of this fully removes the blocked-drain case, since that's really what a PDB is for. This just makes it opt-in and gives an escape hatch.

Happy to go with a --enable-aggregator-pdb flag instead if you'd rather match the config-optimization convention (this also prevents a CR change, which I originally called out trying to prevent when logging the issue). Let me know which you prefer and I'll push it.

[aa1ex]

Let's go with the block over a flag, that's the cleaner fit.

[aa1ex]

LGTM!