diff --git a/pkg/admin/prerun/featuregate_lock.go b/pkg/admin/prerun/featuregate_lock.go index 641c7671f2..40e84bd995 100644 --- a/pkg/admin/prerun/featuregate_lock.go +++ b/pkg/admin/prerun/featuregate_lock.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "reflect" "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util" @@ -16,6 +15,8 @@ import ( var ( featureGateLockFilePath = filepath.Join(config.DataDir, "no-upgrade") errLockFileDoesNotExist = errors.New("feature gate lock file does not exist") + // getExecutableVersion is a function variable that allows tests to override the version + getExecutableVersion = GetVersionOfExecutable ) // featureGateLockFile represents the structure of the lock file @@ -45,58 +46,27 @@ func featureGateLockManagement(cfg *config.Config) error { if err != nil { return fmt.Errorf("failed to check if lock file exists: %w", err) } - // Lock file exists - validate configuration if lockExists { - return validateFeatureGateLockFile(cfg) + return runValidationsChecks(cfg) } - // No lock file exists yet and custom feature gates are configured, so this is the first time configuring custom feature gates - if hasCustomFeatureGates(cfg.ApiServer.FeatureGates) { - klog.InfoS("Custom feature gates detected", "featureSet", cfg.ApiServer.FeatureGates.FeatureSet) + if cfg.ApiServer.FeatureGates.FeatureSet != "" { return createFeatureGateLockFile(cfg) } - // No lock file and no custom feature gates - normal operation - klog.InfoS("No custom feature gates configured - skipping lock file management") return nil } -// hasCustomFeatureGates checks if any custom feature gates are configured -func hasCustomFeatureGates(fg config.FeatureGates) bool { - // Empty feature set means no custom feature gates - if fg.FeatureSet == "" { - return false - } - - // TechPreviewNoUpgrade and DevPreviewNoUpgrade are considered custom - if fg.FeatureSet == config.FeatureSetTechPreviewNoUpgrade || - fg.FeatureSet == config.FeatureSetDevPreviewNoUpgrade { - return true - } - - // CustomNoUpgrade requires actual enabled or disabled features - if fg.FeatureSet == config.FeatureSetCustomNoUpgrade { - return len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0 - } - - return false -} - // createFeatureGateLockFile creates the lock file with current configuration func createFeatureGateLockFile(cfg *config.Config) error { klog.InfoS("Creating feature gate lock file - this cluster can no longer be upgraded", "path", featureGateLockFilePath) - // Get current version from version file - currentVersion, err := getVersionOfData() + // Get current version from executable + currentVersion, err := getExecutableVersion() if err != nil { - // If version file doesn't exist yet, get executable version - klog.InfoS("Version file does not exist yet, using executable version") - currentVersion, err = GetVersionOfExecutable() - if err != nil { - return fmt.Errorf("failed to get version: %w", err) - } + return fmt.Errorf("failed to get version: %w", err) } lockFile := featureGateLockFile{ @@ -116,9 +86,9 @@ func createFeatureGateLockFile(cfg *config.Config) error { return nil } -// validateFeatureGateLockFile validates that the current configuration matches the lock file -// and that no version upgrade has occurred -func validateFeatureGateLockFile(cfg *config.Config) error { +// runValidationsChecks validates the feature gate lock file and the current configuration +// It returns an error if the configuration is invalid or if an x or y stream version upgrade has occurred. +func runValidationsChecks(cfg *config.Config) error { klog.InfoS("Validating feature gate lock file", "path", featureGateLockFilePath) lockFile, err := readFeatureGateLockFile(featureGateLockFilePath) @@ -127,9 +97,8 @@ func validateFeatureGateLockFile(cfg *config.Config) error { } // Check if feature gate configuration has changed - if err := compareFeatureGates(lockFile, cfg.ApiServer.FeatureGates); err != nil { - return fmt.Errorf("feature gate configuration has changed: %w\n\n"+ - "Custom feature gates cannot be modified or reverted once applied.\n"+ + if err := configValidationChecksPass(lockFile, cfg.ApiServer.FeatureGates); err != nil { + return fmt.Errorf("detected invalid changes in feature gate configuration: %w\n\n"+ "To restore MicroShift to a supported state, you must:\n"+ "1. Run: sudo microshift-cleanup-data --all\n"+ "2. Remove custom feature gates from /etc/microshift/config.yaml\n"+ @@ -137,12 +106,12 @@ func validateFeatureGateLockFile(cfg *config.Config) error { } // Check if version has changed (upgrade attempted) - currentVersion, err := getVersionOfData() + currentExecutableVersion, err := getExecutableVersion() if err != nil { - return fmt.Errorf("failed to get current version: %w", err) + return fmt.Errorf("failed to get current executable version: %w", err) } - if lockFile.Version.Major != currentVersion.Major || lockFile.Version.Minor != currentVersion.Minor { + if lockFile.Version.Major != currentExecutableVersion.Major || lockFile.Version.Minor != currentExecutableVersion.Minor { 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"+ @@ -151,7 +120,7 @@ func validateFeatureGateLockFile(cfg *config.Config) error { "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.Version.String(), currentExecutableVersion.String(), lockFile.FeatureSet, lockFile.Version.String(), lockFile.Version.String()) } @@ -159,21 +128,16 @@ func validateFeatureGateLockFile(cfg *config.Config) error { return nil } -// compareFeatureGates compares the lock file with current configuration -func compareFeatureGates(lockFile featureGateLockFile, current config.FeatureGates) error { - var errs []error - - if lockFile.FeatureSet != current.FeatureSet { - errs = append(errs, fmt.Errorf("feature set changed: locked config has %q, current config has %q", - lockFile.FeatureSet, current.FeatureSet)) +func configValidationChecksPass(prev featureGateLockFile, current config.FeatureGates) error { + if prev.FeatureSet != "" && current.FeatureSet == "" { + // Disallow changing from feature set to no feature set + return fmt.Errorf("cannot unset feature set. Previous config had feature set %q, current config has no feature set configured", prev.FeatureSet) } - - if !reflect.DeepEqual(lockFile.CustomNoUpgrade, current.CustomNoUpgrade) { - errs = append(errs, fmt.Errorf("custom feature gates changed: locked config has %#v, current config has %#v", - lockFile.CustomNoUpgrade, current.CustomNoUpgrade)) + if prev.FeatureSet == config.FeatureSetCustomNoUpgrade && current.FeatureSet != config.FeatureSetCustomNoUpgrade { + // Disallow changing from custom feature gates to any other feature set + return fmt.Errorf("cannot change CustomNoUpgrade feature set. Previous feature set was %q, current feature set is %q", prev.FeatureSet, current.FeatureSet) } - - return errors.Join(errs...) + return nil } // writeFeatureGateLockFile writes the lock file to disk in YAML format diff --git a/pkg/admin/prerun/featuregate_lock_test.go b/pkg/admin/prerun/featuregate_lock_test.go index 163a1ae177..1a9a243dd9 100644 --- a/pkg/admin/prerun/featuregate_lock_test.go +++ b/pkg/admin/prerun/featuregate_lock_test.go @@ -1,7 +1,6 @@ package prerun import ( - "encoding/json" "errors" "os" "path/filepath" @@ -125,9 +124,15 @@ func TestIsCustomFeatureGatesConfigured(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := hasCustomFeatureGates(tt.fg) - if got != tt.want { - t.Errorf("isCustomFeatureGatesConfigured() = %v, want %v", got, tt.want) + err := configValidationChecksPass(featureGateLockFile{ + FeatureSet: tt.fg.FeatureSet, + CustomNoUpgrade: tt.fg.CustomNoUpgrade, + }, tt.fg) + if err != nil { + t.Errorf("featureValidationsPass() error = %v", err) + } + if err != nil { + t.Errorf("featureValidationsPass() error = %v", err) } }) } @@ -206,81 +211,49 @@ func TestFeatureGateLockFile_ReadNonExistent(t *testing.T) { } } -func TestCompareFeatureGates(t *testing.T) { +func TestConfigValidationChecksPass(t *testing.T) { tests := []struct { - name string - lockFile featureGateLockFile - current config.FeatureGates - wantMatch bool + name string + lockFile featureGateLockFile + current config.FeatureGates + wantErr bool }{ { - name: "identical custom feature gates", + name: "unset any feature set", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA", "FeatureB"}, - Disabled: []string{"FeatureC"}, - }, }, current: config.FeatureGates{ - FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA", "FeatureB"}, - Disabled: []string{"FeatureC"}, - }, + FeatureSet: "", }, - wantMatch: true, + wantErr: true, }, { - name: "different enabled features", + name: "change CustomNoUpgrade to any other feature set", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA"}, - }, - }, - current: config.FeatureGates{ - FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureB"}, - }, - }, - wantMatch: false, - }, - { - name: "different feature sets", - lockFile: featureGateLockFile{ - FeatureSet: config.FeatureSetTechPreviewNoUpgrade, - }, - current: config.FeatureGates{ - FeatureSet: config.FeatureSetDevPreviewNoUpgrade, - }, - wantMatch: false, - }, - { - name: "identical TechPreviewNoUpgrade", - lockFile: featureGateLockFile{ - FeatureSet: config.FeatureSetTechPreviewNoUpgrade, }, current: config.FeatureGates{ FeatureSet: config.FeatureSetTechPreviewNoUpgrade, }, - wantMatch: true, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := compareFeatureGates(tt.lockFile, tt.current) - gotMatch := err == nil - if gotMatch != tt.wantMatch { - t.Errorf("compareFeatureGates() match = %v, want %v, error = %v", gotMatch, tt.wantMatch, err) + err := configValidationChecksPass(tt.lockFile, tt.current) + if (err != nil) != tt.wantErr { + t.Errorf("configValidationChecksPass() error = %v, wantErr %v", err, tt.wantErr) } }) } } func TestFeatureGateLockManagement_FirstRun(t *testing.T) { + // Use a fixed test version (doesn't depend on ldflags) + testVersion := versionMetadata{Major: 4, Minor: 21, Patch: 0} + // Create a temporary directory for testing tmpDir, err := os.MkdirTemp("", "featuregate-lockFile-test-*") if err != nil { @@ -293,20 +266,12 @@ func TestFeatureGateLockManagement_FirstRun(t *testing.T) { 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 to simulate existing data (version file uses JSON format) - versionData := versionFile{ - Version: versionMetadata{Major: 4, Minor: 18, Patch: 0}, - BootID: "test-boot", - } - versionJSON, _ := json.Marshal(versionData) - if err := os.WriteFile(versionFilePath, versionJSON, 0600); err != nil { - t.Fatal(err) + // Override getExecutableVersion for testing + originalGetExecutableVersion := getExecutableVersion + getExecutableVersion = func() (versionMetadata, error) { + return testVersion, nil } + defer func() { getExecutableVersion = originalGetExecutableVersion }() cfg := &config.Config{ ApiServer: config.ApiServer{ @@ -336,6 +301,9 @@ func TestFeatureGateLockManagement_FirstRun(t *testing.T) { } func TestFeatureGateLockManagement_ConfigChange(t *testing.T) { + // Use a fixed test version (doesn't depend on ldflags) + testVersion := versionMetadata{Major: 4, Minor: 21, Patch: 0} + // Create a temporary directory for testing tmpDir, err := os.MkdirTemp("", "featuregate-lockFile-test-*") if err != nil { @@ -348,51 +316,54 @@ func TestFeatureGateLockManagement_ConfigChange(t *testing.T) { 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 to simulate existing data (version file uses JSON format) - versionData := versionFile{ - Version: versionMetadata{Major: 4, Minor: 18, Patch: 0}, - BootID: "test-boot", - } - versionJSON, _ := json.Marshal(versionData) - if err := os.WriteFile(versionFilePath, versionJSON, 0600); err != nil { - t.Fatal(err) + // Override getExecutableVersion for testing + originalGetExecutableVersion := getExecutableVersion + getExecutableVersion = func() (versionMetadata, error) { + return testVersion, nil } + defer func() { getExecutableVersion = originalGetExecutableVersion }() - // Create lockFile file with initial config + // Create lockFile file with initial config (CustomNoUpgrade feature set) initialLock := featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, CustomNoUpgrade: config.CustomNoUpgrade{ Enabled: []string{"FeatureA"}, }, + Version: testVersion, } if err := writeFeatureGateLockFile(featureGateLockFilePath, initialLock); err != nil { t.Fatal(err) } - // Try to run with different config - should fail + // Try to run with no feature gates configured - should fail + // (configValidationChecksPass blocks unsetting a feature set) cfg := &config.Config{ ApiServer: config.ApiServer{ FeatureGates: config.FeatureGates{ - FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureB"}, // Different feature - }, + FeatureSet: "", // Trying to unset feature gates }, }, } err = FeatureGateLockManagement(cfg) if err == nil { - t.Error("FeatureGateLockManagement() should have failed with config change") + t.Error("FeatureGateLockManagement() should have failed when trying to unset feature gates") } } func TestFeatureGateLockManagement_VersionChange(t *testing.T) { + // Use a fixed base version for testing (doesn't depend on ldflags) + baseVersion := versionMetadata{Major: 4, Minor: 21, Patch: 0} + + // getVersion creates a version with offsets from the base version + getVersion := func(majorOffset, minorOffset, patchOffset int) versionMetadata { + return versionMetadata{ + Major: baseVersion.Major + majorOffset, + Minor: baseVersion.Minor + minorOffset, + Patch: baseVersion.Patch + patchOffset, + } + } + tests := []struct { name string lockFileVer versionMetadata @@ -402,43 +373,43 @@ func TestFeatureGateLockManagement_VersionChange(t *testing.T) { }{ { name: "minor version upgrade should fail", - lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 22, Patch: 0}, + lockFileVer: getVersion(0, 0, 0), + currentVer: getVersion(0, 1, 0), wantErr: true, description: "Minor version upgrade (4.21.0 -> 4.22.0) should be blocked", }, { name: "major version upgrade should fail", - lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, - currentVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, + lockFileVer: getVersion(0, 0, 0), + currentVer: getVersion(1, 0, 0), wantErr: true, description: "Major version upgrade (4.21.0 -> 5.0.0) should be blocked", }, { name: "patch version change should succeed", - lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 1}, + lockFileVer: getVersion(0, 0, 0), + currentVer: getVersion(0, 0, 1), wantErr: false, description: "Patch version change (4.21.0 -> 4.21.1) should be allowed", }, { name: "same version should succeed", - lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, + lockFileVer: getVersion(0, 0, 0), + currentVer: getVersion(0, 0, 0), wantErr: false, description: "Same version (4.21.0 -> 4.21.0) should be allowed", }, { name: "minor version downgrade should fail", - lockFileVer: versionMetadata{Major: 4, Minor: 22, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, + lockFileVer: getVersion(0, 1, 0), + currentVer: getVersion(0, 0, 0), wantErr: true, description: "Minor version downgrade (4.22.0 -> 4.21.0) should be blocked", }, { name: "major version downgrade should fail", - lockFileVer: versionMetadata{Major: 5, Minor: 0, Patch: 0}, - currentVer: versionMetadata{Major: 4, Minor: 21, Patch: 0}, + lockFileVer: getVersion(1, -21, 0), + currentVer: getVersion(0, 0, 0), wantErr: true, description: "Major version downgrade (5.0.0 -> 4.21.0) should be blocked", }, @@ -458,20 +429,12 @@ func TestFeatureGateLockManagement_VersionChange(t *testing.T) { 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) + // Override getExecutableVersion for testing + originalGetExecutableVersion := getExecutableVersion + getExecutableVersion = func() (versionMetadata, error) { + return tt.currentVer, nil } + defer func() { getExecutableVersion = originalGetExecutableVersion }() // Create lockFile file with locked version lockFile := featureGateLockFile{ diff --git a/pkg/config/apiserver.go b/pkg/config/apiserver.go index 4ed96c2166..076b21e816 100644 --- a/pkg/config/apiserver.go +++ b/pkg/config/apiserver.go @@ -157,12 +157,14 @@ type FeatureGates struct { // Validation checks should be performed before calling this function to ensure the FeatureGates struct is valid. func (fg FeatureGates) ToApiserverArgs() ([]string, error) { ret := sets.NewString() - for _, feature := range fg.CustomNoUpgrade.Enabled { - ret.Insert(fmt.Sprintf("%s=true", feature)) - } - for _, feature := range fg.CustomNoUpgrade.Disabled { - ret.Insert(fmt.Sprintf("%s=false", feature)) + addFeatures := func(features []string, enabled bool) { + for _, feature := range features { + ret.Insert(fmt.Sprintf("%s=%t", feature, enabled)) + } } + + addFeatures(fg.CustomNoUpgrade.Enabled, true) + addFeatures(fg.CustomNoUpgrade.Disabled, false) return ret.List(), nil } @@ -171,49 +173,53 @@ func (fg FeatureGates) GoString() string { return fmt.Sprintf("FeatureGates{FeatureSet: %q, CustomNoUpgrade: %#v}", fg.FeatureSet, fg.CustomNoUpgrade) } +// checkFeatureGateConflict checks if two sets of feature gates have any intersection and returns an error if they do. +func checkFeatureGateConflict(a, b sets.Set[string], errorMsg string) error { + if intersect := a.Intersection(b); intersect.Len() > 0 { + return fmt.Errorf("%s: %s", errorMsg, intersect.UnsortedList()) + } + return nil +} + +// validateFeatureGates validates the FeatureGates struct according to the following rules: +// 1. FeatureGates may be unset. +// 2. FeatureSet must be empty or CustomNoUpgrade. +// 3. If FeatureSet is DevPreviewNoUpgrade or TechPreviewNoUpgrade, return an error. +// 4. If FeatureSet is CustomNoUpgrade, CustomNoUpgrade.Enabled/Disabled lists may be set but are not required. +// 5. Required feature gates cannot be disabled. +// 6. Feature gates cannot be both enabled and disabled within the same object. func (fg *FeatureGates) validateFeatureGates() error { - // FG is unset if fg == nil || reflect.DeepEqual(*fg, FeatureGates{}) { return nil } - // FeatureSet must be empty or CustomNoUpgrade. If empty, CustomNoUpgrade.Enabled/Disabled lists must be empty. switch fg.FeatureSet { case "": - if len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0 { - return fmt.Errorf("CustomNoUpgrade enabled/disabled lists must be empty when FeatureSet is empty") - } return nil case FeatureSetCustomNoUpgrade: - // Valid - continue to validate enabled/disabled lists below + // Valid - continue with validation case FeatureSetDevPreviewNoUpgrade, FeatureSetTechPreviewNoUpgrade: return fmt.Errorf("FeatureSet %s is not supported. Use CustomNoUpgrade to enable/disable feature gates", fg.FeatureSet) default: return fmt.Errorf("invalid feature set: %s", fg.FeatureSet) } - var errs = make(sets.Set[error], 0) - for _, requiredFG := range RequiredFeatureGates { - // Edge case: Users must not be allowed to explicitly disable required feature gates. - if sets.NewString(fg.CustomNoUpgrade.Disabled...).Has(requiredFG) { - errs.Insert(fmt.Errorf("required feature gate %s cannot be disabled: %s", requiredFG, fg.CustomNoUpgrade.Disabled)) - } - // Edge case: Users must not be allowed to explicitly enable required feature gates or else the config would be locked and the cluster - // would not be able to be upgraded. - if sets.New(fg.CustomNoUpgrade.Enabled...).Has(requiredFG) { - errs.Insert(fmt.Errorf("feature gate %s is explicitly enabled and cannot be enabled by the user", requiredFG)) - } - } - if errs.Len() > 0 { - return fmt.Errorf("invalid feature gates: %s", errs.UnsortedList()) + enabledCustom := sets.New(fg.CustomNoUpgrade.Enabled...) + disabledCustom := sets.New(fg.CustomNoUpgrade.Disabled...) + + conflictChecks := []struct { + setA sets.Set[string] + setB sets.Set[string] + msg string + }{ + {disabledCustom, sets.New(RequiredFeatureGates...), "required feature gates cannot be disabled"}, + {enabledCustom, disabledCustom, "feature gates cannot be both enabled and disabled"}, } - // Must not have any feature gates that are enabled and disabled at the same time - enabledSet := sets.New(fg.CustomNoUpgrade.Enabled...) - disabledSet := sets.New(fg.CustomNoUpgrade.Disabled...) - inBothSets := enabledSet.Intersection(disabledSet) - if inBothSets.Len() > 0 { - return fmt.Errorf("featuregates cannot be enabled and disabled at the same time: %s", inBothSets.UnsortedList()) + for _, check := range conflictChecks { + if err := checkFeatureGateConflict(check.setA, check.setB, check.msg); err != nil { + return err + } } return nil diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 497fe49953..e2460a8002 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -819,10 +819,10 @@ func TestValidate(t *testing.T) { expectErr: true, }, { - name: "feature-gates-custom-no-upgrade-valid", + name: "feature-gates-custom-no-upgrade-with-feature-set-empty", config: func() *Config { c := mkDefaultConfig() - c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" + c.ApiServer.FeatureGates.FeatureSet = "" c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} return c @@ -830,45 +830,45 @@ func TestValidate(t *testing.T) { expectErr: false, }, { - name: "feature-gates-custom-no-upgrade-with-feature-set-empty", + name: "feature-gates-custom-no-upgrade-valid", config: func() *Config { c := mkDefaultConfig() - c.ApiServer.FeatureGates.FeatureSet = "" + c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} return c }(), - expectErr: true, + expectErr: false, }, { - name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate", + name: "feature-gates-required-feature-gate-cannot-be-explicitly-enabled", config: func() *Config { c := mkDefaultConfig() c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" - c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} - c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature1"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"UserNamespacesSupport"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} return c }(), - expectErr: true, + expectErr: false, }, { - name: "feature-gates-required-feature-gate-cannot-be-disabled", + name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate", config: func() *Config { c := mkDefaultConfig() c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} - c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"UserNamespacesSupport"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature1"} return c }(), expectErr: true, }, { - name: "feature-gates-required-feature-gate-cannot-be-explicitly-enabled", + name: "feature-gates-required-feature-gate-cannot-be-disabled", config: func() *Config { c := mkDefaultConfig() c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" - c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"UserNamespacesSupport"} - c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"UserNamespacesSupport"} return c }(), expectErr: true, diff --git a/test/suites/standard2/feature-gates.robot b/test/suites/standard2/feature-gates.robot index c4744a20d8..9aeea29e7a 100644 --- a/test/suites/standard2/feature-gates.robot +++ b/test/suites/standard2/feature-gates.robot @@ -1,5 +1,6 @@ *** Settings *** -Documentation Tests for feature gate configuration +Documentation Verify that that MicroShift feature gate configuration is correctly integrated into the kube-apiserver. +... Validation testing is handled by prerun unit tests and is not included here. Resource ../../resources/common.resource Resource ../../resources/microshift-config.resource @@ -23,13 +24,6 @@ ${CUSTOM_FEATURE_GATES} SEPARATOR=\n ... \ \ \ \ \ \ \ \ - TestFeatureEnabled ... \ \ \ \ \ \ disabled: ... \ \ \ \ \ \ \ \ - TestFeatureDisabled -${DIFFERENT_FEATURE_GATES} SEPARATOR=\n -... apiServer: -... \ \ featureGates: -... \ \ \ \ featureSet: CustomNoUpgrade -... \ \ \ \ customNoUpgrade: -... \ \ \ \ \ \ enabled: -... \ \ \ \ \ \ \ \ - DifferentTestFeature ${FEATURE_GATE_LOCK_FILE} /var/lib/microshift/no-upgrade @@ -50,20 +44,6 @@ Custom Feature Gates Are Passed To Kube APIServer Feature Gate Lock File Should Contain Feature Gates CustomNoUpgrade TestFeatureEnabled [Teardown] Teardown Custom Feature Gates Test -Feature Gate Config Change Blocked After Lock Created - [Documentation] Verify that changing feature gate config is blocked after lock file exists. - ... MicroShift must refuse to start if feature gates change after CustomNoUpgrade is set. - [Setup] Setup Custom Feature Gates Test - Stop MicroShift - Drop In MicroShift Config ${DIFFERENT_FEATURE_GATES} 10-featuregates - Save Journal Cursor - MicroShift Should Fail To Start - Pattern Should Appear In Log Output ${CURSOR} feature gate configuration has changed - # Restore original config and verify that MicroShift starts - Drop In MicroShift Config ${CUSTOM_FEATURE_GATES} 10-featuregates - Start MicroShift - [Teardown] Teardown Custom Feature Gates Test - Feature Gate Lock File Persists Across Restarts With Same Config [Documentation] Verify that feature gate lock file persists and validation succeeds across restarts ... when the same feature gate configuration is maintained. @@ -123,13 +103,3 @@ Feature Gate Lock File Should Contain Feature Gates ${contents}= Command Should Work cat ${FEATURE_GATE_LOCK_FILE} Should Contain ${contents} ${feature_set} Should Contain ${contents} ${feature_name} - -MicroShift Should Fail To Start - [Documentation] Verify that MicroShift fails to start and returns a non-zero exit code. - ... This keyword is unique and differs from a composite keyword like - ... Run Keyword And Expect Error 1 != 0 Start MicroShift - ... because there is no need to poll the service for an "active" state, which Start MicroShift does. - ${stdout} ${stderr} ${rc}= Execute Command sudo systemctl start microshift.service - ... sudo=True return_stdout=True return_stderr=True return_rc=True - Log Many ${stdout} ${stderr} ${rc} - Should Be Equal As Integers 1 ${rc}