Skip to content

Sync kubex charts from automation-controller main @ f1e348f#114

Merged
gasarekubex merged 1 commit into
masterfrom
sync/kubex-charts/main-f1e348f
May 20, 2026
Merged

Sync kubex charts from automation-controller main @ f1e348f#114
gasarekubex merged 1 commit into
masterfrom
sync/kubex-charts/main-f1e348f

Conversation

@kubexautomation
Copy link
Copy Markdown
Contributor

This PR was created automatically to sync managed Kubex chart content.

Source commit:

  • f1e348f

Managed chart scope:

  • charts/kubex-automation-engine
  • charts/kubex-crds

@kubexautomation kubexautomation Bot requested a review from a team as a code owner May 20, 2026 16:15
Copy link
Copy Markdown
Contributor Author

@kubexautomation kubexautomation Bot left a comment

Choose a reason for hiding this comment

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

CONTENT OF THIS REVIEW IS AI GENERATED

Overall Assessment

This is a well-scoped automated sync PR introducing the new PodAffinityPolicy CRD, its associated RBAC rules, a self-protection annotation on the Deployment, and documentation improvements. The CRD schema itself is structurally sound and follows existing patterns. Two actionable issues were found across the docs and templates.

Risk level: Low–Medium


Critical issues

  • None

Major issues

  • Hardcoded args[3] index in Troubleshooting.md (Troubleshooting.md:18): The kubectl patch snippet hardcodes /spec/template/spec/containers/0/args/3 as the path for --zap-log-level. Since args are rendered from a Helm template helper, any future reordering silently patches the wrong argument. See inline comment for a safer approach.

  • Deployment-level vs Pod-template-level annotation (deployment.yaml:13): The rightsizing.kubex.ai/pause-until: infinite annotation is added to the Deployment's own metadata.annotations, but an identical annotation already exists on .spec.template.metadata.annotations. Confirm which level the controller evaluates; if only the Pod template annotation is needed, the Deployment-level copy is redundant and may mislead operators.

Minor issues

  • Copy-paste artifact in CRD description (podaffinitypolicies.yaml:165): status.conditions says "StaticPolicy resource" instead of "PodAffinityPolicy resource".
  • Inconsistent field name in docs (Policy-Configuration.md:75): Section heading uses namespaces.values but the CRD and all other docs use namespaceSelector.values.
  • workloadTypes default vs enum ordering mismatch (podaffinitypolicies.yaml:131): Default list order and enum list order differ, inconsistent with other CRDs in the chart.

DRY improvement opportunities

  • The rightsizing.kubex.ai/pause-until: infinite annotation now appears in two places in the same template (Deployment metadata + Pod template metadata). If the Deployment-level copy is intentional, a shared comment block or a single YAML anchor would reduce the risk of them drifting.

Suggested next steps

  1. Verify the correct annotation level (Deployment vs Pod template) and remove the redundant one if applicable.
  2. Fix the args/3 hardcoded index in the troubleshooting docs before this ships to avoid operator incidents.
  3. Correct the copy-paste description in the CRD (StaticPolicyPodAffinityPolicy) and align the field name in Policy-Configuration.md.

