added gpu-process-exporter chart, changed logos

[tsipo]

This is the first PR to include the gpu-process-exporter chart (and fix some outdated logos), after approval and chart publication we'll issue a second PR to update dependencies on the gpu-process-exporter chart.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: This PR introduces a well-structured new gpu-process-exporter chart alongside clean logo URL updates across the repo. The chart architecture is sound, but there are several correctness and usability issues in the new chart — one data loss regression in an existing chart (appVersion removal), and a common Helm convention gap that could cause multi-install conflicts.

Risk level: Medium

---

Critical issues

• None

Major issues

1. kubex-crds/Chart.yaml — appVersion field accidentally removed. The diff swaps icon/version but drops appVersion: v0.1 entirely. Any tooling relying on appVersion (ArgoCD, Artifact Hub, release pipelines) will break or misreport the deployed app version. (inline comment on charts/kubex-crds/Chart.yaml:4)

2. daemonset.yaml — Empty annotations: block rendered unconditionally. When both prometheusScrape.annotate and service.annotations are falsy/empty (the default annotations: {}), the Service manifest still emits a bare or empty annotations: key, which may fail strict linting and is noisy in diffs. (inline comment on charts/gpu-process-exporter/templates/daemonset.yaml:69)

3. daemonset.yaml — NVML_SEARCH_PATH value not quoted. All other value: fields in the env block use | quote; this one does not. A path with special characters or spaces will produce malformed YAML. (inline comment on charts/gpu-process-exporter/templates/daemonset.yaml:48)

4. rbac.yaml — serviceAccount.create / rbac.create flag independence is a footgun. When rbac.create: true but serviceAccount.create: false and no serviceAccount.name is given, the required guard in _helpers.tpl produces a cryptic failure. When a name is given, the ClusterRoleBinding targets an externally-managed SA — valid but undocumented. (inline comment on charts/gpu-process-exporter/templates/rbac.yaml:1)

Minor issues

5. values.yaml — image.tag: "1.0.0" contradicts README's mandatory designation. Because a value is already present, the required enforcement in _helpers.tpl will never fire. Users silently get 1.0.0 if they forget to override. (inline comment on charts/gpu-process-exporter/values.yaml:3)

6. daemonset.yaml / rbac.yaml — Resource names are hardcoded to {{ .Chart.Name }}. Standard Helm convention uses {{ .Release.Name }}-{{ .Chart.Name }} (via a fullname helper) to allow multiple installs in the same namespace. Hardcoding the chart name causes name conflicts and makes ownership harder to identify. (inline comment on charts/gpu-process-exporter/templates/daemonset.yaml:1)

DRY improvement opportunities

• The app.kubernetes.io/name and app.kubernetes.io/instance label pairs are repeated across daemonset.yaml (DaemonSet metadata, selector, pod template, Service) and rbac.yaml. A shared gpu-process-exporter.labels helper would reduce duplication and ensure label consistency.
• The helper functions in _helpers.tpl that simply wrap default (e.g. imageRepository, imagePullPolicy, serviceType) add indirection without meaningful logic. These could be inlined at the call site or replaced by a single common helper, similar to what the helm/common library chart provides.

Suggested next steps

1. Restore appVersion: v0.1 in charts/kubex-crds/Chart.yaml (or update it intentionally) before merging.
2. Fix the NVML_SEARCH_PATH quoting issue and wrap the Service annotations: block conditionally.
3. Add a {{- fail }} guard for the rbac.create + serviceAccount.create: false + no name combination in rbac.yaml or _helpers.tpl.
4. Decide on the image.tag contract: either null out the default in values.yaml (to make required meaningful) or document 1.0.0 as the real default and drop the mandatory marker.
5. Consider adding a fullname helper and a labels helper to align with standard Helm chart conventions and avoid future naming conflicts.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: This PR introduces a well-structured gpu-process-exporter Helm chart with a thoughtful design (JSON schema validation, configurable RBAC, node-scoped DaemonSet), alongside a clean logo URL refresh across existing charts. The new chart is production-functional but has a few operational and correctness gaps worth addressing before the first published release.

Risk level: Medium

---

Critical issues

None.

---

Major issues

• Empty annotations:{} rendered on Service by default (daemonset.yaml:69) — service.annotations: {} (the default) is truthy in Go templates, so $hasAnnotations is true even when no annotations should be emitted. This produces a bare annotations: block in the rendered manifest. Fix: use a length check (gt (len .Values.service.annotations) 0) or change the default to null. (See inline comment.)

