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
60 changes: 60 additions & 0 deletions bundle/manifests/argoproj.io_argocds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,11 @@ spec:
by the ApplicationSet controller. Defaults to ArgoCDDefaultLogLevel
if not set. Valid options are debug,info, error, and warn.
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
Comment on lines +253 to +257

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Schema exposes priorityClassName, but runtime contract is incomplete.

These CRD fields are now accepted, but the linked codebase findings show no matching Go API fields in api/v1beta1/argocd_types.go and no reconciliation logic applying PriorityClassName to pod specs (for example in controllers/argocd/deployment.go). This makes the feature a silent no-op at runtime.

Please land the corresponding API struct fields (v1alpha1/v1beta1 as applicable) and controller wiring before release so accepted CR input is actually enforced on workloads.

As per path instructions, “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”

Also applies to: 1255-1259, 2472-2476, 4269-4273, 8301-8305, 8520-8524, 9175-9179, 13735-13739, 18632-18636, 20448-20452, 26256-26260, 30121-30125

🤖 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 `@bundle/manifests/argoproj.io_argocds.yaml` around lines 253 - 257, The CRD
schema now accepts priorityClassName fields in multiple component
configurations, but the corresponding Go API struct fields are missing from
api/v1beta1/argocd_types.go and the reconciliation logic to apply these values
to pod specs is missing from controllers/argocd/deployment.go. Add the
priorityClassName fields to the relevant Go struct definitions in the API types,
and then update the controller logic in the pod/deployment creation methods to
read these priorityClassName values from the CR and apply them to the generated
pod specs (by setting the PriorityClassName field on the PodSpec). Apply these
changes to all locations mentioned in the line ranges.

Sources: Path instructions, Linked repositories

