From 8ec1548b3c68b4004528d6d402f19d69f9b57652 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Wed, 18 Sep 2019 12:57:04 -0400 Subject: [PATCH] Add features package (#1849) * Add features package Signed-off-by: Nolan Brubaker --- .github/ISSUE_TEMPLATE/bug_report.md | 1 + changelogs/unreleased/1798-nrb | 1 + pkg/client/config.go | 46 +++++++++++-- pkg/client/config_test.go | 33 ++++++++++ pkg/client/factory.go | 13 +--- pkg/client/factory_test.go | 6 +- pkg/cmd/cli/bug/bug.go | 7 +- pkg/cmd/server/server.go | 6 ++ pkg/cmd/velero/velero.go | 26 +++++++- pkg/features/feature_flags.go | 69 ++++++++++++++++++++ pkg/features/feature_flags_test.go | 45 +++++++++++++ pkg/plugin/clientmgmt/client_builder.go | 4 ++ pkg/plugin/clientmgmt/client_builder_test.go | 8 +++ site/docs/master/plugins.md | 5 ++ 14 files changed, 249 insertions(+), 21 deletions(-) create mode 100644 changelogs/unreleased/1798-nrb create mode 100644 pkg/client/config_test.go create mode 100644 pkg/features/feature_flags.go create mode 100644 pkg/features/feature_flags_test.go diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 2d398fbee..93bd33ebb 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -28,6 +28,7 @@ about: Tell us about a problem you are experiencing **Environment:** - Velero version (use `velero version`): +- Velero features (use `velero client config get features`): - Kubernetes version (use `kubectl version`): - Kubernetes installer & version: - Cloud provider or hardware configuration: diff --git a/changelogs/unreleased/1798-nrb b/changelogs/unreleased/1798-nrb new file mode 100644 index 000000000..579db4366 --- /dev/null +++ b/changelogs/unreleased/1798-nrb @@ -0,0 +1 @@ +Add `--features` argument to all velero commands to provide feature flags that can control enablement of pre-release features. diff --git a/pkg/client/config.go b/pkg/client/config.go index 3372f6292..fb71b788e 100644 --- a/pkg/client/config.go +++ b/pkg/client/config.go @@ -1,5 +1,5 @@ /* -Copyright 2018 the Velero contributors. +Copyright 2018, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,23 +20,29 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "github.com/pkg/errors" ) const ( ConfigKeyNamespace = "namespace" + ConfigKeyFeatures = "features" ) -// LoadConfig loads the Velero client configuration file and returns it as a map[string]string. If the +// VeleroConfig is a map of strings to interface{} for deserializing Velero client config options. +// The alias is a way to attach type-asserting convenience methods. +type VeleroConfig map[string]interface{} + +// LoadConfig loads the Velero client configuration file and returns it as a VeleroConfig. If the // file does not exist, an empty map is returned. -func LoadConfig() (map[string]string, error) { +func LoadConfig() (VeleroConfig, error) { fileName := configFileName() _, err := os.Stat(fileName) if os.IsNotExist(err) { // If the file isn't there, just return an empty map - return map[string]string{}, nil + return VeleroConfig{}, nil } if err != nil { // For any other Stat() error, return it @@ -49,7 +55,7 @@ func LoadConfig() (map[string]string, error) { } defer configFile.Close() - var config map[string]string + var config VeleroConfig if err := json.NewDecoder(configFile).Decode(&config); err != nil { return nil, errors.WithStack(err) } @@ -58,7 +64,7 @@ func LoadConfig() (map[string]string, error) { } // SaveConfig saves the passed in config map to the Velero client configuration file. -func SaveConfig(config map[string]string) error { +func SaveConfig(config VeleroConfig) error { fileName := configFileName() // Try to make the directory in case it doesn't exist @@ -76,6 +82,34 @@ func SaveConfig(config map[string]string) error { return json.NewEncoder(configFile).Encode(&config) } +func (c VeleroConfig) Namespace() string { + val, ok := c[ConfigKeyNamespace] + if !ok { + return "" + } + + ns, ok := val.(string) + if !ok { + return "" + } + + return ns +} + +func (c VeleroConfig) Features() []string { + val, ok := c[ConfigKeyFeatures] + if !ok { + return []string{} + } + + features, ok := val.(string) + if !ok { + return []string{} + } + + return strings.Split(features, ",") +} + func configFileName() string { return filepath.Join(os.Getenv("HOME"), ".config", "velero", "config.json") } diff --git a/pkg/client/config_test.go b/pkg/client/config_test.go new file mode 100644 index 000000000..9ef7fad8e --- /dev/null +++ b/pkg/client/config_test.go @@ -0,0 +1,33 @@ +/* +Copyright 2019 the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestVeleroConfig(t *testing.T) { + c := VeleroConfig{ + "namespace": "foo", + "features": "feature1,feature2", + } + + assert.Equal(t, "foo", c.Namespace()) + assert.Equal(t, []string{"feature1", "feature2"}, c.Features()) +} diff --git a/pkg/client/factory.go b/pkg/client/factory.go index aad003399..2393ae086 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -17,7 +17,6 @@ limitations under the License. package client import ( - "fmt" "os" "github.com/pkg/errors" @@ -68,21 +67,15 @@ type factory struct { } // NewFactory returns a Factory. -func NewFactory(baseName string) Factory { +func NewFactory(baseName string, config VeleroConfig) Factory { f := &factory{ flags: pflag.NewFlagSet("", pflag.ContinueOnError), baseName: baseName, } f.namespace = os.Getenv("VELERO_NAMESPACE") - - if config, err := LoadConfig(); err == nil { - // Only override the namespace if the config key is set - if _, ok := config[ConfigKeyNamespace]; ok { - f.namespace = config[ConfigKeyNamespace] - } - } else { - fmt.Fprintf(os.Stderr, "WARNING: error retrieving namespace from config file: %v\n", err) + if config.Namespace() != "" { + f.namespace = config.Namespace() } // We didn't get the namespace via env var or config file, so use the default. diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index 475269eb3..6ff4a5dd9 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -31,14 +31,14 @@ func TestFactory(t *testing.T) { // Env variable should set the namespace if no config or argument are used os.Setenv("VELERO_NAMESPACE", "env-velero") - f := NewFactory("velero") + f := NewFactory("velero", make(map[string]interface{})) assert.Equal(t, "env-velero", f.Namespace()) os.Unsetenv("VELERO_NAMESPACE") // Argument should change the namespace - f = NewFactory("velero") + f = NewFactory("velero", make(map[string]interface{})) s := "flag-velero" flags := new(pflag.FlagSet) @@ -50,7 +50,7 @@ func TestFactory(t *testing.T) { // An arugment overrides the env variable if both are set. os.Setenv("VELERO_NAMESPACE", "env-velero") - f = NewFactory("velero") + f = NewFactory("velero", make(map[string]interface{})) flags = new(pflag.FlagSet) f.BindFlags(flags) diff --git a/pkg/cmd/cli/bug/bug.go b/pkg/cmd/cli/bug/bug.go index c6ff495d6..f3c6a39ca 100644 --- a/pkg/cmd/cli/bug/bug.go +++ b/pkg/cmd/cli/bug/bug.go @@ -32,6 +32,7 @@ import ( "github.com/heptio/velero/pkg/buildinfo" "github.com/heptio/velero/pkg/cmd" + "github.com/heptio/velero/pkg/features" ) const ( @@ -72,6 +73,7 @@ about: Tell us about a problem you are experiencing **Environment:** - Velero version (use ` + "`velero version`" + `):{{.VeleroVersion}} {{.GitCommit}} +- Velero features (use ` + "`velero client config get features`" + `): {{.Features}} - Kubernetes version (use ` + "`kubectl version`" + `): {{- if .KubectlVersion}} ` + "```" + ` @@ -112,6 +114,7 @@ type VeleroBugInfo struct { RuntimeOS string RuntimeArch string KubectlVersion string + Features string } // cmdExistsOnPath checks to see if an executable is available on the current PATH @@ -164,7 +167,9 @@ func newBugInfo(kubectlVersion string) *VeleroBugInfo { GitCommit: buildinfo.FormattedGitSHA(), RuntimeOS: runtime.GOOS, RuntimeArch: runtime.GOARCH, - KubectlVersion: kubectlVersion} + KubectlVersion: kubectlVersion, + Features: features.Serialize(), + } } // renderToString renders IssueTemplate to a string using the diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 9531ad9d4..4602cee4d 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -52,6 +52,7 @@ import ( "github.com/heptio/velero/pkg/cmd/util/signals" "github.com/heptio/velero/pkg/controller" velerodiscovery "github.com/heptio/velero/pkg/discovery" + "github.com/heptio/velero/pkg/features" clientset "github.com/heptio/velero/pkg/generated/clientset/versioned" informers "github.com/heptio/velero/pkg/generated/informers/externalversions" "github.com/heptio/velero/pkg/metrics" @@ -170,6 +171,11 @@ func NewCommand(f client.Factory) *cobra.Command { logger.Infof("setting log-level to %s", strings.ToUpper(logLevel.String())) logger.Infof("Starting Velero server %s (%s)", buildinfo.Version, buildinfo.FormattedGitSHA()) + if len(features.All()) > 0 { + logger.Infof("%d feature flags enabled %s", len(features.All()), features.All()) + } else { + logger.Info("No feature flags enabled") + } if volumeSnapshotLocations.Data() != nil { config.defaultVolumeSnapshotLocations = volumeSnapshotLocations.Data() diff --git a/pkg/cmd/velero/velero.go b/pkg/cmd/velero/velero.go index 73decbbbd..9032d0b4b 100644 --- a/pkg/cmd/velero/velero.go +++ b/pkg/cmd/velero/velero.go @@ -18,6 +18,8 @@ package velero import ( "flag" + "fmt" + "os" "github.com/spf13/cobra" "k8s.io/klog" @@ -41,9 +43,21 @@ import ( "github.com/heptio/velero/pkg/cmd/cli/version" "github.com/heptio/velero/pkg/cmd/server" runplugin "github.com/heptio/velero/pkg/cmd/server/plugin" + veleroflag "github.com/heptio/velero/pkg/cmd/util/flag" + "github.com/heptio/velero/pkg/features" ) func NewCommand(name string) *cobra.Command { + // Load the config here so that we can extract features from it. + config, err := client.LoadConfig() + if err != nil { + fmt.Fprintf(os.Stderr, "WARNING: Error reading config file: %v\n", err) + } + + // Declare cmdFeatures here so we can access them in the PreRun hooks + // without doing a chain of calls into the command's FlagSet + var cmdFeatures veleroflag.StringArray + c := &cobra.Command{ Use: name, Short: "Back up and restore Kubernetes cluster resources.", @@ -54,11 +68,21 @@ way to back up your application state and associated data. If you're familiar with kubectl, Velero supports a similar model, allowing you to execute commands such as 'velero get backup' and 'velero create schedule'. The same operations can also be performed as 'velero backup get' and 'velero schedule create'.`, + // PersistentPreRun will run before all subcommands EXCEPT in the following conditions: + // - a subcommand defines its own PersistentPreRun function + // - the command is run without arguments or with --help and only prints the usage info + PersistentPreRun: func(cmd *cobra.Command, args []string) { + features.Enable(config.Features()...) + features.Enable(cmdFeatures...) + }, } - f := client.NewFactory(name) + f := client.NewFactory(name, config) f.BindFlags(c.PersistentFlags()) + // Bind features directly to the root command so it's available to all callers. + c.PersistentFlags().Var(&cmdFeatures, "features", "Comma-separated list of features to enable for this Velero process. Combines with values from $HOME/.config/velero/config.json if present") + c.AddCommand( backup.NewCommand(f), schedule.NewCommand(f), diff --git a/pkg/features/feature_flags.go b/pkg/features/feature_flags.go new file mode 100644 index 000000000..7fd41a5a1 --- /dev/null +++ b/pkg/features/feature_flags.go @@ -0,0 +1,69 @@ +/* +Copyright 2019 the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package features + +import ( + "strings" + + "k8s.io/apimachinery/pkg/util/sets" +) + +type featureFlagSet struct { + set sets.String +} + +// featureFlags will store all the flags for this process until NewFeatureFlagSet is called. +var featureFlags featureFlagSet + +// IsEnabled returns True if a specified flag is enabled. +func IsEnabled(name string) bool { + return featureFlags.set.Has(name) +} + +// Enable adds a given slice of feature names to the current feature list. +func Enable(names ...string) { + // Initialize the flag set so that users don't have to + if featureFlags.set == nil { + NewFeatureFlagSet() + } + + featureFlags.set.Insert(names...) +} + +// Disable removes all feature flags in a given slice from the current feature list. +func Disable(names ...string) { + featureFlags.set.Delete(names...) +} + +// All returns enabled features as a slice of strings. +func All() []string { + return featureFlags.set.List() +} + +// Serialize returns all features as a comma-separated string. +func Serialize() string { + return strings.Join(All(), ",") +} + +// NewFeatureFlagSet initializes and populates a new FeatureFlagSet. +// This must be called to properly initialize the set for tracking flags. +// It is also useful for selectively controlling flags during tests. +func NewFeatureFlagSet(flags ...string) { + featureFlags = featureFlagSet{ + set: sets.NewString(flags...), + } +} diff --git a/pkg/features/feature_flags_test.go b/pkg/features/feature_flags_test.go new file mode 100644 index 000000000..1c6854392 --- /dev/null +++ b/pkg/features/feature_flags_test.go @@ -0,0 +1,45 @@ +/* +Copyright 2019 the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package features + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFeatureFlags(t *testing.T) { + // Prepare a new flag set + NewFeatureFlagSet("feature1", "feature2") + + assert.True(t, IsEnabled("feature1")) + assert.False(t, IsEnabled("feature3")) + assert.Equal(t, []string{"feature1", "feature2"}, All()) + + Enable("feature3") + assert.True(t, IsEnabled("feature3")) + assert.Equal(t, []string{"feature1", "feature2", "feature3"}, All()) + + Disable("feature3") + assert.Equal(t, []string{"feature1", "feature2"}, All()) + + assert.Equal(t, "feature1,feature2", Serialize()) + + // Calling NewFeatureFlagSet re-initializes the set of flags + NewFeatureFlagSet() + assert.Empty(t, All()) +} diff --git a/pkg/plugin/clientmgmt/client_builder.go b/pkg/plugin/clientmgmt/client_builder.go index 6d0140ab8..8349d4411 100644 --- a/pkg/plugin/clientmgmt/client_builder.go +++ b/pkg/plugin/clientmgmt/client_builder.go @@ -25,6 +25,7 @@ import ( hcplugin "github.com/hashicorp/go-plugin" "github.com/sirupsen/logrus" + "github.com/heptio/velero/pkg/features" "github.com/heptio/velero/pkg/plugin/framework" ) @@ -50,6 +51,9 @@ func newClientBuilder(command string, logger logrus.FieldLogger, logLevel logrus } b.commandArgs = append(b.commandArgs, "--log-level", logLevel.String()) + if len(features.All()) > 0 { + b.commandArgs = append(b.commandArgs, "--features", features.Serialize()) + } return b } diff --git a/pkg/plugin/clientmgmt/client_builder_test.go b/pkg/plugin/clientmgmt/client_builder_test.go index c5292b6d5..6ee9b511a 100644 --- a/pkg/plugin/clientmgmt/client_builder_test.go +++ b/pkg/plugin/clientmgmt/client_builder_test.go @@ -25,6 +25,7 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + "github.com/heptio/velero/pkg/features" "github.com/heptio/velero/pkg/plugin/framework" "github.com/heptio/velero/pkg/test" ) @@ -41,6 +42,13 @@ func TestNewClientBuilder(t *testing.T) { assert.Equal(t, cb.commandName, os.Args[0]) assert.Equal(t, []string{"run-plugins", "--log-level", "info"}, cb.commandArgs) assert.Equal(t, newLogrusAdapter(logger, logLevel), cb.pluginLogger) + + features.NewFeatureFlagSet("feature1", "feature2") + cb = newClientBuilder(os.Args[0], logger, logLevel) + assert.Equal(t, []string{"run-plugins", "--log-level", "info", "--features", "feature1,feature2"}, cb.commandArgs) + // Clear the features list in case other tests run in the same process. + features.NewFeatureFlagSet() + } func TestClientConfig(t *testing.T) { diff --git a/site/docs/master/plugins.md b/site/docs/master/plugins.md index e40253d06..2cef45cd3 100644 --- a/site/docs/master/plugins.md +++ b/site/docs/master/plugins.md @@ -76,6 +76,11 @@ data: Then, in your plugin's implementation, you can read this ConfigMap to fetch the necessary configuration. See the [restic restore action][3] for an example of this -- in particular, the `getPluginConfig(...)` function. +## Feature Flags + +Velero will pass any known features flags as a comma-separated list of strings to the `--features` argument. + +Once parsed into a `[]string`, the features can then be registered using the `NewFeatureFlagSet` function and queried with `features.Enabled()`. [1]: https://github.com/heptio/velero-plugin-example [2]: https://github.com/heptio/velero/blob/master/pkg/plugin/logger.go