• No resources limits/requests on the container (daemonset.yaml:55) — This DaemonSet runs on every GPU node as a privileged container; an unconstrained pod can starve co-located GPU workloads. Define resources.requests and resources.limits defaults in values.yaml and template them into the container spec. (See inline comment.)

---

Minor issues

• Open review thread on daemonset.yaml:48 is stale — The NVML_SEARCH_PATH quoting issue raised in a previous review is already addressed in the current diff (| quote is present). The thread should be resolved.

• Missing appVersion in gpu-process-exporter/Chart.yaml — The chart deploys densify/gpu-process-exporter:1.0.0 but does not declare appVersion. helm list will show a blank app version, making audit harder. Add appVersion: "1.0.0". (See inline comment.)

• prometheusScrape.annotate defaults to true — Opt-in annotations are safer as a default; clusters using ServiceMonitor-based scraping will accumulate unnecessary annotations silently. Consider defaulting to false or null. (See inline comment.)

• appVersion removed from kubex-crds/Chart.yaml — The author confirmed this is intentional, but it is a one-way metadata loss. Consider a comment in the YAML if appVersion is permanently dropped for this chart. (See inline comment.)

---

DRY improvement opportunities

• The _helpers.tpl file uses a consistent pattern of default <value> .Values.<field> wrappers, which is well-structured. No significant duplication observed.
• The logo URL https://kubex.ai/wp-content/uploads/kubex-logo-landscape.svg is now referenced across 9 files. If the URL ever changes again, a global search-and-replace is needed. Consider centralizing it — though this is a Helm limitation (no global constants file across charts), so this is purely advisory.

---

Suggested next steps

1. Address the service.annotations: {} truthy edge case — either change the default to null or tighten the $hasAnnotations guard.
2. Add a resources block to values.yaml and wire it into the container spec in daemonset.yaml.
3. Add appVersion: "1.0.0" to gpu-process-exporter/Chart.yaml.
4. Resolve the stale open inline thread on daemonset.yaml:48.
5. Decide and document whether prometheusScrape.annotate: true as the default is the intended experience.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: This PR adds a well-conceived gpu-process-exporter chart with good JSON schema validation, clear documentation, and a sensible RBAC design, alongside a clean logo URL refresh across all sibling charts. Previous review rounds (from this session) addressed the main correctness issues (resource limits, NVML_SEARCH_PATH quoting, annotations guard). The remaining findings below focus on new gaps not yet addressed by earlier rounds.

Risk level: Medium

---

Critical issues

None.

---

Major issues

1. Unguarded resources block with nil/empty risk (daemonset.yaml:55) — {{- toYaml .Values.resources | nindent 12 }} is unconditional. If .Values.resources is null or emptied by a user override, it renders resources: null which the API server rejects, or an empty resources: key, making pods BestEffort. Guard with {{- if .Values.resources }}. (See inline comment.)

2. No ServiceAccount existence guard when serviceAccount.create is unset/nil (rbac.yaml:1) — If serviceAccount is cleared entirely from values (e.g. --set serviceAccount=null), no SA is created but the DaemonSet still demands "gpu-process-exporter". A top-level {{- fail }} guard covering the nil-SA path would prevent silent runtime failures. (See inline comment.)

3. prometheusScrape.annotate type confusion (daemonset.yaml:69) — The schema correctly constrains this to boolean | null, but there is no CI evidence that helm lint is run with schema validation enabled. If it isn't, a string value like "yes" passes the template but silently breaks Prometheus scraping. Confirm helm lint (which exercises values.schema.json) is part of the CI pipeline. (See inline comment.)

---

Minor issues

4. Hardcoded nodeSelector and tolerations (daemonset.yaml:20) — Both are baked into the template with no override path. Non-standard GPU node labeling or additional taints will require template edits. Exposing these as values.yaml keys with the current values as defaults is standard Helm practice. (See inline comment.)

5. Generic ClusterRole/ClusterRoleBinding default names (rbac.yaml:12) — gpu-exporter-role / gpu-exporter-binding are short/generic names that will collide if the chart is installed into two namespaces of the same cluster. Consider using gpu-process-exporter-role / gpu-process-exporter-binding as the default, or document the multi-tenant collision risk in the README. (See inline comment.)

