Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add features package #1849

Merged
merged 16 commits into from Sep 18, 2019
Merged
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/bug_report.md
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions 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.
46 changes: 40 additions & 6 deletions 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.
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very subjective, but I think this and the next function would read better if the check on the map key was all inlined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can't do this here since val is used outside the if... block?

return ""
}

nrb marked this conversation as resolved.
Show resolved Hide resolved
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")
}
33 changes: 33 additions & 0 deletions 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())
}
13 changes: 3 additions & 10 deletions pkg/client/factory.go
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package client

import (
"fmt"
"os"

"github.com/pkg/errors"
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/factory_test.go
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion pkg/cmd/cli/bug/bug.go
Expand Up @@ -32,6 +32,7 @@ import (

"github.com/heptio/velero/pkg/buildinfo"
"github.com/heptio/velero/pkg/cmd"
"github.com/heptio/velero/pkg/features"
)

const (
Expand Down Expand Up @@ -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}}
` + "```" + `
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/server.go
Expand Up @@ -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"
Expand Down Expand Up @@ -168,6 +169,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()
Expand Down
26 changes: 25 additions & 1 deletion pkg/cmd/velero/velero.go
Expand Up @@ -18,6 +18,8 @@ package velero

import (
"flag"
"fmt"
"os"

"github.com/spf13/cobra"
"k8s.io/klog"
Expand All @@ -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.",
Expand All @@ -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),
Expand Down
70 changes: 70 additions & 0 deletions pkg/features/feature_flags.go
@@ -0,0 +1,70 @@
/*
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) {

nrb marked this conversation as resolved.
Show resolved Hide resolved
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...),
}
}