diff --git a/command/apply.go b/command/apply.go index 2b40eb947..1f353593a 100644 --- a/command/apply.go +++ b/command/apply.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/go-getter" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/terraform" ) @@ -96,7 +97,7 @@ func (c *ApplyCommand) Run(args []string) int { } // Check for the new apply - if terraform.X_newApply { + if experiment.Enabled(experiment.X_newApply) && !experiment.Force() { desc := "Experimental new apply graph has been enabled. This may still\n" + "have bugs, and should be used with care. If you'd like to continue,\n" + "you must enter exactly 'yes' as a response." @@ -116,7 +117,7 @@ func (c *ApplyCommand) Run(args []string) int { } // Check for the new destroy - if terraform.X_newDestroy { + if experiment.Enabled(experiment.X_newDestroy) && !experiment.Force() { desc := "Experimental new destroy graph has been enabled. This may still\n" + "have bugs, and should be used with care. If you'd like to continue,\n" + "you must enter exactly 'yes' as a response." diff --git a/command/meta.go b/command/meta.go index b8c419c1e..bd2016564 100644 --- a/command/meta.go +++ b/command/meta.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/go-getter" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/cli" @@ -336,8 +337,7 @@ func (m *Meta) flagSet(n string) *flag.FlagSet { f.BoolVar(&m.shadow, "shadow", true, "shadow graph") // Experimental features - f.BoolVar(&terraform.X_newApply, "Xnew-apply", false, "experiment: new apply") - f.BoolVar(&terraform.X_newDestroy, "Xnew-destroy", false, "experiment: new destroy") + experiment.Flag(f) // Create an io.Writer that writes to our Ui properly for errors. // This is kind of a hack, but it does the job. Basically: create diff --git a/helper/experiment/experiment.go b/helper/experiment/experiment.go new file mode 100644 index 000000000..1a49c4d27 --- /dev/null +++ b/helper/experiment/experiment.go @@ -0,0 +1,162 @@ +// experiment package contains helper functions for tracking experimental +// features throughout Terraform. +// +// This package should be used for creating, enabling, querying, and deleting +// experimental features. By unifying all of that onto a single interface, +// we can have the Go compiler help us by enforcing every place we touch +// an experimental feature. +// +// To create a new experiment: +// +// 1. Add the experiment to the global vars list below, prefixed with X_ +// +// 2. Add the experiment variable to the All listin the init() function +// +// 3. Use it! +// +// To remove an experiment: +// +// 1. Delete the experiment global var. +// +// 2. Try to compile and fix all the places where the var was referenced. +// +// To use an experiment: +// +// 1. Use Flag() if you want the experiment to be available from the CLI. +// +// 2. Use Enabled() to check whether it is enabled. +// +// As a general user: +// +// 1. The `-Xexperiment-name` flag +// 2. The `TF_X_` env var. +// 3. The `TF_X_FORCE` env var can be set to force an experimental feature +// without human verifications. +// +package experiment + +import ( + "flag" + "fmt" + "os" + "strconv" + "strings" + "sync" +) + +// The experiments that are available are listed below. Any package in +// Terraform defining an experiment should define the experiments below. +// By keeping them all within the experiment package we force a single point +// of definition and use. This allows the compiler to enforce references +// so it becomes easy to remove the features. +var ( + // New apply graph. This will be removed and be the default in 0.8.0. + X_newApply = newBasicID("new-apply", "NEW_APPLY", false) + + // New destroy graph. This will be reomved and be the default in 0.8.0. + X_newDestroy = newBasicID("new-destroy", "NEW_DESTROY", false) + + // Shadow graph. This is already on by default. Disabling it will be + // allowed for awhile in order for it to not block operations. + X_shadow = newBasicID("shadow", "SHADOW", true) +) + +// Global variables this package uses because we are a package +// with global state. +var ( + // all is the list of all experiements. Do not modify this. + All []ID + + // enabled keeps track of what flags have been enabled + enabled map[string]bool + enabledLock sync.Mutex + + // Hidden "experiment" that forces all others to be on without verification + x_force = newBasicID("force", "FORCE", false) +) + +func init() { + // The list of all experiments, update this when an experiment is added. + All = []ID{ + X_newApply, + X_newDestroy, + X_shadow, + x_force, + } + + // Load + reload() +} + +// reload is used by tests to reload the global state. This is called by +// init publicly. +func reload() { + // Initialize + enabledLock.Lock() + enabled = make(map[string]bool) + enabledLock.Unlock() + + // Set defaults and check env vars + for _, id := range All { + // Get the default value + def := id.Default() + + // If we set it in the env var, default it to true + key := fmt.Sprintf("TF_X_%s", strings.ToUpper(id.Env())) + if v := os.Getenv(key); v != "" { + def = v != "0" + } + + // Set the default + SetEnabled(id, def) + } +} + +// Enabled returns whether an experiment has been enabled or not. +func Enabled(id ID) bool { + enabledLock.Lock() + defer enabledLock.Unlock() + return enabled[id.Flag()] +} + +// SetEnabled sets an experiment to enabled/disabled. Please check with +// the experiment docs for when calling this actually affects the experiment. +func SetEnabled(id ID, v bool) { + enabledLock.Lock() + defer enabledLock.Unlock() + enabled[id.Flag()] = v +} + +// Force returns true if the -Xforce of TF_X_FORCE flag is present, which +// advises users of this package to not verify with the user that they want +// experimental behavior and to just continue with it. +func Force() bool { + return Enabled(x_force) +} + +// Flag configures the given FlagSet with the flags to configure +// all active experiments. +func Flag(fs *flag.FlagSet) { + for _, id := range All { + desc := id.Flag() + key := fmt.Sprintf("X%s", id.Flag()) + fs.Var(&idValue{X: id}, key, desc) + } +} + +// idValue implements flag.Value for setting the enabled/disabled state +// of an experiment from the CLI. +type idValue struct { + X ID +} + +func (v *idValue) IsBoolFlag() bool { return true } +func (v *idValue) String() string { return strconv.FormatBool(Enabled(v.X)) } +func (v *idValue) Set(raw string) error { + b, err := strconv.ParseBool(raw) + if err == nil { + SetEnabled(v.X, b) + } + + return err +} diff --git a/helper/experiment/experiment_test.go b/helper/experiment/experiment_test.go new file mode 100644 index 000000000..32055c2c8 --- /dev/null +++ b/helper/experiment/experiment_test.go @@ -0,0 +1,117 @@ +package experiment + +import ( + "flag" + "fmt" + "os" + "testing" +) + +// Test experiments +var ( + X_test1 = newBasicID("test1", "TEST1", false) + X_test2 = newBasicID("test2", "TEST2", true) +) + +// Reinitializes the package to a clean slate +func testReinit() { + All = []ID{X_test1, X_test2, x_force} + reload() +} + +func init() { + testReinit() + + // Clear all env vars so they don't affect tests + for _, id := range All { + os.Unsetenv(fmt.Sprintf("TF_X_%s", id.Env())) + } +} + +func TestDefault(t *testing.T) { + testReinit() + + if Enabled(X_test1) { + t.Fatal("test1 should not be enabled") + } + + if !Enabled(X_test2) { + t.Fatal("test2 should be enabled") + } +} + +func TestEnv(t *testing.T) { + os.Setenv("TF_X_TEST2", "0") + defer os.Unsetenv("TF_X_TEST2") + + testReinit() + + if Enabled(X_test2) { + t.Fatal("test2 should be enabled") + } +} + +func TestFlag(t *testing.T) { + testReinit() + + // Verify default + if !Enabled(X_test2) { + t.Fatal("test2 should be enabled") + } + + // Setup a flag set + fs := flag.NewFlagSet("test", flag.ContinueOnError) + Flag(fs) + fs.Parse([]string{"-Xtest2=false"}) + + if Enabled(X_test2) { + t.Fatal("test2 should not be enabled") + } +} + +func TestFlag_overEnv(t *testing.T) { + os.Setenv("TF_X_TEST2", "1") + defer os.Unsetenv("TF_X_TEST2") + + testReinit() + + // Verify default + if !Enabled(X_test2) { + t.Fatal("test2 should be enabled") + } + + // Setup a flag set + fs := flag.NewFlagSet("test", flag.ContinueOnError) + Flag(fs) + fs.Parse([]string{"-Xtest2=false"}) + + if Enabled(X_test2) { + t.Fatal("test2 should not be enabled") + } +} + +func TestForce(t *testing.T) { + os.Setenv("TF_X_FORCE", "1") + defer os.Unsetenv("TF_X_FORCE") + + testReinit() + + if !Force() { + t.Fatal("should force") + } +} + +func TestForce_flag(t *testing.T) { + os.Unsetenv("TF_X_FORCE") + + testReinit() + + // Setup a flag set + fs := flag.NewFlagSet("test", flag.ContinueOnError) + Flag(fs) + fs.Parse([]string{"-Xforce"}) + + if !Force() { + t.Fatal("should force") + } +} diff --git a/helper/experiment/id.go b/helper/experiment/id.go new file mode 100644 index 000000000..8e2f70732 --- /dev/null +++ b/helper/experiment/id.go @@ -0,0 +1,34 @@ +package experiment + +// ID represents an experimental feature. +// +// The global vars defined on this package should be used as ID values. +// This interface is purposely not implement-able outside of this package +// so that we can rely on the Go compiler to enforce all experiment references. +type ID interface { + Env() string + Flag() string + Default() bool + + unexported() // So the ID can't be implemented externally. +} + +// basicID implements ID. +type basicID struct { + EnvValue string + FlagValue string + DefaultValue bool +} + +func newBasicID(flag, env string, def bool) ID { + return &basicID{ + EnvValue: env, + FlagValue: flag, + DefaultValue: def, + } +} + +func (id *basicID) Env() string { return id.EnvValue } +func (id *basicID) Flag() string { return id.FlagValue } +func (id *basicID) Default() bool { return id.DefaultValue } +func (id *basicID) unexported() {} diff --git a/terraform/context.go b/terraform/context.go index 176c1e182..80bdf2d50 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -11,20 +11,7 @@ import ( "github.com/hashicorp/hcl" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" -) - -// Variables prefixed with X_ are experimental features. They can be enabled -// by setting them to true. This should be done before any API is called. -// These should be expected to be removed at some point in the future; each -// option should mention a schedule. -var ( - // X_newApply will enable the new apply graph. This will be removed - // and be on by default in 0.8.0. - X_newApply = false - - // X_newDestroy will enable the new destroy graph. This will be removed - // and be on by default in 0.8.0. - X_newDestroy = false + "github.com/hashicorp/terraform/helper/experiment" ) // InputMode defines what sort of input will be asked for when Input @@ -55,11 +42,6 @@ var ( // Plan operation, effectively testing the Diff DeepCopy whenever // a Plan occurs. This is enabled for tests. contextTestDeepCopyOnPlan = false - - // contextTestShadow will enable the shadow graph for the new graphs. - // This is enabled for tests. This will be removed very shortly and - // be enabled by default. - contextTestShadow = false ) // ContextOpts are the user-configurable options to create a context with @@ -375,6 +357,8 @@ func (c *Context) Apply() (*State, error) { // Copy our own state c.state = c.state.DeepCopy() + X_newApply := experiment.Enabled(experiment.X_newApply) + X_newDestroy := experiment.Enabled(experiment.X_newDestroy) newGraphEnabled := (c.destroy && X_newDestroy) || (!c.destroy && X_newApply) // Build the original graph. This is before the new graph builders @@ -432,12 +416,6 @@ func (c *Context) Apply() (*State, error) { log.Printf("[WARN] terraform: real graph is original, shadow is experiment") } - // For now, always shadow with the real graph for verification. We don't - // want to shadow yet with the new graphs. - if !contextTestShadow { - shadow = real - } - // Determine the operation operation := walkApply if c.destroy { @@ -507,6 +485,9 @@ func (c *Context) Plan() (*Plan, error) { c.diff.init() c.diffLock.Unlock() + // Used throughout below + X_newDestroy := experiment.Enabled(experiment.X_newDestroy) + // Build the graph. We have a branch here since for the pure-destroy // plan (c.destroy) we use a much simpler graph builder that simply // walks the state and reverses edges. @@ -714,6 +695,11 @@ func (c *Context) walk( // the real work: talking to real providers, modifying real state, etc. realCtx := c + // If we don't want shadowing, remove it + if !experiment.Enabled(experiment.X_shadow) { + shadow = nil + } + // If we have a shadow graph, walk that as well var shadowCtx *Context var shadowCloser Shadow diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index a1265cefa..42994fe7d 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/go-getter" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/helper/logging" ) @@ -22,19 +23,9 @@ import ( const fixtureDir = "./test-fixtures" func TestMain(m *testing.M) { - // Experimental features - xNewApply := flag.Bool("Xnew-apply", false, "Experiment: new apply graph") - xNewDestroy := flag.Bool("Xnew-destroy", false, "Experiment: new destroy graph") - - // Normal features - shadow := flag.Bool("shadow", true, "Enable shadow graph") - + experiment.Flag(flag.CommandLine) flag.Parse() - // Setup experimental features - X_newApply = *xNewApply - X_newDestroy = *xNewDestroy - if testing.Verbose() { // if we're verbose, use the logging requested by TF_LOG logging.SetOutput() @@ -49,9 +40,6 @@ func TestMain(m *testing.M) { // Always DeepCopy the Diff on every Plan during a test contextTestDeepCopyOnPlan = true - // Shadow the new graphs - contextTestShadow = *shadow - os.Exit(m.Run()) }