6. Broad / host filesystem mount — undocumented security implication (daemonset.yaml:32) — The host-root volume exposes the entire node filesystem read-only to a privileged container. This is explained in the context of NVML library discovery but the security surface is not explicitly called out in the README. Adding a brief security note would help operators make an informed deployment decision. (See inline comment.)

---

DRY improvement opportunities

• The label pair app.kubernetes.io/name / app.kubernetes.io/instance is repeated five times across daemonset.yaml and rbac.yaml. A gpu-process-exporter.labels helper in _helpers.tpl would reduce this boilerplate and ensure consistency if the label set ever changes.
• The _helpers.tpl wrappers that are purely default X .Values.Y (e.g. imageRepository, imagePullPolicy, serviceType) add indirection with no logic; consider inlining them or using the common library chart for a more idiomatic structure.

---

Suggested next steps

1. Guard the resources block in daemonset.yaml with {{- if .Values.resources }} to prevent nil-render rejection.
2. Add a top-level fail guard in rbac.yaml covering the case where serviceAccount is cleared to null entirely.
3. Expose nodeSelector and tolerations as configurable values.yaml keys with the current values as defaults.
4. Rename the default ClusterRole/Binding names to gpu-process-exporter-role / gpu-process-exporter-binding (more specific, less collision-prone).
5. Add a brief security note to the README about the privileged + full-node-root-mount combination.
6. Confirm helm lint (which validates values.schema.json) runs in CI.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: This PR introduces a well-structured gpu-process-exporter chart with a thorough README, JSON schema validation, and sensible security defaults. The logo updates across all existing charts are clean and consistent. A prior review pass has already been addressed by the author; all 18 prior threads are resolved. The remaining findings below are new, net-new issues not covered by the previous review.

Risk level: Low

---

Critical issues:

• None

Major issues:

• Cross-namespace SA binding silent failure (rbac.yaml:1): When serviceAccount.create: false, the ClusterRoleBinding silently references a SA that may not exist in the release namespace. No template guard or documentation warns operators about this constraint. A NOTES.txt or README callout is the minimum mitigation.
• No limits.cpu in default resources (daemonset.yaml:56): The default resource block ships without limits.cpu, making the container Burstable QoS. On clusters with strict admission policies (e.g. OPA require-resource-limits), pods will be rejected with no obvious error. This should be documented so operators on such clusters know to add it explicitly.

Minor issues:

• {{- end}} missing space (daemonset.yaml:46): Inconsistent whitespace inside the closing Helm action tag — {{- end}} vs {{- end }} used everywhere else. Cosmetic but noisy in linting tools.
• prometheusScrape.annotate: true default undocumented intent (values.yaml:16): The default is intentional for prometheus-community/prometheus, but without a comment in values.yaml, operators using Prometheus Operator will be confused by the annotation's presence.
• Missing NOTES.txt (Chart.yaml:1): No post-install notes template. Given the non-obvious prerequisites (GPU-labeled nodes, hostPID, privileged), a brief operational summary would meaningfully improve the operator experience.

DRY improvement opportunities:

• The _helpers.tpl defines individual helpers for every scalar value (port, serviceType, pullPolicy, etc.) rather than using a single values lookup pattern. This is a valid chart-style choice, but it results in a large helpers file for what could be inline default calls in templates. Not a blocking concern, just worth noting for future template additions.

Suggested next steps:

1. Add a brief comment in values.yaml above prometheusScrape.annotate documenting that true is appropriate for prometheus-community/prometheus and false/null for Prometheus Operator setups.
2. Add a templates/NOTES.txt with the Service endpoint and a reminder about GPU node prerequisites.
3. Document in the README (under a "Service Account" section) that when serviceAccount.create: false, the named SA must exist in the release namespace before helm install.

[pgarousi]

@tsipo Here are my feedback:

1. Please also update the Kubex icon in these files:

• kubex-collection-gke-autopilot / Chart.yaml
• kubex-automation-stack / Chart.yaml

2. We should also add the Kubex icon to the kubex-automation-engine / README.md

3. The documentation reference in these files should be changed from 'https://www.docs.kubex.ai' to 'https://docs.kubex.ai':

• kubex-automation-stack / README.md
• gpu-process-exporter / README.md

[tsipo]

@tsipo Here are my feedback:

1. Please also update the Kubex icon in these files:

• kubex-collection-gke-autopilot / Chart.yaml
• kubex-automation-stack / Chart.yaml

