diff --git a/cli/kata-env.go b/cli/kata-env.go index 4a66b1921..8608bf97d 100644 --- a/cli/kata-env.go +++ b/cli/kata-env.go @@ -27,7 +27,7 @@ import ( // // XXX: Increment for every change to the output format // (meaning any change to the EnvInfo type). -const formatVersion = "1.0.20" +const formatVersion = "1.0.21" // MetaInfo stores information on the format of the output itself type MetaInfo struct { diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index 57b617f1b..3b114f5c4 100644 --- a/pkg/katautils/config.go +++ b/pkg/katautils/config.go @@ -858,11 +858,11 @@ func LoadConfiguration(configPath string, ignoreLogging, builtIn bool) (resolved config.DisableNewNetNs = tomlConf.Runtime.DisableNewNetNs for _, f := range tomlConf.Runtime.Experimental { - feature := exp.Feature(f) - if !exp.Supported(feature) { + feature := exp.Get(f) + if feature == nil { return "", config, fmt.Errorf("Unsupported experimental feature %q", f) } - config.Experimental = append(config.Experimental, feature) + config.Experimental = append(config.Experimental, *feature) } if err := checkConfig(config); err != nil { diff --git a/virtcontainers/experimental/README.md b/virtcontainers/experimental/README.md index 378b7e538..075eece90 100644 --- a/virtcontainers/experimental/README.md +++ b/virtcontainers/experimental/README.md @@ -8,7 +8,7 @@ They are **always disabled** by default in Kata components releases, and can only be enabled by users when they want to have a try. We suggest you **NEVER** enable "experimental" features in production environment, -unless you know what breakage they can bring and have confidence to handle it by youself. +unless you know what breakage they can bring and have confidence to handle it by yourself. Criteria of an experimental feature are: @@ -28,7 +28,13 @@ so it can improve in next few releases to be stable enough. Some features could be big, it adds/changes lots of codes so may need more tests. Our CI can help guarantee correctness of the feature, but it may not cover all scenarios. Before we're confident that the feature is ready for production use, -the feature can be marked as "experimental" first, and users can test it manually in their own environment if intested in it. +the feature can be marked as "experimental" first, and users can test it manually in their own environment if interested in it. + +We make no guarantees about experimental features, they can be removed entirely at any point, +or become non-experimental at some release, so relative configuration options can change radically. + +An experimental feature **MUST** have a descriptive name containing only lower-case characters, numbers or '_', +e.g. new_hypervisor_2, the name **MUST** be unique and will never be re-used in future. ## 2. What's the difference between "WIP" and "experimental"? @@ -42,7 +48,7 @@ In one word, "experimental" can be unstable currently but it **MUST** be complet That depends. -For the feature who breaks backward compatibility, we usually land it as formal feature in a major version bump(x in x.y.z, e.g. 2.0.0). +For the feature that breaks backward compatibility, we usually land it as formal feature in a major version bump(x in x.y.z, e.g. 2.0.0). But for a new feature who becomes stable and ready, we can release it formally in any minor version bump. Check Kata Container [versioning rules](https://github.com/kata-containers/documentation/blob/c556f1853f2e3df69d336de01ad4bb38e64ecc1b/Releases.md#versioning). diff --git a/virtcontainers/experimental/experimental.go b/virtcontainers/experimental/experimental.go index de208b71c..4204e963e 100644 --- a/virtcontainers/experimental/experimental.go +++ b/virtcontainers/experimental/experimental.go @@ -7,26 +7,57 @@ package experimental import ( "fmt" + "regexp" +) + +const ( + nameRegStr = "^[a-z][a-z0-9_]*$" ) // Feature to be experimental -type Feature string +type Feature struct { + Name string + Description string + // the expected release version to move out from experimental + ExpRelease string +} var ( - supportedFeatures = make(map[Feature]struct{}) + supportedFeatures = make(map[string]Feature) ) // Register register a new experimental feature func Register(feature Feature) error { - if _, ok := supportedFeatures[feature]; ok { - return fmt.Errorf("Feature %q had been registered before", feature) + if err := validateFeature(feature); err != nil { + return err } - supportedFeatures[feature] = struct{}{} + + if _, ok := supportedFeatures[feature.Name]; ok { + return fmt.Errorf("Feature %q had been registered before", feature.Name) + } + supportedFeatures[feature.Name] = feature return nil } -// Supported check if the feature is supported -func Supported(feature Feature) bool { - _, ok := supportedFeatures[feature] - return ok +// Get returns Feature with requested name +func Get(name string) *Feature { + if f, ok := supportedFeatures[name]; ok { + return &f + } + return nil +} + +func validateFeature(feature Feature) error { + if len(feature.Name) == 0 || + len(feature.Description) == 0 || + len(feature.ExpRelease) == 0 { + return fmt.Errorf("experimental feature must have valid name, description and expected release") + } + + reg := regexp.MustCompile(nameRegStr) + if !reg.MatchString(feature.Name) { + return fmt.Errorf("feature name must in the format %q", nameRegStr) + } + + return nil } diff --git a/virtcontainers/experimental/experimental_test.go b/virtcontainers/experimental/experimental_test.go index 1c24c0738..ef6c98813 100644 --- a/virtcontainers/experimental/experimental_test.go +++ b/virtcontainers/experimental/experimental_test.go @@ -12,15 +12,49 @@ import ( ) func TestExperimental(t *testing.T) { - f := Feature("mock") - assert.False(t, Supported(f)) + f := Feature{ + Name: "mock", + Description: "mock experimental feature for test", + ExpRelease: "2.0", + } + assert.Nil(t, Get(f.Name)) - err := Register("mock") + err := Register(f) assert.Nil(t, err) - err = Register("mock") + err = Register(f) assert.NotNil(t, err) assert.Equal(t, len(supportedFeatures), 1) - assert.True(t, Supported(f)) + assert.NotNil(t, Get(f.Name)) +} + +func TestValidateFeature(t *testing.T) { + f := Feature{} + assert.NotNil(t, validateFeature(f)) + + for _, names := range []struct { + name string + valid bool + }{ + {"mock_test_1", true}, + {"m1234ock_test_1", true}, + {"1_mock_test", false}, + {"_mock_test_1", false}, + {"Mock", false}, + {"mock*&", false}, + } { + f := Feature{ + Name: names.name, + Description: "test", + ExpRelease: "2.0", + } + + err := validateFeature(f) + if names.valid { + assert.Nil(t, err) + } else { + assert.NotNil(t, err) + } + } } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 851145fb7..d39c46cd8 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -142,7 +142,7 @@ func (sandboxConfig *SandboxConfig) valid() bool { // validate experimental features for _, f := range sandboxConfig.Experimental { - if !exp.Supported(f) { + if exp.Get(f.Name) == nil { return false } } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 45b6f7a23..2fbb7908f 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1683,16 +1683,20 @@ func TestSandboxUpdateResources(t *testing.T) { } func TestSandboxExperimentalFeature(t *testing.T) { - testFeature := exp.Feature("mock") + testFeature := exp.Feature{ + Name: "mock", + Description: "exp feature for test", + ExpRelease: "1.8.0", + } sconfig := SandboxConfig{ ID: testSandboxID, Experimental: []exp.Feature{testFeature}, } - assert.False(t, exp.Supported(testFeature)) + assert.Nil(t, exp.Get(testFeature.Name)) assert.False(t, sconfig.valid()) exp.Register(testFeature) - assert.True(t, exp.Supported(testFeature)) + assert.NotNil(t, exp.Get(testFeature.Name)) assert.True(t, sconfig.valid()) }