```

Revert back to info:

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.

CONTENT OF THIS REVIEW IS AI GENERATED

[Severity: Major] [Confidence: High]

Location: charts/kubex-automation-engine/docs/Troubleshooting.md:18

Issue: The kubectl patch command hardcodes /spec/template/spec/containers/0/args/3 as the JSON Patch path for --zap-log-level, which will silently break if the args list order ever changes.

Why it matters: args in the Deployment are rendered via {{- include "kubex-automation-engine.managerArgs" . | nindent 8 }}, whose output order is determined by the Helm template. If a future Helm release inserts or removes an arg before index 3, the patch will overwrite the wrong argument—potentially disabling a critical flag—without producing any visible error. This is an operational hazard for users who follow the documentation.

Suggested fix: Replace the index-based approach with a value filter to make the command index-agnostic. For example:

# Find the current index of the --zap-log-level arg
INDEX=$(kubectl -n kubex get deploy/$(kubectl -n kubex get deploy -l app.kubernetes.io/name=kubex-automation-engine -o jsonpath='{.items[0].metadata.name}') \
  -o jsonpath='{range .spec.template.spec.containers[0].args[*]}{@}{"\n"}{end}' \
  | grep -n 'zap-log-level' | cut -d: -f1)
INDEX=$((INDEX - 1))  # zero-indexed

kubectl -n kubex patch deploy/... --type='json' \
  -p="[{\"op\":\"replace\",\"path\":\"/spec/template/spec/containers/0/args/${INDEX}\",\"value\":\"--zap-log-level=debug\"}]"

Alternatively, document the use of --zap-log-level exclusively via helm upgrade --set controllerManager.extraArgs[0]=... (the persistent path already documented below) to avoid the fragile index reference altogether.

metadata:
name: {{ include "kubex-automation-engine.fullname" . }}
namespace: {{ include "kubex-automation-engine.namespace" . }}
annotations:
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.

CONTENT OF THIS REVIEW IS AI GENERATED

[Severity: Major] [Confidence: Medium]

Location: charts/kubex-automation-engine/templates/deployment.yaml:13

Issue: The rightsizing.kubex.ai/pause-until: infinite annotation is added to the Deployment metadata but the same annotation already existed on the Pod template metadata (.spec.template.metadata.annotations). The Deployment-level annotation is not the mechanism the controller uses to pause rightsizing; the controller reads the annotation on the Pod template spec, not on the enclosing Deployment object itself.

Why it matters: If the controller evaluates this annotation on the Deployment object it would work, but most admission-time/recommendation-time controllers inspect the Pod or Pod template annotations, not the Deployment-level metadata. A mismatch could result in the engine still attempting to automate its own Deployment despite the intent expressed by the comment, or it may be dead/redundant metadata that misleads operators. Given the Pod template already carries the annotation, the Deployment-level copy may be harmless—but if it is relied upon for correctness it should be validated.

Suggested fix: Confirm whether the rightsizing.kubex.ai/pause-until annotation is evaluated at Deployment-level or Pod-level by the controller. If Pod-level is sufficient (as is typical for admission controllers), remove the newly added Deployment-level annotation to avoid confusion. If both levels are intentionally checked, add a comment explaining why both are needed.

minimum: 0
type: integer
required:
- affinity
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.

CONTENT OF THIS REVIEW IS AI GENERATED

[Severity: Minor] [Confidence: High]

Location: charts/kubex-crds/templates/rightsizing.kubex.ai_podaffinitypolicies.yaml:165

Issue: The status.conditions description copy-pastes "represent the current state of the StaticPolicy resource" instead of "PodAffinityPolicy resource".

Why it matters: Stale copy-paste in the CRD schema description is surfaced in kubectl describe output and any OpenAPI-aware tooling (e.g., kubectl explain podaffinitypolicies.status.conditions). It will mislead users debugging PodAffinityPolicy resources and indicates the schema block was templated from StaticPolicy without a full find-replace pass.

Suggested fix: Change the description to reference PodAffinityPolicy:

description: |-
  conditions represent the current state of the PodAffinityPolicy resource.
  ...

| `policy.policies.<name>.safetyChecks.maxAnalysisAgeDays` | `ClusterProactivePolicy.spec.safetyChecks.maxAnalysisAgeDays` | Per-policy value wins over top-level `policy.safetyChecks.maxAnalysisAgeDays`. |
| `policy.safetyChecks.maxAnalysisAgeDays` | `ClusterProactivePolicy.spec.safetyChecks.maxAnalysisAgeDays` | Backward-compatible fallback when not set per policy. |

### Namespace wildcards in `scope[].namespaces.values`
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.

CONTENT OF THIS REVIEW IS AI GENERATED

[Severity: Minor] [Confidence: High]

Location: charts/kubex-automation-engine/docs/Policy-Configuration.md:75

Issue: The new section heading references scope[].namespaces.values but all other documentation and the ClusterProactivePolicy CRD field path use scope.namespaceSelector.values. The Cluster-Proactive-Policies.md doc updated in this same PR correctly refers to spec.scope.namespaceSelector.values.

Why it matters: Users searching the docs for the correct field path will find two different names for the same concept (namespaces.values vs namespaceSelector.values), leading to confusion and likely misconfiguration.

Suggested fix: Align the heading and prose to match the canonical field name used in the CRD and other docs:

### Namespace wildcards in `scope[].namespaceSelector.values`

`scope[].namespaceSelector.values` supports shell-style `*` wildcards ...

type: object
workloadTypes:
default:
- Deployment
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.

CONTENT OF THIS REVIEW IS AI GENERATED

[Severity: Minor] [Confidence: High]

Location: charts/kubex-crds/templates/rightsizing.kubex.ai_podaffinitypolicies.yaml:131

Issue: The workloadTypes default list order differs between the new PodAffinityPolicy CRD and other policies in the chart. The CRD lists [Deployment, StatefulSet, CronJob, Rollout, Job, AnalysisRun, DaemonSet] but the workloadTypes items enum lists [Deployment, StatefulSet, DaemonSet, CronJob, Rollout, Job, AnalysisRun].

Why it matters: While functionally equivalent (defaults and enums are sets), inconsistency across CRDs in the same chart makes maintenance harder and can confuse users comparing schemas. It also signals a copy-paste from two different sources without reconciliation.

Suggested fix: Align the default list order with the items enum order, or align all CRDs in this chart to use the same canonical ordering (e.g., [Deployment, StatefulSet, DaemonSet, CronJob, Rollout, Job, AnalysisRun]).

@gasarekubex gasarekubex merged commit 481cc1b into master May 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant