New config framework for the different schedulers

README.md

@@ -197,7 +197,6 @@ Configuration mechanisms do have an order of precedence. As presented, options a
 As shown above, there are several ways to configure the number of simultaneous virtual users k6 will launch. There are also different ways to specify how long those virtual users will be running. For simple tests you can:
 - Set the test duration by the `--duration`/`-d` CLI flag (or the `K6_DURATION` environment variable and the `duration` script/JSON option). For ease of use, `duration` is specified with human readable values like `1h30m10s` - `k6 run --duration 30s script.js`, `k6 cloud -d 15m10s script.js`, `export K6_DURATION=1h`, etc. If set to `0`, k6 wouldn't stop executing the script unless the user manually stops it.
 - Set the total number of script iterations with the `--iterations`/`-i` CLI flag (or the `K6_ITERATIONS` environment variable and the `iterations` script/JSON option). k6 will stop executing the script whenever the **total** number of iterations (i.e. the number of iterations across all VUs) reaches the specified number. So if you have `k6 run --iterations 10 --vus 10 script.js`, then each VU would make only a single iteration.
-- Set both the test duration and the total number of script iterations. In that case, k6 would stop the script execution whenever either one of the above conditions is reached first.
 
 For more complex cases, you can specify execution stages. They are a combination of `duration,target-VUs` pairs. These pairs instruct k6 to linearly ramp up, ramp down, or stay at the number of VUs specified for the period specified. Execution stages can be set via the `stages` script/JSON option as an array of `{ duration: ..., target: ... }` pairs, or with the `--stage`/`-s` CLI flags and the `K6_STAGE` environment variable via the `duration:target,duration:target...` syntax.
 

cmd/archive.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 var archiveOut = "archive.tar"
@@ -75,6 +76,10 @@ An archive is a fully self-contained test run, and can be executed identically e
 			return err
 		}
 
+		if cerr := validateConfig(conf); cerr != nil {
+			return ExitCode{cerr, invalidConfigErrorCode}
+		}
+
 		err = r.SetOptions(conf.Options)
 		if err != nil {
 			return err
@@ -90,11 +95,18 @@ An archive is a fully self-contained test run, and can be executed identically e
 	},
 }
 
+func archiveCmdFlagSet() *pflag.FlagSet {
+	flags := pflag.NewFlagSet("", pflag.ContinueOnError)
+	flags.SortFlags = false
+	flags.AddFlagSet(optionFlagSet())
+	flags.AddFlagSet(runtimeOptionFlagSet(false))
+	//TODO: figure out a better way to handle the CLI flags - global variables are not very testable... :/
+	flags.StringVarP(&archiveOut, "archive-out", "O", archiveOut, "archive output filename")
+	return flags
+}
+
 func init() {
 	RootCmd.AddCommand(archiveCmd)
 	archiveCmd.Flags().SortFlags = false
-	archiveCmd.Flags().AddFlagSet(optionFlagSet())
-	archiveCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false))
-	archiveCmd.Flags().AddFlagSet(configFileFlagSet())
-	archiveCmd.Flags().StringVarP(&archiveOut, "archive-out", "O", archiveOut, "archive output filename")
+	archiveCmd.Flags().AddFlagSet(archiveCmdFlagSet())
 }

cmd/cloud.go

@@ -36,6 +36,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 
 	log "github.com/sirupsen/logrus"
 )
