Repository navigation
feat!: Replace the two spec booleans with enums - #75
Merged
Merged
Conversation
storage.<role>.fileLogging and coreDumps.configureCorePattern were the only booleans in the spec, and both answer a question with more than two answers. Where the log files go already lost one, the separate log claim, and could grow another (an emptyDir). Who sets the node's core_pattern could be a per-node DaemonSet instead of a privileged init container in every pod. - storage.<role>.fileLogging (bool, default true) becomes storage.<role>.logFile: LibClaim | None, default LibClaim. - coreDumps.configureCorePattern (bool, default true) becomes coreDumps.corePattern: Operator | Node, default Operator. The pods are unchanged: the builders derive the same switches from the enums, treating anything but None and Node as the default so a spec that never passed admission behaves like an admitted one. Admission rejects a value it does not know. BREAKING CHANGE: the old fields are gone from the schema, so the API server prunes them, and a cluster that set either to false reads the new default once the CRD is upgraded: its pods roll onto file logging, or gain the privileged core pattern init container, which a restricted namespace rejects. Stop the operator, upgrade the CRD, set logFile: None or corePattern: Node on those clusters, then start the new operator.
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
1 participant
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.
Why
The API conventions say to "think twice about bool fields": many start as on/off and later need a third, mutually exclusive option. Our only two booleans both answer questions like that:
fileLogginganswers "where do the log files go". It has already lost one answer (the separate log claim, removed 2026-10-07) and could gain another (anemptyDir).configureCorePatternanswers "who sets the node'score_pattern". A plausible third answer is a per-node DaemonSet instead of a privileged init container in every pod.On
v1alpha1this is cheap to change now.What
storage.<role>.fileLogging: true(default) /falsestorage.<role>.logFile: LibClaim(default) /NonecoreDumps.configureCorePattern: true(default) /falsecoreDumps.corePattern: Operator(default) /NodeLogFileLocationandCorePatternOwner, plain string enums with schema defaults, likeretentionPolicy. TheDefaultLogFile/DefaultCorePatternconstants replace the bool defaults.None/Nodeas the default. A spec that never passed admission therefore behaves like an admitted one, and the generated pods are identical.docs/{flags,backup-restore,rolling-restart-pacing}.md, the e2e manifests and CLAUDE.md now use the new names. CLAUDE.md now says the spec has no bools, as a convention.version1.0.1-rc.2 → 1.0.1-rc.3 for the CRD change.Testing
logFile: LibClaimandcorePattern: Operator, the claim-pin and core dumps update specs use the enums, and new cases reject unknown values (EmptyDir,DaemonSet).LogFileNone/CorePatternNode.make test,make lint,make chart-verify,make helm-lint,make chart-version-check, andgo vet -tags e2e ./test/.... The e2e suite itself didn't run here; its manifests only change field names.