[CELEBORN-2354] Improve helm chart

[zemin-piao]

What changes were proposed in this pull request?

Fix helm chart indentation, podmonitor flags and remove the namespace option for priorityClass.

Why are the changes needed?

Cleanup the helm chart .

Does this PR resolve a correctness bug?

• Yes

Does this PR introduce any user-facing change?

• Yes

How was this patch tested?

No.

[SteNicholas]

@zemin-piao, thanks for contribution. Could you please create jira ticket which refers to #1053 for this issue?

[Copilot]

Pull request overview

This PR cleans up the Celeborn Helm chart by correcting template rendering details (indentation/conditionals), aligning generated config formatting, and ensuring cluster-scoped resources are rendered without inappropriate namespace fields. The accompanying Helm-unittest suites are updated to match the new rendered output (including updated config checksum hashes).

Changes:

• Remove metadata.namespace from PriorityClass manifests (cluster-scoped) and update related tests.
• Simplify the master PodMonitor creation gate into a single combined conditional.
• Make celeborn.master.internal.endpoints render with consistent key=value spacing and update checksum- and regex-based tests accordingly.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
charts/celeborn/templates/worker/priorityclass.yaml	Remove metadata.namespace from worker PriorityClass.
charts/celeborn/templates/master/priorityclass.yaml	Remove metadata.namespace from master PriorityClass.
charts/celeborn/templates/master/podmonitor.yaml	Combine podMonitor.enable and API availability checks into a single and condition.
charts/celeborn/templates/configmap.yaml	Render celeborn.master.internal.endpoints as key=value (no spaces) for consistent formatting.
charts/celeborn/tests/worker/priorityclass_test.yaml	Assert metadata.namespace does not exist for worker PriorityClass.
charts/celeborn/tests/master/priorityclass_test.yaml	Assert metadata.namespace does not exist for master PriorityClass.
charts/celeborn/tests/worker/statefulset_test.yaml	Update expected conf-hash annotation values due to config rendering changes.
charts/celeborn/tests/master/statefulset_test.yaml	Update expected conf-hash annotation values due to config rendering changes.
charts/celeborn/tests/configmap_test.yaml	Tighten regex expectations for endpoints lines and add consistency checks for key=value spacing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[zemin-piao]

@zemin-piao, thanks for contribution. Could you please create jira ticket which refers to #1053 for this issue?

done!

[SteNicholas]

Thanks for the cleanup! The substantive changes all look correct and behavior-preserving:

• PodMonitor and merge — and evaluates both operands, but both are always safe to evaluate, and the result is logically identical to the nested if for every value of podMonitor.enable. Whitespace chomping is unchanged, so the rendered output is byte-identical.
• namespace removed from PriorityClass — PriorityClass is cluster-scoped, so the API server already silently ignored that field; this is a correct cleanup with no functional regression.
• configmap = spacing — Celeborn parses config via Properties.load, which trims the separator and leading value whitespace, so endpoints = X and endpoints=X parse identically. The conf-hash change is consistent, and both master/worker statefulset tests share one hash since they hash the same configmap.yaml.

One incomplete-cleanup item worth addressing before merge:

charts/celeborn/templates/worker/podmonitor.yaml was left on the old pattern. This PR collapses the double-nested

{{- if .Values.podMonitor.enable }}
{{- if .Capabilities.APIVersions.Has "monitoring.coreos.com/v1/PodMonitor" -}}
...
{{- end }}
{{- end }}

into a single {{- if and ... -}} / {{- end }} in master/podmonitor.yaml, but worker/podmonitor.yaml still uses the old nested form. The two templates were intentionally identical; applying the same change there keeps them consistent and fully delivers the stated "podmonitor flags" cleanup.

No correctness bugs found. (I couldn't byte-verify the new conf-hash literals locally since helm isn't installed, but they're structurally correct and CI will confirm.)

[SteNicholas]

Minor: this celeborn.master.internal.endpoints= assert duplicates the (tightened) assert in the preceding test, so the internal.endpoints coverage now lives in two places. The genuinely new bit is the celeborn.master.endpoints= assert above.

Optional: since the PR's goal is enforcing no-space =, a negative guard like notMatchRegex on a spaced-separator pattern would actually protect all keys from regressing, whereas the positive matchRegex only checks these two specific keys.