2. We should also add the Kubex icon to the kubex-automation-engine / README.md
3. The documentation reference in these files should be changed from 'https://www.docs.kubex.ai' to 'https://docs.kubex.ai':

• kubex-automation-stack / README.md
• gpu-process-exporter / README.md

@pgarousi Thank you.

1. will be attended in a follow-up PR, as these two chartshave dependencies on other charts (which needs to be published first), I will fix the icon there when I update those dependencies. Every change to Chart.yaml publishes the chart, so no need to publish it twice.
2. fixed.
3. fixed

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: This is a well-structured PR that introduces the gpu-process-exporter chart with a solid values schema, thoughtful RBAC gating, and correct Helm templating. The logo/URL cleanup across sibling charts is consistent and clean. All prior review threads have been addressed (even if some were intentionally rejected by the author). The remaining findings are minor documentation and style issues; there are no new blocking correctness bugs.

Risk level: Low

Critical issues:

• None

Major issues:

• None (all previously raised major issues have been addressed or intentionally accepted by the author)

Minor issues:

• {{- end}} missing space before }} in daemonset.yaml:49 — inconsistent template formatting.
• image.tag is pre-populated as "1.0.0" in values.yaml but the README documents it as mandatory with no default — README should be corrected to reflect the actual behavior.
• Default resources.limits omits cpu — clusters with strict admission policies (OPA require-resource-limits) will silently reject pods; should be documented.
• Default clusterRoleName: gpu-exporter-role / clusterRoleBindingName: gpu-exporter-binding are generic names — the single-install constraint should be explicitly documented in the README to prevent operator confusion.
• prometheusScrape.annotate: true default lacks a comment explaining the intended Prometheus scraping model (annotation-based vs. ServiceMonitor).

DRY improvement opportunities:

• The _helpers.tpl defines separate helpers for each scalar value (port, serviceType, hostProcMount, etc.). This is acceptable for a new chart, though it is more verbose than necessary — default could be applied inline in the templates for simple scalars.

Pending human-reviewer request (from @pgarousi):

• charts/kubex-collection-gke-autopilot/Chart.yaml and charts/kubex-automation-stack/Chart.yaml icon URLs were not updated in this PR. The author has committed to updating them in a follow-up PR when dependency versions are bumped. This is acceptable — no action needed in this PR.

Suggested next steps:

1. Fix the {{- end}} spacing nit (line 49, daemonset.yaml).
2. Update the README configuration table for image.tag to remove the misleading mandatory marker and show the default value.
3. Add inline comments to values.yaml to clarify prometheusScrape.annotate intent, the single-install constraint for RBAC, and the deliberate omission of limits.cpu.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: This PR introduces a well-structured gpu-process-exporter Helm chart with solid schema validation and cleans up logos/URLs across the repo. The chart is thoughtfully documented. A few issues remain — two of which are tracked as unresolved review threads — and a handful of new correctness concerns are noted below.
Risk level: Medium

---

Issue 1 — Orphan --- separator inside conditional block

Location: charts/gpu-process-exporter/templates/rbac.yaml:11

[Severity: Major] [Confidence: High]

{{- if .Values.serviceAccount.create }}
...ServiceAccount...
---          ← this line is INSIDE the if-block
{{- end }}
{{- if .Values.rbac.create }}
...ClusterRole...

Issue: The --- document separator is emitted only when serviceAccount.create is true, but NOT when it's false. If serviceAccount.create: false and rbac.create: true, the ClusterRole YAML is rendered without a leading ---, which is still valid YAML but is inconsistent and fragile. More critically, if serviceAccount.create: true and rbac.create: false, the trailing --- is emitted at the end of the file with no following document, which some YAML parsers and helm lint may flag.

Suggested fix: Move the --- separator outside both {{- if }} blocks, placed unconditionally between the two sections and guarded:

{{- if .Values.serviceAccount.create }}
...ServiceAccount...
{{- end }}
{{- if and .Values.serviceAccount.create .Values.rbac.create }}
---
{{- end }}
{{- if .Values.rbac.create }}
...ClusterRole...

---

Issue 2 — rbac.create single flag prevents binding to a pre-existing ClusterRole

Location: charts/gpu-process-exporter/templates/rbac.yaml:30

[Severity: Major] [Confidence: High]

