refactor(mnnvl): replace two-annotation model with single mnnvl-group annotation#574
Conversation
… annotation
Remove grove.io/auto-mnnvl; use grove.io/mnnvl-group as the sole
annotation controlling MNNVL enrollment and group assignment.
- Remove AnnotationAutoMNNVL and related constants/helpers
- Add AnnotationMNNVLGroupOptOut ("none") for explicit opt-out
- Introduce groupStatus enum (enrolled/withdrawn/absent) for cleaner resolution
- Simplify webhook validation, PCSG propagation, and CD naming
- Extract generic ValidateAnnotationsImmutability to utils/kubernetes
- Use case-sensitive annotation matching per K8s convention
- Update all unit tests for single-annotation semantics
Co-authored-by: Cursor <cursoragent@cursor.com>
f52f0c5 to
8ff06a6
Compare
|
worth fixing: can we reject the legacy i buy the alpha-breaking-change argument, but silent acceptance is a bad failure mode: a user can submit an old manifest, think MNNVL is enabled, and just get worse performance with no obvious signal. even if we do not support backward compatibility, the webhook should probably fail fast with “use grove.io/mnnvl-group: default instead” so the migration is explicit. |
I thought about this, however, in phase 1, the legacy |
- Reduce Test_generateComputeDomainName from 5 to 2 distinct cases - Add TestSyncMultipleGroups and TestSyncMultipleGroupsScaleDown for multi-group CD reconciliation coverage - Allow mnnvl-group "none" when feature is disabled (harmless opt-out) - Add missing "none + disabled -> no error" webhook test case Co-authored-by: Cursor <cursoragent@cursor.com>
49dde61 to
4f7b760
Compare
/kind feature
Ref #417
Summary
grove.io/auto-mnnvlannotation — usegrove.io/mnnvl-groupas the sole annotation controlling MNNVL enrollment and group assignmentChanges
AnnotationAutoMNNVL,AnnotationAutoMNNVLEnabled,AnnotationAutoMNNVLDisabled; addAnnotationMNNVLGroupOptOut("none")groupStatusenum (groupEnrolled/groupWithdrawn/groupAbsent) replacing the two-boolean return fromresolveGroupName; simplifyResolveGroupNameHierarchically; use case-sensitive matching per K8s conventionauto-mnnvlvalidation and conflict detection; immutability check now only coversmnnvl-grouppropagateMNNVLAnnotationsonly propagatesmnnvl-group(PCS → PCSG)generateComputeDomainNamealways uses{pcs}-{replica}-{group}format (single scheme)ValidateAnnotationsImmutabilitytoutils/kubernetes/annotations.gowith full test coverageTest plan
go build ./...passesgo test ./...)Made with Cursor
Made with Cursor