Merge pull request #1227 from WeiZhang555/experimental-support

config: Add config flag "experimental"

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" \

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@

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@

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,
 	}
 }

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"
@@ -126,6 +127,7 @@ type runtime struct {
 	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"`
 }
 
@@ -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 == "" {
+	tomlConf, resolved, err := decodeConfig(configPath)
-		resolved, err = getDefaultConfigFile()
-	} else {
-		resolved, err = ResolvePath(configPath)
-	}
-
 	if err != nil {
-		return "", config, fmt.Errorf("Cannot find usable config file (%v)", err)
+		return "", oci.RuntimeConfig{}, 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
 	}
 
 	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 {

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.
+

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
+}

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)
+		}
+	}
+}

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)

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 {

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())
+}