@@ -54,7 +55,8 @@ This will execute the test on the Load Impact cloud service. Use "k6 login cloud
         k6 cloud script.js`[1:],
 	Args: exactArgsWithMsg(1, "arg should either be \"-\", if reading script from stdin, or a path to a script file"),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		_, _ = BannerColor.Fprint(stdout, Banner+"\n\n")
+		//TODO: disable in quiet mode?
+		_, _ = BannerColor.Fprintf(stdout, "\n%s\n\n", Banner)
 		initBar := ui.ProgressBar{
 			Width: 60,
 			Left:  func() string { return "    uploading script" },
@@ -93,6 +95,10 @@ This will execute the test on the Load Impact cloud service. Use "k6 login cloud
 			return err
 		}
 
+		if cerr := validateConfig(conf); cerr != nil {
+			return ExitCode{cerr, invalidConfigErrorCode}
+		}
+
 		err = r.SetOptions(conf.Options)
 		if err != nil {
 			return err
@@ -235,10 +241,26 @@ This will execute the test on the Load Impact cloud service. Use "k6 login cloud
 	},
 }
 
+func cloudCmdFlagSet() *pflag.FlagSet {
+	flags := pflag.NewFlagSet("", pflag.ContinueOnError)
+	flags.SortFlags = false
+	flags.AddFlagSet(optionFlagSet())
+	flags.AddFlagSet(runtimeOptionFlagSet(false))
+
+	//TODO: Figure out a better way to handle the CLI flags:
+	// - the default value is specified in this way so we don't overwrire whatever
+	//   was specified via the environment variable
+	// - global variables are not very testable... :/
+	flags.BoolVar(&exitOnRunning, "exit-on-running", exitOnRunning, "exits when test reaches the running status")
+	// We also need to explicitly set the default value for the usage message here, so setting
+	// K6_EXIT_ON_RUNNING=true won't affect the usage message
+	flags.Lookup("exit-on-running").DefValue = "false"
+
+	return flags
+}
+
 func init() {
 	RootCmd.AddCommand(cloudCmd)
 	cloudCmd.Flags().SortFlags = false
-	cloudCmd.Flags().AddFlagSet(optionFlagSet())
-	cloudCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false))
-	cloudCmd.Flags().BoolVar(&exitOnRunning, "exit-on-running", exitOnRunning, "exits when test reaches the running status")
+	cloudCmd.Flags().AddFlagSet(cloudCmdFlagSet())
 }

cmd/common.go

@@ -68,6 +68,8 @@ func (w consoleWriter) Write(p []byte) (n int, err error) {
 	return
 }
 
+//TODO: refactor the CLI config so these functions aren't needed - they
+// can mask errors by failing only at runtime, not at compile time
 func getNullBool(flags *pflag.FlagSet, key string) null.Bool {
 	v, err := flags.GetBool(key)
 	if err != nil {

cmd/config.go

@@ -1,7 +1,7 @@
 /*
  *
  * k6 - a next-generation load testing tool
- * Copyright (C) 2016 Load Impact
+ * Copyright (C) 2019 Load Impact
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Affero General Public License as
@@ -22,34 +22,27 @@ package cmd
 
 import (
 	"encoding/json"
-	"io/ioutil"
+	"fmt"
 	"os"
+	"path/filepath"
+	"strings"
+
+	"errors"
 
 	"github.com/kelseyhightower/envconfig"
 	"github.com/loadimpact/k6/lib"
+	"github.com/loadimpact/k6/lib/scheduler"
 	"github.com/loadimpact/k6/stats/cloud"
 	"github.com/loadimpact/k6/stats/datadog"
 	"github.com/loadimpact/k6/stats/influxdb"
 	"github.com/loadimpact/k6/stats/kafka"
 	"github.com/loadimpact/k6/stats/statsd/common"
-	"github.com/shibukawa/configdir"
+	log "github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
 	"github.com/spf13/pflag"
 	null "gopkg.in/guregu/null.v3"
 )
 
-const configFilename = "config.json"
-
-var configDirs = configdir.New("loadimpact", "k6")
-var configFile = os.Getenv("K6_CONFIG") // overridden by `-c` flag!
-
-// configFileFlagSet returns a FlagSet that contains flags needed for specifying a config file.
-func configFileFlagSet() *pflag.FlagSet {
-	flags := pflag.NewFlagSet("", 0)
-	flags.StringVarP(&configFile, "config", "c", configFile, "specify config file to read")
-	return flags
-}
-
 // configFlagSet returns a FlagSet with the default run configuration flags.
 func configFlagSet() *pflag.FlagSet {
 	flags := pflag.NewFlagSet("", 0)
@@ -59,7 +52,6 @@ func configFlagSet() *pflag.FlagSet {
 	flags.Bool("no-usage-report", false, "don't send anonymous stats to the developers")
 	flags.Bool("no-thresholds", false, "don't run thresholds")
 	flags.Bool("no-summary", false, "don't show the summary at the end of the test")
-	flags.AddFlagSet(configFileFlagSet())
 	return flags
 }
 
@@ -126,41 +118,51 @@ func getConfig(flags *pflag.FlagSet) (Config, error) {
 	}, nil
 }
 
-// Reads a configuration file from disk.
-func readDiskConfig(fs afero.Fs) (Config, *configdir.Config, error) {
-	if configFile != "" {
-		data, err := ioutil.ReadFile(configFile)
-		if err != nil {
-			return Config{}, nil, err
-		}
-		var conf Config
-		err = json.Unmarshal(data, &conf)
-		return conf, nil, err
+// Reads the configuration file from the supplied filesystem and returns it and its path.
+// It will first try to see if the user explicitly specified a custom config file and will
+// try to read that. If there's a custom config specified and it couldn't be read or parsed,
+// an error will be returned.
+// If there's no custom config specified and no file exists in the default config path, it will
+// return an empty config struct, the default config location and *no* error.
+func readDiskConfig(fs afero.Fs) (Config, string, error) {
+	realConfigFilePath := configFilePath
+	if realConfigFilePath == "" {
+		// The user didn't specify K6_CONFIG or --config, use the default path
+		realConfigFilePath = defaultConfigFilePath
 	}
 
-	cdir := configDirs.QueryFolderContainsFile(configFilename)
-	if cdir == nil {
-		return Config{}, configDirs.QueryFolders(configdir.Global)[0], nil
+	// Try to see if the file exists in the supplied filesystem
+	if _, err := fs.Stat(realConfigFilePath); err != nil {
+		if os.IsNotExist(err) && configFilePath == "" {
+			// If the file doesn't exist, but it was the default config file (i.e. the user
+			// didn't specify anything), silence the error
+			err = nil
+		}
+		return Config{}, realConfigFilePath, err
 	}
-	data, err := cdir.ReadFile(configFilename)
+
+	data, err := afero.ReadFile(fs, realConfigFilePath)
 	if err != nil {
-		return Config{}, cdir, err
+		return Config{}, realConfigFilePath, err
 	}
 	var conf Config
 	err = json.Unmarshal(data, &conf)
-	return conf, cdir, err
+	return conf, realConfigFilePath, err
 }
 
-// Writes configuration back to disk.
-func writeDiskConfig(fs afero.Fs, cdir *configdir.Config, conf Config) error {
+// Serializes the configuration to a JSON file and writes it in the supplied
+// location on the supplied filesystem
+func writeDiskConfig(fs afero.Fs, configPath string, conf Config) error {
 	data, err := json.MarshalIndent(conf, "", "  ")
 	if err != nil {
 		return err
 	}
-	if configFile != "" {
-		return afero.WriteFile(fs, configFilename, data, 0644)
+
+	if err := fs.MkdirAll(filepath.Dir(configPath), 0755); err != nil {
+		return err
 	}
-	return cdir.WriteFile(configFilename, data)
+
+	return afero.WriteFile(fs, configPath, data, 0644)
 }
 
 // Reads configuration variables from the environment.
@@ -177,6 +179,100 @@ func readEnvConfig() (conf Config, err error) {
 	return conf, nil
 }
 
+type executionConflictConfigError string
+
+func (e executionConflictConfigError) Error() string {
+	return string(e)
+}
+
+var _ error = executionConflictConfigError("")
+
+// This checks for conflicting options and turns any shortcut options (i.e. duration, iterations,
+// stages) into the proper scheduler configuration
+func buildExecutionConfig(conf Config) (Config, error) {
+	result := conf
+	switch {
+	case conf.Duration.Valid:
+		if conf.Iterations.Valid {
+			//TODO: make this an executionConflictConfigError in the next version
+			log.Warnf("Specifying both duration and iterations is deprecated and won't be supported in the future k6 versions")
+		}
+
+		if len(conf.Stages) > 0 { // stages isn't nil (not set) and isn't explicitly set to empty
+			//TODO: make this an executionConflictConfigError in the next version
+			log.Warnf("Specifying both duration and stages is deprecated and won't be supported in the future k6 versions")
+		}
+
+		if conf.Execution != nil {
+			return result, executionConflictConfigError("specifying both duration and execution is not supported")
+		}
+
+		if conf.Duration.Duration <= 0 {
+			//TODO: make this an executionConflictConfigError in the next version
+			log.Warnf("Specifying infinite duration in this way is deprecated and won't be supported in the future k6 versions")
+		} else {
+			ds := scheduler.NewConstantLoopingVUsConfig(lib.DefaultSchedulerName)
+			ds.VUs = conf.VUs
+			ds.Duration = conf.Duration
+			ds.Interruptible = null.NewBool(true, false) // Preserve backwards compatibility
+			result.Execution = scheduler.ConfigMap{lib.DefaultSchedulerName: ds}
+		}
+
+	case len(conf.Stages) > 0: // stages isn't nil (not set) and isn't explicitly set to empty
+		if conf.Iterations.Valid {
+			//TODO: make this an executionConflictConfigError in the next version
+			log.Warnf("Specifying both iterations and stages is deprecated and won't be supported in the future k6 versions")
+		}
+
+		if conf.Execution != nil {
+			return conf, executionConflictConfigError("specifying both stages and execution is not supported")
+		}
+
+		ds := scheduler.NewVariableLoopingVUsConfig(lib.DefaultSchedulerName)
+		ds.StartVUs = conf.VUs
+		for _, s := range conf.Stages {
+			if s.Duration.Valid {
+				ds.Stages = append(ds.Stages, scheduler.Stage{Duration: s.Duration, Target: s.Target})
+			}
+		}
+		ds.Interruptible = null.NewBool(true, false) // Preserve backwards compatibility
+		result.Execution = scheduler.ConfigMap{lib.DefaultSchedulerName: ds}
+
+	case conf.Iterations.Valid:
+		if conf.Execution != nil {
+			return conf, executionConflictConfigError("specifying both iterations and execution is not supported")
+		}
+		// TODO: maybe add a new flag that will be used as a shortcut to per-VU iterations?
+
+		ds := scheduler.NewSharedIterationsConfig(lib.DefaultSchedulerName)
+		ds.VUs = conf.VUs
+		ds.Iterations = conf.Iterations
+		result.Execution = scheduler.ConfigMap{lib.DefaultSchedulerName: ds}
+
+	default:
+		if conf.Execution != nil { // If someone set this, regardless if its empty
+			//TODO: remove this warning in the next version
+			log.Warnf("The execution settings are not functional in this k6 release, they will be ignored")
+		}
+
+		if len(conf.Execution) == 0 { // If unset or set to empty
+			// No execution parameters whatsoever were specified, so we'll create a per-VU iterations config
+			// with 1 VU and 1 iteration. We're choosing the per-VU config, since that one could also
+			// be executed both locally, and in the cloud.
+			result.Execution = scheduler.ConfigMap{
+				lib.DefaultSchedulerName: scheduler.NewPerVUIterationsConfig(lib.DefaultSchedulerName),
+			}
+		}
+	}
+
+	//TODO: validate the config; questions:
+	// - separately validate the duration, iterations and stages for better error messages?
+	// - or reuse the execution validation somehow, at the end? or something mixed?
+	// - here or in getConsolidatedConfig() or somewhere else?
+
+	return result, nil
+}
+
 // Assemble the final consolidated configuration from all of the different sources:
 // - start with the CLI-provided options to get shadowed (non-Valid) defaults in there
 // - add the global file config options
@@ -185,6 +281,7 @@ func readEnvConfig() (conf Config, err error) {
 // - merge the user-supplied CLI flags back in on top, to give them the greatest priority
 // - set some defaults if they weren't previously specified
 // TODO: add better validation, more explicit default values and improve consistency between formats
+// TODO: accumulate all errors and differentiate between the layers?
 func getConsolidatedConfig(fs afero.Fs, cliConf Config, runner lib.Runner) (conf Config, err error) {
 	cliConf.Collectors.InfluxDB = influxdb.NewConfig().Apply(cliConf.Collectors.InfluxDB)
 	cliConf.Collectors.Cloud = cloud.NewConfig().Apply(cliConf.Collectors.Cloud)
@@ -205,5 +302,24 @@ func getConsolidatedConfig(fs afero.Fs, cliConf Config, runner lib.Runner) (conf
 	}
 	conf = conf.Apply(envConf).Apply(cliConf)
 
-	return conf, nil
+	return buildExecutionConfig(conf)
+}
+
+//TODO: remove ↓
+//nolint:unparam
+func validateConfig(conf Config) error {
+	errList := conf.Validate()
+	if len(errList) == 0 {
+		return nil
+	}
+
+	errMsgParts := []string{"There were problems with the specified script configuration:"}
+	for _, err := range errList {
+		errMsgParts = append(errMsgParts, fmt.Sprintf("\t- %s", err.Error()))
+	}
+	errMsg := errors.New(strings.Join(errMsgParts, "\n"))
+
+	//TODO: actually return the error here instead of warning, so k6 aborts on config validation errors
+	log.Warn(errMsg)
+	return nil
 }