diff --git a/Makefile b/Makefile index d690b5598..0f802e3fb 100644 --- a/Makefile +++ b/Makefile @@ -157,6 +157,8 @@ DEFMEMSLOTS := 10 #Default number of bridges DEFBRIDGES := 1 DEFDISABLEGUESTSECCOMP := true +#Default experimental features enabled +DEFAULTEXPFEATURES := [] #Default entropy source DEFENTROPYSOURCE := /dev/urandom @@ -314,6 +316,7 @@ USER_VARS += DEFBRIDGES USER_VARS += DEFNETWORKMODEL_FC USER_VARS += DEFNETWORKMODEL_QEMU USER_VARS += DEFDISABLEGUESTSECCOMP +USER_VARS += DEFAULTEXPFEATURES USER_VARS += DEFDISABLEBLOCK USER_VARS += DEFBLOCKSTORAGEDRIVER_FC USER_VARS += DEFBLOCKSTORAGEDRIVER_QEMU @@ -503,6 +506,7 @@ $(GENERATED_FILES): %: %.in Makefile VERSION -e "s|@DEFNETWORKMODEL_FC@|$(DEFNETWORKMODEL_FC)|g" \ -e "s|@DEFNETWORKMODEL_QEMU@|$(DEFNETWORKMODEL_QEMU)|g" \ -e "s|@DEFDISABLEGUESTSECCOMP@|$(DEFDISABLEGUESTSECCOMP)|g" \ + -e "s|@DEFAULTEXPFEATURES@|$(DEFAULTEXPFEATURES)|g" \ -e "s|@DEFDISABLEBLOCK@|$(DEFDISABLEBLOCK)|g" \ -e "s|@DEFBLOCKSTORAGEDRIVER_FC@|$(DEFBLOCKSTORAGEDRIVER_FC)|g" \ -e "s|@DEFBLOCKSTORAGEDRIVER_QEMU@|$(DEFBLOCKSTORAGEDRIVER_QEMU)|g" \ diff --git a/cli/config/configuration-fc.toml.in b/cli/config/configuration-fc.toml.in index 6894b19c1..2c2a14e79 100644 --- a/cli/config/configuration-fc.toml.in +++ b/cli/config/configuration-fc.toml.in @@ -289,3 +289,12 @@ disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ # If you are using docker, `disable_new_netns` only works with `docker run --net=none` # (default: false) #disable_new_netns = true + +# Enabled experimental feature list, format: ["a", "b"]. +# Experimental features are features not stable enough for production, +# They may break compatibility, and are prepared for a big version bump. +# Supported experimental features: +# 1. "newstore": new persist storage driver which breaks backward compatibility, +# expected to move out of experimental in 2.0.0. +# (default: []) +experimental=@DEFAULTEXPFEATURES@ diff --git a/cli/config/configuration-qemu.toml.in b/cli/config/configuration-qemu.toml.in index f655136bf..8c8989b7a 100644 --- a/cli/config/configuration-qemu.toml.in +++ b/cli/config/configuration-qemu.toml.in @@ -336,3 +336,12 @@ disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ # If you are using docker, `disable_new_netns` only works with `docker run --net=none` # (default: false) #disable_new_netns = true + +# Enabled experimental feature list, format: ["a", "b"]. +# Experimental features are features not stable enough for production, +# They may break compatibility, and are prepared for a big version bump. +# Supported experimental features: +# 1. "newstore": new persist storage driver which breaks backward compatibility, +# expected to move out of experimental in 2.0.0. +# (default: []) +experimental=@DEFAULTEXPFEATURES@ diff --git a/cli/kata-env.go b/cli/kata-env.go index 8bf2d0576..8608bf97d 100644 --- a/cli/kata-env.go +++ b/cli/kata-env.go @@ -16,6 +16,7 @@ import ( "github.com/BurntSushi/toml" "github.com/kata-containers/runtime/pkg/katautils" vc "github.com/kata-containers/runtime/virtcontainers" + exp "github.com/kata-containers/runtime/virtcontainers/experimental" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" vcUtils "github.com/kata-containers/runtime/virtcontainers/utils" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -26,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 { @@ -69,6 +70,7 @@ type RuntimeInfo struct { Trace bool DisableGuestSeccomp bool DisableNewNetNs bool + Experimental []exp.Feature Path string } @@ -181,6 +183,7 @@ func getRuntimeInfo(configFile string, config oci.RuntimeConfig) RuntimeInfo { Config: runtimeConfig, Path: runtimePath, DisableNewNetNs: config.DisableNewNetNs, + Experimental: config.Experimental, DisableGuestSeccomp: config.DisableGuestSeccomp, } } diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index 7d812f066..4d07c6e46 100644 --- a/pkg/katautils/config.go +++ b/pkg/katautils/config.go @@ -16,6 +16,7 @@ import ( "github.com/BurntSushi/toml" vc "github.com/kata-containers/runtime/virtcontainers" "github.com/kata-containers/runtime/virtcontainers/device/config" + exp "github.com/kata-containers/runtime/virtcontainers/experimental" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" "github.com/kata-containers/runtime/virtcontainers/utils" "github.com/sirupsen/logrus" @@ -122,11 +123,12 @@ type proxy struct { } type runtime struct { - Debug bool `toml:"enable_debug"` - Tracing bool `toml:"enable_tracing"` - DisableNewNetNs bool `toml:"disable_new_netns"` - DisableGuestSeccomp bool `toml:"disable_guest_seccomp"` - InterNetworkModel string `toml:"internetworking_model"` + Debug bool `toml:"enable_debug"` + Tracing bool `toml:"enable_tracing"` + DisableNewNetNs bool `toml:"disable_new_netns"` + DisableGuestSeccomp bool `toml:"disable_guest_seccomp"` + Experimental []string `toml:"experimental"` + InterNetworkModel string `toml:"internetworking_model"` } type shim struct { @@ -800,32 +802,15 @@ func initConfig() (config oci.RuntimeConfig, err error) { // All paths are resolved fully meaning if this function does not return an // error, all paths are valid at the time of the call. func LoadConfiguration(configPath string, ignoreLogging, builtIn bool) (resolvedConfigPath string, config oci.RuntimeConfig, err error) { - var resolved string config, err = initConfig() if err != nil { return "", oci.RuntimeConfig{}, err } - if configPath == "" { - resolved, err = getDefaultConfigFile() - } else { - resolved, err = ResolvePath(configPath) - } - + tomlConf, resolved, err := decodeConfig(configPath) if err != nil { - return "", config, fmt.Errorf("Cannot find usable config file (%v)", err) - } - - configData, err := ioutil.ReadFile(resolved) - if err != nil { - return "", config, err - } - - var tomlConf tomlConfig - _, err = toml.Decode(string(configData), &tomlConf) - if err != nil { - return "", config, err + return "", oci.RuntimeConfig{}, err } config.Debug = tomlConf.Runtime.Debug @@ -872,6 +857,13 @@ func LoadConfiguration(configPath string, ignoreLogging, builtIn bool) (resolved } config.DisableNewNetNs = tomlConf.Runtime.DisableNewNetNs + for _, f := range tomlConf.Runtime.Experimental { + feature := exp.Get(f) + if feature == nil { + return "", config, fmt.Errorf("Unsupported experimental feature %q", f) + } + config.Experimental = append(config.Experimental, *feature) + } if err := checkConfig(config); err != nil { return "", config, err @@ -880,6 +872,36 @@ func LoadConfiguration(configPath string, ignoreLogging, builtIn bool) (resolved return resolved, config, nil } +func decodeConfig(configPath string) (tomlConfig, string, error) { + var ( + resolved string + tomlConf tomlConfig + err error + ) + + if configPath == "" { + resolved, err = getDefaultConfigFile() + } else { + resolved, err = ResolvePath(configPath) + } + + if err != nil { + return tomlConf, "", fmt.Errorf("Cannot find usable config file (%v)", err) + } + + configData, err := ioutil.ReadFile(resolved) + if err != nil { + return tomlConf, resolved, err + } + + _, err = toml.Decode(string(configData), &tomlConf) + if err != nil { + return tomlConf, resolved, err + } + + return tomlConf, resolved, nil +} + // checkConfig checks the validity of the specified config. func checkConfig(config oci.RuntimeConfig) error { if err := checkNetNsConfig(config); err != nil { diff --git a/virtcontainers/experimental/README.md b/virtcontainers/experimental/README.md new file mode 100644 index 000000000..075eece90 --- /dev/null +++ b/virtcontainers/experimental/README.md @@ -0,0 +1,66 @@ +# Experimental package description + +## 1. What are "experimental" features? + +"Experimental" features are features living in master branch, +but Kata community thinks they're not ready for production use. +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 yourself. + +Criteria of an experimental feature are: + +* the feature breaks backward compatibility + +compatibility is important to Kata Containers, +We will **NEVER** accept any new features/codes which break the backward compatibility of Kata components, +unless they are so important and there's no way to avoid the breakage. +If we decide to involve such changes, maintainers will help to make the decision that which Kata release +it should land. + +Before it's landed as a formal feature, we allow the codes to be merged first into our master with the tag "experimental", +so it can improve in next few releases to be stable enough. + +* the feature is not stable enough currently + +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 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"? + +"WIP"(work in progress) are usually used to mark the PR as incomplete before the PR can be reviewed and merged, +after the PR finishes its designed purpose(fix bugs, add new features etc) and all CI jobs pass, the codes can be merged into master branch. +After merging, we can still mark this part as "experimental", and leave some space for its evolvement in next several releases. + +In one word, "experimental" can be unstable currently but it **MUST** be complete and functional, thus different from "WIP". + +## 3. When should "experimental" features be moved out from "experimental"? + +That depends. + +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). + +The experimental feature should state clearly in documentation the rationale for it to be experimental, +and when it is expected to be non-experimental, +so that maintainers can consider to make it formal in right release. + +## 4. Can "experimental" features fail the CI temporarily? + +No. + +"Experimental" features are part of Kata master codes, it should pass all CI jobs or we can't merge them, +that's different from "WIP", a "WIP" PR can fail the CI temporarily before it can be reviewed and merged. + diff --git a/virtcontainers/experimental/experimental.go b/virtcontainers/experimental/experimental.go new file mode 100644 index 000000000..4204e963e --- /dev/null +++ b/virtcontainers/experimental/experimental.go @@ -0,0 +1,63 @@ +// Copyright (c) 2019 Huawei Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package experimental + +import ( + "fmt" + "regexp" +) + +const ( + nameRegStr = "^[a-z][a-z0-9_]*$" +) + +// Feature to be experimental +type Feature struct { + Name string + Description string + // the expected release version to move out from experimental + ExpRelease string +} + +var ( + supportedFeatures = make(map[string]Feature) +) + +// Register register a new experimental feature +func Register(feature Feature) error { + if err := validateFeature(feature); err != nil { + return err + } + + if _, ok := supportedFeatures[feature.Name]; ok { + return fmt.Errorf("Feature %q had been registered before", feature.Name) + } + supportedFeatures[feature.Name] = feature + return nil +} + +// 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 new file mode 100644 index 000000000..ef6c98813 --- /dev/null +++ b/virtcontainers/experimental/experimental_test.go @@ -0,0 +1,60 @@ +// Copyright (c) 2019 Huawei Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package experimental + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestExperimental(t *testing.T) { + f := Feature{ + Name: "mock", + Description: "mock experimental feature for test", + ExpRelease: "2.0", + } + assert.Nil(t, Get(f.Name)) + + err := Register(f) + assert.Nil(t, err) + + err = Register(f) + assert.NotNil(t, err) + assert.Equal(t, len(supportedFeatures), 1) + + 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/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index 269edc962..cefa8f90f 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -23,6 +23,7 @@ import ( vc "github.com/kata-containers/runtime/virtcontainers" "github.com/kata-containers/runtime/virtcontainers/device/config" + exp "github.com/kata-containers/runtime/virtcontainers/experimental" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" dockershimAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations/dockershim" "github.com/kata-containers/runtime/virtcontainers/types" @@ -134,6 +135,9 @@ type RuntimeConfig struct { //Determines if create a netns for hypervisor process DisableNewNetNs bool + + //Experimental features enabled + Experimental []exp.Feature } // AddKernelParam allows the addition of new kernel parameters to an existing @@ -500,6 +504,8 @@ func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid SystemdCgroup: systemdCgroup, DisableGuestSeccomp: runtime.DisableGuestSeccomp, + + Experimental: runtime.Experimental, } addAssetAnnotations(ocispec, &sandboxConfig) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 630f282f4..d39c46cd8 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -25,6 +25,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" deviceManager "github.com/kata-containers/runtime/virtcontainers/device/manager" + exp "github.com/kata-containers/runtime/virtcontainers/experimental" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/store" @@ -100,6 +101,9 @@ type SandboxConfig struct { SystemdCgroup bool DisableGuestSeccomp bool + + // Experimental features enabled + Experimental []exp.Feature } func (s *Sandbox) trace(name string) (opentracing.Span, context.Context) { @@ -136,6 +140,12 @@ func (sandboxConfig *SandboxConfig) valid() bool { sandboxConfig.HypervisorType = QemuHypervisor } + // validate experimental features + for _, f := range sandboxConfig.Experimental { + if exp.Get(f.Name) == nil { + return false + } + } return true } @@ -439,6 +449,10 @@ func createSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Fac return nil, err } + if len(s.config.Experimental) != 0 { + s.Logger().WithField("features", s.config.Experimental).Infof("Enable experimental features") + } + // Fetch sandbox network to be able to access it from the sandbox structure. var networkNS NetworkNamespace if err := s.store.Load(store.Network, &networkNS); err == nil { diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index f9d78cce4..2fbb7908f 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -22,6 +22,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/device/manager" + exp "github.com/kata-containers/runtime/virtcontainers/experimental" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" "github.com/kata-containers/runtime/virtcontainers/store" "github.com/kata-containers/runtime/virtcontainers/types" @@ -1680,3 +1681,22 @@ func TestSandboxUpdateResources(t *testing.T) { t.Fatal(err) } } + +func TestSandboxExperimentalFeature(t *testing.T) { + testFeature := exp.Feature{ + Name: "mock", + Description: "exp feature for test", + ExpRelease: "1.8.0", + } + sconfig := SandboxConfig{ + ID: testSandboxID, + Experimental: []exp.Feature{testFeature}, + } + + assert.Nil(t, exp.Get(testFeature.Name)) + assert.False(t, sconfig.valid()) + + exp.Register(testFeature) + assert.NotNil(t, exp.Get(testFeature.Name)) + assert.True(t, sconfig.valid()) +}