Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions openshift/catalogd/manifests-experimental.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,14 @@ spec:
operator: In
values:
- linux
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- podAffinityTerm:
labelSelector:
matchLabels:
control-plane: catalogd-controller-manager
topologyKey: kubernetes.io/hostname
weight: 100
nodeSelector:
kubernetes.io/os: linux
node-role.kubernetes.io/control-plane: ""
Expand Down
8 changes: 8 additions & 0 deletions openshift/catalogd/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,14 @@ spec:
operator: In
values:
- linux
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- podAffinityTerm:
labelSelector:
matchLabels:
control-plane: catalogd-controller-manager
topologyKey: kubernetes.io/hostname
weight: 100
nodeSelector:
kubernetes.io/os: linux
node-role.kubernetes.io/control-plane: ""
Expand Down
10 changes: 10 additions & 0 deletions openshift/helm/catalogd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ options:
enabled: true
deployment:
image: ${CATALOGD_IMAGE}
replicas: 1
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avoid hard-coding catalogd to a single replica.

Line 11 fixes replicas at 1, which prevents HA rollout protection on multi-node clusters. This should follow cluster topology (SingleReplica vs HighlyAvailable) rather than being globally pinned.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openshift/helm/catalogd.yaml` at line 11, The manifest hard-codes "replicas:
1" which prevents HA; replace that literal with a Helm-driven value so replica
count follows cluster topology—e.g., remove the fixed "replicas: 1" in
catalogd.yaml and reference a chart value such as .Values.replicaCount (or use a
conditional on .Values.topology / .Values.highlyAvailable to choose 1 vs N) and
add the corresponding default in values.yaml; ensure the template and values are
used everywhere catalogd is deployed so multi-node clusters can scale replicas
appropriately.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is updated via cluster-olm-operator; so this value is appropriate here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@tmshort, understood — the replicas: 1 in the Helm chart is intentionally a conservative default that gets overridden by cluster-olm-operator based on the actual cluster topology (e.g. HA vs. SNO). I'll withdraw my concern. Thanks for the clarification!


📝 Learnings were identified and not saved (knowledge base disabled). Enable

podDisruptionBudget:
enabled: false
operatorController:
Expand All @@ -25,6 +26,15 @@ namespaces:
# Deployment values for catalogd
deployments:
templateSpec:
affinity:
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- weight: 100
podAffinityTerm:
topologyKey: kubernetes.io/hostname
labelSelector:
matchLabels:
control-plane: catalogd-controller-manager
priorityClassName: system-cluster-critical
securityContext:
seLinuxOptions:
Expand Down
10 changes: 10 additions & 0 deletions openshift/helm/operator-controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ options:
enabled: true
deployment:
image: ${OPERATOR_CONTROLLER_IMAGE}
replicas: 1
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Make replica count topology-aware instead of hard-coding 1.

Line 11 pins replicas to 1, so the HA path (multi-node clusters running 2 replicas with disruption protection) cannot be realized from this config. This should be topology-aware (for example, SingleReplica=1 and HighlyAvailable=2) and paired with PDB enablement in the same flow.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openshift/helm/operator-controller.yaml` at line 11, Replace the hard-coded
replicas: 1 with a topology-aware Helm/template-driven value and pair it with
PDB enablement; specifically, change the replicas field to use a templated value
(e.g., replicas: {{ .Values.topology.replicas }} or a helper like {{ include
"operator.replicas" . }}), implement a helper/template (operator.replicas or
values.topology.mode) that returns 1 for SingleReplica and 2 (or configurable HA
count) for HighlyAvailable, and ensure the same topology flag drives
creation/enabling of a PodDisruptionBudget resource so disruption protection is
applied in HA mode.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is updated via cluster-olm-operator; so this value is appropriate here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@tmshort, understood — thanks for the clarification! Since cluster-olm-operator overrides the replica count based on topology at a higher level, replicas: 1 here is the correct baseline default. I'll note this for future reviews.


📝 Learnings were identified and not saved (knowledge base disabled). Enable

podDisruptionBudget:
enabled: false
catalogd:
Expand All @@ -26,6 +27,15 @@ namespaces:
# Deployment values for operator-controller
deployments:
templateSpec:
affinity:
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- weight: 100
podAffinityTerm:
topologyKey: kubernetes.io/hostname
labelSelector:
matchLabels:
control-plane: operator-controller-controller-manager
priorityClassName: system-cluster-critical
securityContext:
seLinuxOptions:
Expand Down
8 changes: 8 additions & 0 deletions openshift/operator-controller/manifests-experimental.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1345,6 +1345,14 @@ spec:
operator: In
values:
- linux
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- podAffinityTerm:
labelSelector:
matchLabels:
control-plane: operator-controller-controller-manager
topologyKey: kubernetes.io/hostname
weight: 100
nodeSelector:
kubernetes.io/os: linux
node-role.kubernetes.io/control-plane: ""
Expand Down
8 changes: 8 additions & 0 deletions openshift/operator-controller/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,14 @@ spec:
operator: In
values:
- linux
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- podAffinityTerm:
labelSelector:
matchLabels:
control-plane: operator-controller-controller-manager
topologyKey: kubernetes.io/hostname
weight: 100
nodeSelector:
kubernetes.io/os: linux
node-role.kubernetes.io/control-plane: ""
Expand Down