From 95e50978515e6c9e24a0026c36f33822081d2bf5 Mon Sep 17 00:00:00 2001 From: Jon Cope Date: Wed, 14 Jan 2026 15:51:52 -0600 Subject: [PATCH 1/3] align featuregate upgrade blocking behavior by allowing only z-stream upgrades add unit test for upgrade validation --- pkg/admin/prerun/featuregate_lock.go | 13 +- pkg/admin/prerun/featuregate_lock_test.go | 145 +++++++++++++++------- 2 files changed, 106 insertions(+), 52 deletions(-) diff --git a/pkg/admin/prerun/featuregate_lock.go b/pkg/admin/prerun/featuregate_lock.go index 467660f711..f42e80285f 100644 --- a/pkg/admin/prerun/featuregate_lock.go +++ b/pkg/admin/prerun/featuregate_lock.go @@ -142,15 +142,14 @@ func validateFeatureGateLockFile(cfg *config.Config) error { return fmt.Errorf("failed to get current version: %w", err) } - if lockFile.Version != currentVersion { - return fmt.Errorf("version upgrade detected with custom feature gates: locked version %s, current version %s\n\n"+ - "Upgrades are not supported when custom feature gates are configured.\n"+ + if lockFile.Version.Major != currentVersion.Major || lockFile.Version.Minor != currentVersion.Minor { + return fmt.Errorf("major or minor version upgrade detected with custom feature gates: locked version %s, current version %s\n\n"+ + "Upgrades (major or minor) are not supported when custom feature gates are configured.\n"+ "Custom feature gates (%s) were configured in version %s.\n"+ "To restore MicroShift to a supported state, you must:\n"+ - "1. Roll back to version %s, OR\n"+ - "2. Run: sudo microshift-cleanup-data --all\n"+ - "3. Remove custom feature gates from /etc/microshift/config.yaml\n"+ - "4. Restart MicroShift: sudo systemctl restart microshift", + "- Roll back MicroShift to version %s, OR\n"+ + "- Run: sudo microshift-cleanup-data --all\n"+ + " and remove custom feature gates from /etc/microshift/config.yaml before restarting MicroShift.", lockFile.Version.String(), currentVersion.String(), lockFile.FeatureSet, lockFile.Version.String(), lockFile.Version.String()) } diff --git a/pkg/admin/prerun/featuregate_lock_test.go b/pkg/admin/prerun/featuregate_lock_test.go index 0c3db09eb8..92cff2385e 100644 --- a/pkg/admin/prerun/featuregate_lock_test.go +++ b/pkg/admin/prerun/featuregate_lock_test.go @@ -393,59 +393,114 @@ func TestFeatureGateLockManagement_ConfigChange(t *testing.T) { } func TestFeatureGateLockManagement_VersionChange(t *testing.T) { - // Create a temporary directory for testing - tmpDir, err := os.MkdirTemp("", "featuregate-lockFile-test-*") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpDir) - - // Override the lockFile file path for testing - originalPath := featureGateLockFilePath - featureGateLockFilePath = filepath.Join(tmpDir, "no-upgrade") - defer func() { featureGateLockFilePath = originalPath }() - - // Override version file path for testing - originalVersionPath := versionFilePath - versionFilePath = filepath.Join(tmpDir, "version") - defer func() { versionFilePath = originalVersionPath }() - - // Create a version file with NEW version (simulating upgrade) (version file uses JSON format) - versionData := versionFile{ - Version: versionMetadata{Major: 4, Minor: 19, Patch: 0}, // Newer version - BootID: "test-boot", - } - versionJSON, _ := json.Marshal(versionData) - if err := os.WriteFile(versionFilePath, versionJSON, 0600); err != nil { - t.Fatal(err) - } - - // Create lockFile file with OLD version - lockFile := featureGateLockFile{ - FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA"}, + tests := []struct { + name string + lockFileVer versionMetadata + currentVer versionMetadata + wantErr bool + description string + }{ + { + name: "minor version upgrade should fail", + lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 19, Patch: 0}, + wantErr: true, + description: "Minor version upgrade (4.18.0 -> 4.19.0) should be blocked", + }, + { + name: "major version upgrade should fail", + lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + currentVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, + wantErr: true, + description: "Major version upgrade (4.18.0 -> 5.0.0) should be blocked", + }, + { + name: "patch version change should succeed", + lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 1}, + wantErr: false, + description: "Patch version change (4.18.0 -> 4.18.1) should be allowed", + }, + { + name: "same version should succeed", + lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + wantErr: false, + description: "Same version (4.18.0 -> 4.18.0) should be allowed", + }, + { + name: "version downgrade minor should fail", + lockFileVer: versionMetadata{Major: 4, Minor: 19, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + wantErr: true, + description: "Minor version downgrade (4.19.0 -> 4.18.0) should be blocked", + }, + { + name: "version downgrade major should fail", + lockFileVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + wantErr: true, + description: "Major version downgrade (5.0.0 -> 4.18.0) should be blocked", }, - Version: versionMetadata{Major: 4, Minor: 18, Patch: 0}, // Older version - } - if err := writeFeatureGateLockFile(featureGateLockFilePath, lockFile); err != nil { - t.Fatal(err) } - cfg := &config.Config{ - ApiServer: config.ApiServer{ - FeatureGates: config.FeatureGates{ + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a temporary directory for testing + tmpDir, err := os.MkdirTemp("", "featuregate-lockFile-test-*") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Override the lockFile file path for testing + originalPath := featureGateLockFilePath + featureGateLockFilePath = filepath.Join(tmpDir, "no-upgrade") + defer func() { featureGateLockFilePath = originalPath }() + + // Override version file path for testing + originalVersionPath := versionFilePath + versionFilePath = filepath.Join(tmpDir, "version") + defer func() { versionFilePath = originalVersionPath }() + + // Create a version file with current version (version file uses JSON format) + versionData := versionFile{ + Version: tt.currentVer, + BootID: "test-boot", + } + versionJSON, _ := json.Marshal(versionData) + if err := os.WriteFile(versionFilePath, versionJSON, 0600); err != nil { + t.Fatal(err) + } + + // Create lockFile file with locked version + lockFile := featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, CustomNoUpgrade: config.CustomNoUpgrade{ Enabled: []string{"FeatureA"}, }, - }, - }, - } + Version: tt.lockFileVer, + } + if err := writeFeatureGateLockFile(featureGateLockFilePath, lockFile); err != nil { + t.Fatal(err) + } - err = FeatureGateLockManagement(cfg) - if err == nil { - t.Error("FeatureGateLockManagement() should have failed with version change") + cfg := &config.Config{ + ApiServer: config.ApiServer{ + FeatureGates: config.FeatureGates{ + FeatureSet: config.FeatureSetCustomNoUpgrade, + CustomNoUpgrade: config.CustomNoUpgrade{ + Enabled: []string{"FeatureA"}, + }, + }, + }, + } + + err = FeatureGateLockManagement(cfg) + if (err != nil) != tt.wantErr { + t.Errorf("FeatureGateLockManagement() error = %v, wantErr %v. %s", err, tt.wantErr, tt.description) + } + }) } } From 745510debc57585b742836b1a991a7fba881f677 Mon Sep 17 00:00:00 2001 From: Jon Cope Date: Fri, 16 Jan 2026 14:36:53 -0600 Subject: [PATCH 2/3] revert logged instructions; tests now use minor version 21 and up for clarity --- pkg/admin/prerun/featuregate_lock.go | 11 ++++---- pkg/admin/prerun/featuregate_lock_test.go | 32 +++++++++++------------ 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/admin/prerun/featuregate_lock.go b/pkg/admin/prerun/featuregate_lock.go index f42e80285f..641c7671f2 100644 --- a/pkg/admin/prerun/featuregate_lock.go +++ b/pkg/admin/prerun/featuregate_lock.go @@ -143,13 +143,14 @@ func validateFeatureGateLockFile(cfg *config.Config) error { } if lockFile.Version.Major != currentVersion.Major || lockFile.Version.Minor != currentVersion.Minor { - return fmt.Errorf("major or minor version upgrade detected with custom feature gates: locked version %s, current version %s\n\n"+ - "Upgrades (major or minor) are not supported when custom feature gates are configured.\n"+ + return fmt.Errorf("version upgrade detected with custom feature gates: locked version %s, current version %s\n\n"+ + "Upgrades are not supported when custom feature gates are configured.\n"+ "Custom feature gates (%s) were configured in version %s.\n"+ "To restore MicroShift to a supported state, you must:\n"+ - "- Roll back MicroShift to version %s, OR\n"+ - "- Run: sudo microshift-cleanup-data --all\n"+ - " and remove custom feature gates from /etc/microshift/config.yaml before restarting MicroShift.", + "1. Roll back to version %s, OR\n"+ + "2. Run: sudo microshift-cleanup-data --all\n"+ + "3. Remove custom feature gates from /etc/microshift/config.yaml\n"+ + "4. Restart MicroShift: sudo systemctl restart microshift", lockFile.Version.String(), currentVersion.String(), lockFile.FeatureSet, lockFile.Version.String(), lockFile.Version.String()) } diff --git a/pkg/admin/prerun/featuregate_lock_test.go b/pkg/admin/prerun/featuregate_lock_test.go index 92cff2385e..1be66954d5 100644 --- a/pkg/admin/prerun/featuregate_lock_test.go +++ b/pkg/admin/prerun/featuregate_lock_test.go @@ -402,45 +402,45 @@ func TestFeatureGateLockManagement_VersionChange(t *testing.T) { }{ { name: "minor version upgrade should fail", - lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 19, Patch: 0}, + lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 22, Patch: 0}, wantErr: true, - description: "Minor version upgrade (4.18.0 -> 4.19.0) should be blocked", + description: "Minor version upgrade (4.21.0 -> 4.22.0) should be blocked", }, { name: "major version upgrade should fail", - lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, currentVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, wantErr: true, - description: "Major version upgrade (4.18.0 -> 5.0.0) should be blocked", + description: "Major version upgrade (4.21.0 -> 5.0.0) should be blocked", }, { name: "patch version change should succeed", - lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 1}, + lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 1}, wantErr: false, - description: "Patch version change (4.18.0 -> 4.18.1) should be allowed", + description: "Patch version change (4.21.0 -> 4.21.1) should be allowed", }, { name: "same version should succeed", - lockFileVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, wantErr: false, - description: "Same version (4.18.0 -> 4.18.0) should be allowed", + description: "Same version (4.21.0 -> 4.21.0) should be allowed", }, { name: "version downgrade minor should fail", - lockFileVer: versionMetadata{Major: 4, Minor: 19, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + lockFileVer: versionMetadata{Major: 4, Minor: 22, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, wantErr: true, - description: "Minor version downgrade (4.19.0 -> 4.18.0) should be blocked", + description: "Minor version downgrade (4.22.0 -> 4.21.0) should be blocked", }, { name: "version downgrade major should fail", lockFileVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, wantErr: true, - description: "Major version downgrade (5.0.0 -> 4.18.0) should be blocked", + description: "Major version downgrade (5.0.0 -> 4.21.0) should be blocked", }, } From 3b01e540294d625cf96efb51e0c50da28c85a814 Mon Sep 17 00:00:00 2001 From: Jon Cope Date: Tue, 20 Jan 2026 15:01:46 -0600 Subject: [PATCH 3/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandro Gullón --- pkg/admin/prerun/featuregate_lock_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/admin/prerun/featuregate_lock_test.go b/pkg/admin/prerun/featuregate_lock_test.go index 1be66954d5..163a1ae177 100644 --- a/pkg/admin/prerun/featuregate_lock_test.go +++ b/pkg/admin/prerun/featuregate_lock_test.go @@ -429,14 +429,14 @@ func TestFeatureGateLockManagement_VersionChange(t *testing.T) { description: "Same version (4.21.0 -> 4.21.0) should be allowed", }, { - name: "version downgrade minor should fail", + name: "minor version downgrade should fail", lockFileVer: versionMetadata{Major: 4, Minor: 22, Patch: 0}, currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, wantErr: true, description: "Minor version downgrade (4.22.0 -> 4.21.0) should be blocked", }, { - name: "version downgrade major should fail", + name: "major version downgrade should fail", lockFileVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, wantErr: true,