versionPrecedes / HasMOFeature version gate has fragile semantics and high maintenance cost

[loveRhythm1990]

Summary

The MO feature gate in api/core/v1alpha1/semver.go (HasMOFeature + versionPrecedes) uses custom version-matching rules that are easy to misconfigure and have already caused production-impacting bugs (see #597). This issue tracks the design risks and possible long-term improvements — no code change is proposed here yet.

Related: #596, #597

Current design

func versionPrecedes(baseVersion semver.Version, current semver.Version) bool {
    if baseVersion.Major != current.Major {
        return false  // major-version isolation
    }
    if baseVersion.Patch == 0 && current.Minor >= baseVersion.Minor {
        return true
    }
    return baseVersion.Minor == current.Minor && current.Patch >= baseVersion.Patch
}

featureVersions stores one or more minimum versions per major branch. HasMOFeature returns true if any entry matches via versionPrecedes.

A partial mitigation was added for stable cross-major features (featureGlobalMinVersions + versionPrecedesCrossMajor), currently only used by MOFeatureDiscoveryFixed.

Observed incident (#597)

MOFeatureDiscoveryFixed had entries {2.0.0} but not {3.0.0}. Because versionPrecedes(2.0.0, 3.0.16) returns false (different major), all MO 3.x clusters incorrectly fell back to legacy service-addresses instead of discovery-address.

This was silent — no compile error, no runtime error, just wrong ConfigMap content.

Risk analysis

1. Major-version isolation requires manual entry per major

Every new MO major branch (3.x → 4.x → 5.x) needs an explicit entry in featureVersions for each feature that should remain enabled. Forgetting one entry silently disables the feature for the entire major branch.

Impact: High — already caused #597.

2. Non-standard semver semantics

versionPrecedes is not current >= base:

base	current	Standard semver	versionPrecedes
1.2.0	1.3.0	true	true
1.2.3	1.3.0	true	false
2.0.0	3.0.16	true	false

When base.patch != 0, only the same minor line matches. This matches MO's release-branch model but is counter-intuitive and easy to misconfigure when adding entries.

3. Pre-release / RC versions treated as stable

Tests expect RC/alpha tags to enable features:

HasMOFeature("v1.1.2-rc1", PipelineInfo)      → true
HasMOFeature("v1.2.0-alpha.1", LockMigration) → true

ParseTolerant strips pre-release suffixes. If a feature is not actually available in RC/alpha builds, operator may enable it prematurely.

4. No upper bound within a major branch

When base is X.Y.0, any X.Z.* where Z >= Y matches (e.g. base 1.2.0 → 1.99.99 is true). If a feature is removed in a later minor within the same major, the gate cannot express that.

5. Misleading naming

Comment says "strict preceding version" but semantics are "at or after within branch" (>=). The function name suggests ordering in the opposite direction.

6. OR semantics across multiple entries

MOFeaturePipelineInfo: {1.1.2, 1.2.0, 2.0.0}

Any single matching entry enables the feature. Missing one branch entry (e.g. forgetting 1.2.0) can silently drop support for that release line.

Features currently gated

Feature	Map	Call sites
MOFeatureDiscoveryFixed	featureGlobalMinVersions (cross-major)	logset/dnset/cnset ConfigMap, start script
MOFeatureLockMigration	featureVersions (per-major)	cnstore
MOFeaturePipelineInfo	featureVersions	cnstore
MOFeatureShardingMigration	featureVersions	cnstore
MOFeatureSessionSource	featureVersions	cnstore

Only DiscoveryFixed has been migrated to cross-major global min. The other four still require per-major verification and manual entry when MO 4.x ships.

Possible directions (for discussion)

1. Keep current model + tests — rely on regression tests like TestHasMOFeature_OtherFeaturesNotExtendedTo3x to prevent bulk misconfiguration. Low effort, risks remain.

2. Migrate verified-stable features to featureGlobalMinVersions — for features that are stable config/protocol (like discovery-address), avoid per-major entries. Already done for DiscoveryFixed.

3. Replace versionPrecedes with standard current.GTE(base) per major entry — simpler mental model; may need to revisit whether patch==0 special rule is still required for MO release branches.

4. Add a CI check / linter — when a new MO major tag appears in the repo or release notes, warn if featureVersions entries are incomplete.

5. Document the release checklist — "when MO N.0 ships, verify and update featureVersions for: LockMigration, PipelineInfo, ..."

Ask

This issue is for tracking and design discussion. Before changing behavior:

• Confirm which features are safe to treat as cross-major stable vs. per-major verified
• Decide whether versionPrecedes patch==0 rule is still needed for MO release branches
• Evaluate pre-release version handling (should RC/alpha disable feature gates?)

/cc @loveRhythm1990