Repository navigation
feat(charts): add optional priorityClassName to op-geth - #585
Merged
Merged
Conversation
op-geth had no way to set a PriorityClass on its pods, matching the gap just closed for op-node, op-reth and op-conductor in #584. erigon, prysm and nethermind already expose `priorityClassName`. Those charts render the field unconditionally: priorityClassName: {{ .Values.priorityClassName | quote }} Copying that verbatim would emit `priorityClassName: ""` into every pod template for every consumer of this chart, so the field is wrapped in `with` instead: unset renders nothing at all. Verified with `helm template` against ci/ct-values.yaml. With the value unset the rendered output is identical to the previous chart version apart from the `helm.sh/chart` label and the `checksum/configmap-scripts` pod annotation, which changes because configmap-scripts.yaml carries the chart labels — that annotation shifts on any op-geth version bump, not because of this change. With the value set the field renders as `priorityClassName: "high-priority"` in the pod spec. Chart version bumped 0.3.13 -> 0.3.14 so the release job does not push an existing tag. README is left untouched; the "[Automatic] - Update chart README.md" job regenerates it from README.md.gotmpl. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
priorityClassNamevalue to theop-gethchart, so its pods can be assigned a Kubernetes PriorityClass. Same change as #584, which coveredop-node,op-rethandop-conductor.erigon,prysmandnethermindalready expose this.Why it is conditional
The older charts render the field unconditionally:
Copying that verbatim would emit
priorityClassName: ""into the pod template of every consumer of this chart. The block is wrapped inwithinstead — unset renders nothing:Changes
op-gethserviceAccountName, beforesecurityContextvalues.yamlgains the following aftertolerations, matching erigon's comment style:Chart version is bumped so the release job does not try to push a tag that already exists.
Verification
helm templaterun againstcharts/op-geth/ci/ct-values.yaml, comparing rendered output before and after this change:helm.sh/chartlabel carrying the new version, and thechecksum/configmap-scriptspod annotation — see the note below.--set priorityClassName=high-priority): the only difference from the unset render ispriorityClassName: "high-priority"in the pod spec.helm lintpasses (0 failed).Note: one difference from #584
Unlike
op-nodeandop-reth, this chart's pod template carrieschecksum/configmap-scripts, andtemplates/configmap-scripts.yamlincludesop-geth.labels— which containshelm.sh/chart. So bumping the chart version changes that ConfigMap's sha256, changes the pod annotation, and existing op-geth pods will restart on their next sync.This is not caused by the
withguard — it happens on any op-geth chart version bump. With the chart version and that annotation normalised away, the rendered output before and after is byte-identical.Not included
charts/op-geth/README.md— left alone; the[Automatic] - Update chart README.mdjob regenerates it fromREADME.md.gotmpl.ci/ct-values.yaml— no edit needed; the empty default keeps chart-testing rendering unchanged.🤖 Generated with Claude Code