resources:
description: Resources defines the Compute Resources required
by the container for ApplicationSet.
Expand Down Expand Up @@ -1292,6 +1297,11 @@ spec:
operations
format: int32
type: integer
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
processors:
description: Processors contains the options for the Application
Controller processors.
Expand Down Expand Up @@ -2530,6 +2540,11 @@ spec:
image:
description: Image is the Redis container image.
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for Redis.
Expand Down Expand Up @@ -4337,6 +4352,11 @@ spec:
description: MountSAToken describes whether you would like to
have the Repo server mount the service account token
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
replicas:
description: Replicas defines the number of replicas for argocd-repo-server.
Value should be greater than or equal to 0. Default is nil.
Expand Down Expand Up @@ -8379,6 +8399,11 @@ spec:
If empty, Prometheus uses the global scrape timeout.
type: string
type: object
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
replicas:
description: Replicas defines the number of replicas for argocd-server.
Default is nil. Value should be greater than or equal to 0.
Expand Down Expand Up @@ -8593,6 +8618,11 @@ spec:
description: OpenShiftOAuth enables OpenShift OAuth authentication
for the Dex server.
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to the Dex
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for Dex.
Expand Down Expand Up @@ -9385,6 +9415,11 @@ spec:
logformat:
description: 'Deprecated: use LogFormat instead.'
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for ApplicationSet.
Expand Down Expand Up @@ -14114,6 +14149,11 @@ spec:
operations
format: int32
type: integer
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
processors:
description: Processors contains the options for the Application
Controller processors.
Expand Down Expand Up @@ -19032,6 +19072,11 @@ spec:
image:
description: Image is the Redis container image.
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
remote:
description: Remote specifies the remote URL of the Redis container.
(optional, by default, a local instance managed by the operator
Expand Down Expand Up @@ -20858,6 +20903,11 @@ spec:
description: MountSAToken describes whether you would like to
have the Repo server mount the service account token
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
remote:
description: Remote specifies the remote URL of the Repo Server
container. (optional, by default, a local instance managed by
Expand Down Expand Up @@ -26676,6 +26726,11 @@ spec:
If empty, Prometheus uses the global scrape timeout.
type: string
type: object
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
replicas:
description: Replicas defines the number of replicas for argocd-server.
Default is nil. Value should be greater than or equal to 0.
Expand Down Expand Up @@ -30536,6 +30591,11 @@ spec:
description: OpenShiftOAuth enables OpenShift OAuth authentication
for the Dex server.
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to the Dex
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for Dex.
Expand Down
60 changes: 60 additions & 0 deletions config/crd/bases/argoproj.io_argocds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ spec:
by the ApplicationSet controller. Defaults to ArgoCDDefaultLogLevel
if not set. Valid options are debug,info, error, and warn.
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
Comment on lines +242 to +246

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Schema/controller contract is currently broken for priorityClassName.

These schema additions expose priorityClassName, but the linked argoproj-labs/argocd-operator findings show no corresponding API struct fields and no reconciliation logic applying PodSpec.PriorityClassName. In that state, users can set the field in CRs and it will be silently ignored at runtime. Please ship this schema change together with (or gated behind) the API/controller implementation to avoid a no-op user-facing feature.

As per path instructions, this focuses on a major cross-layer integration issue and avoids minor/nitpick feedback.

Also applies to: 1244-1248, 2461-2465, 4258-4262, 8290-8294, 8509-8513, 9164-9168, 13724-13728, 18621-18625, 20437-20441, 26245-26249, 30110-30114

🤖 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 `@config/crd/bases/argoproj.io_argocds.yaml` around lines 242 - 246, The
priorityClassName schema field is exposed in the CRD but lacks corresponding API
struct fields and reconciliation logic in the operator to actually apply it to
PodSpec.PriorityClassName, causing the field to be silently ignored at runtime.
Either remove all priorityClassName schema additions (including those at lines
1244-1248, 2461-2465, 4258-4262, 8290-8294, 8509-8513, 9164-9168, 13724-13728,
18621-18625, 20437-20441, 26245-26249, 30110-30114) from this CRD definition, or
ensure the corresponding API struct field and controller reconciliation logic
are implemented in the argoproj-labs/argocd-operator to actually set and apply
the priorityClassName value before merging this schema change.

Sources: Path instructions, Linked repositories

resources:
description: Resources defines the Compute Resources required
by the container for ApplicationSet.
Expand Down Expand Up @@ -1281,6 +1286,11 @@ spec:
operations
format: int32
type: integer
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
processors:
description: Processors contains the options for the Application
Controller processors.
Expand Down Expand Up @@ -2519,6 +2529,11 @@ spec:
image:
description: Image is the Redis container image.
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for Redis.
Expand Down Expand Up @@ -4326,6 +4341,11 @@ spec:
description: MountSAToken describes whether you would like to
have the Repo server mount the service account token
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
replicas:
description: Replicas defines the number of replicas for argocd-repo-server.
Value should be greater than or equal to 0. Default is nil.
Expand Down Expand Up @@ -8368,6 +8388,11 @@ spec:
If empty, Prometheus uses the global scrape timeout.
type: string
type: object
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
replicas:
description: Replicas defines the number of replicas for argocd-server.
Default is nil. Value should be greater than or equal to 0.
Expand Down Expand Up @@ -8582,6 +8607,11 @@ spec:
description: OpenShiftOAuth enables OpenShift OAuth authentication
for the Dex server.
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to the Dex
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for Dex.
Expand Down Expand Up @@ -9374,6 +9404,11 @@ spec:
logformat:
description: 'Deprecated: use LogFormat instead.'
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for ApplicationSet.
Expand Down Expand Up @@ -14103,6 +14138,11 @@ spec:
operations
format: int32
type: integer
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
processors:
description: Processors contains the options for the Application
Controller processors.
Expand Down Expand Up @@ -19021,6 +19061,11 @@ spec:
image:
description: Image is the Redis container image.
type: string
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
remote:
description: Remote specifies the remote URL of the Redis container.
(optional, by default, a local instance managed by the operator
Expand Down Expand Up @@ -20847,6 +20892,11 @@ spec:
description: MountSAToken describes whether you would like to
have the Repo server mount the service account token
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
remote:
description: Remote specifies the remote URL of the Repo Server
container. (optional, by default, a local instance managed by
Expand Down Expand Up @@ -26665,6 +26715,11 @@ spec:
If empty, Prometheus uses the global scrape timeout.
type: string
type: object
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to this component's
pod. The PriorityClass must already exist in the cluster.
type: string
replicas:
description: Replicas defines the number of replicas for argocd-server.
Default is nil. Value should be greater than or equal to 0.
Expand Down Expand Up @@ -30525,6 +30580,11 @@ spec:
description: OpenShiftOAuth enables OpenShift OAuth authentication
for the Dex server.
type: boolean
priorityClassName:
description: |-
PriorityClassName is the name of the PriorityClass resource to assign to the Dex
pod. The PriorityClass must already exist in the cluster.
type: string
resources:
description: Resources defines the Compute Resources required
by the container for Dex.
Expand Down