Issue: The single rbac.create boolean controls creation of both the ClusterRole and the ClusterRoleBinding. There is no way to bind a service account to a pre-existing/shared ClusterRole without also triggering re-creation of the ClusterRole (which would fail if the role already exists and is owned by another release). This is a common operational scenario in shared clusters.

Suggested fix: Either split into rbac.createClusterRole and rbac.createClusterRoleBinding boolean flags, or document the limitation clearly in the README.

---

Issue 3 — $hasAnnotations guard: empty map {} evaluates as falsy via gt (len ...) 0 but the expression is correct

Location: charts/gpu-process-exporter/templates/daemonset.yaml:74

[Severity: Minor] [Confidence: High]

{{- $hasAnnotations := or .Values.prometheusScrape.annotate (and .Values.service.annotations (gt (len .Values.service.annotations) 0)) }}

This is now correct — len {} returns 0, so gt 0 0 is false. No bare annotations: key will be emitted for the default annotations: {} case. ✅ This is confirmed fixed.

---

Issue 4 — prometheusScrape.annotate value passed directly as annotation value without boolean coercion

Location: charts/gpu-process-exporter/templates/daemonset.yaml:81

[Severity: Major] [Confidence: High]

prometheus.io/scrape: {{ .Values.prometheusScrape.annotate | quote }}

Issue: If prometheusScrape.annotate is set to a boolean true (as per values.yaml), | quote renders it as "true" which is what Prometheus expects. However, the $hasAnnotations guard uses .Values.prometheusScrape.annotate in a truthy check, meaning any non-nil value (including false) will also pass the or check and enter the annotations block. If a user sets prometheusScrape.annotate: false to disable scraping, the annotation block will still be rendered with prometheus.io/scrape: "false" — which is technically correct (Prometheus ignores pods with "false"), but the $hasAnnotations logic will incorrectly trigger the annotations block header even when the intent is to suppress it.

Suggested fix: Change the guard to explicitly check for true:

{{- $hasAnnotations := or (eq .Values.prometheusScrape.annotate true) (and .Values.service.annotations (gt (len .Values.service.annotations) 0)) }}

---

Issue 5 — Missing imagePullSecrets support for private registries

Location: charts/gpu-process-exporter/templates/daemonset.yaml:26

[Severity: Minor] [Confidence: High]

Issue: The DaemonSet spec has no imagePullSecrets field. Given that the default image is densify/gpu-process-exporter (Docker Hub), enterprises pulling from a mirrored private registry will have no way to inject pull secrets without patching the chart. All comparable charts in this repo (e.g. container-optimization-data-forwarder) typically support this.

Suggested fix: Add an optional imagePullSecrets value and wire it into the pod spec:

{{- if .Values.imagePullSecrets }}
      imagePullSecrets:
        {{- toYaml .Values.imagePullSecrets | nindent 8 }}
{{- end }}

---

Issue 6 — kubex-crds/Chart.yaml removes appVersion without replacement

Location: charts/kubex-crds/Chart.yaml:4

[Severity: Minor] [Confidence: Medium]

Issue: The existing appVersion: v0.1 line was removed with no replacement. While marked intentional by the author, this means helm list will show a blank app version for this chart, and any tooling (ArgoCD, Flux, release dashboards) that surfaces appVersion to correlate deployed CRD versions will silently lose that signal. At minimum, a comment in Chart.yaml documenting why appVersion is omitted would help future maintainers.

---

DRY Improvement Opportunities

• The _helpers.tpl defines individual named templates for every scalar value (imageRepository, imageTag, imagePullPolicy, port, serviceType, etc.). This is more verbose than necessary — most sibling charts in this repo access .Values.* directly. This pattern is defensible for validation (required, default) but consider consolidating trivial pass-throughs to reduce template surface area.
• Logo/icon URLs are now consistently updated across READMEs and Chart.yaml files ✅. The www.kubex.ai → kubex.ai and www.docs.kubex.ai → docs.kubex.ai cleanup is thorough across all touched files.

---

Suggested Next Steps

1. Fix the dangling --- separator in rbac.yaml (Issue 1) — this is the highest-risk correctness issue.
2. Address prometheusScrape.annotate: false rendering (Issue 4) — a two-line fix to the $hasAnnotations guard.
3. Consider imagePullSecrets support (Issue 5) before publishing if enterprise use cases are expected.
4. The two open/unresolved prior review threads (rbac.yaml lines 11 and 30) should be explicitly acknowledged or resolved before merge.