multus: add config file for validation tool

Allow the validation tool to read test config from a yaml file. To help
users, also allow outputting a config file with default values and
comments instructing how to use the config file.

This work is in anticipation of adding more advanced configuration
options that would be too cumbersome to set using cli flags.

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
(cherry picked from commit f8d9076a38)

.commitlintrc.json

@@ -23,6 +23,7 @@
         "mgr",
         "mon",
         "monitoring",
+        "multus",
         "nfs",
         "object",
         "operator",

cmd/rook/userfacing/multus/validation/config.go

@@ -0,0 +1,50 @@
+/*
+Copyright 2023 The Rook Authors. All rights reserved.
+
+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 validation
+
+import (
+	"fmt"
+
+	"github.com/rook/rook/pkg/daemon/multus"
+	"github.com/spf13/cobra"
+)
+
+var (
+	configCmd = &cobra.Command{
+		Use:   "config",
+		Short: "Generate a validation test config file for different default scenarios.",
+		Args:  cobra.NoArgs,
+	}
+
+	simpleConfigCmd = &cobra.Command{
+		Use:   "simple",
+		Short: "Generate a simple default config file.",
+		Args:  cobra.NoArgs,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			y, err := multus.NewDefaultValidationTestConfig().ToYAML()
+			if err != nil {
+				return err
+			}
+			fmt.Print(y)
+			return nil
+		},
+	}
+)
+
+func init() {
+	configCmd.AddCommand(simpleConfigCmd)
+}

cmd/rook/userfacing/multus/validation/validation.go

@@ -26,24 +26,13 @@ import (
 	"github.com/coreos/pkg/capnslog"
 	"github.com/rook/rook/cmd/rook/rook"
 	"github.com/rook/rook/pkg/daemon/multus"
-	"github.com/rook/rook/pkg/operator/k8sutil"
 	"github.com/spf13/cobra"
 )
 
-// defaults
-var (
-	DefaultValidationNamespace = "rook-ceph"
-
-	// 1 mon, 3 osds, 1 mgr, 1 mds, 1 nfs, 1 rgw, 1 rbdmirror, 1 cephfsmirror,
-	// (1 csi provisioner, 1 csi plugin) x3 for rbd, cephfs, and nfs CSI drivers
-	DefaultDaemonsPerNode = 16
-
-	DefaultTimeoutMinutes = 3
-)
-
 // config
 var (
-	validationConfig = multus.ValidationTest{
+	validationConfigFile = ""
+	validationConfig     = multus.ValidationTest{
 		Logger: capnslog.NewPackageLogger("github.com/rook/rook", "multus-validation"),
 	}
 )
@@ -64,6 +53,8 @@ var (
 Run a validation test that determines whether the current Multus and system
 configurations will support Rook with Multus.
 
+This should be run BEFORE Rook is installed.
+
 This is a fairly long-running test. It starts up a web server and many
 clients to verify that Multus network communication works properly.
 
@@ -71,6 +62,7 @@ It does *not* perform any load testing. Networks that cannot support high
 volumes of Ceph traffic may still encounter runtime issues. This may be
 particularly noticeable with high I/O load or during OSD rebalancing
 (see: https://docs.ceph.com/en/latest/architecture/#rebalancing).
+For example, during Rook or Ceph cluster upgrade.
 `,
 		Run: func(cmd *cobra.Command, args []string) {
 			runValidation(cmd.Context())
@@ -94,21 +86,19 @@ func init() {
 
 	Cmd.AddCommand(runCmd)
 	Cmd.AddCommand(cleanupCmd)
+	Cmd.AddCommand(configCmd)
 
-	// flags on ALL subcommands - makes output more straightforward than using PersistentFlags() global flags on parent
-	for _, subCommand := range Cmd.Commands() {
-		// the default namespace is the current namespace the operator pod is running in if possible
-		defaultNamespace := os.Getenv(k8sutil.PodNamespaceEnvVar)
-		if defaultNamespace == "" {
-			defaultNamespace = DefaultValidationNamespace
-		}
-		subCommand.Flags().StringVarP(&validationConfig.Namespace, "namespace", "n", defaultNamespace,
+	defaultConfig := multus.NewDefaultValidationTestConfig()
+
+	// flags on run/cleanup subcommands - makes output more straightforward than using PersistentFlags() global flags on parent
+	for _, subCommand := range []*cobra.Command{runCmd, cleanupCmd} {
+		subCommand.Flags().StringVarP(&validationConfig.Namespace, "namespace", "n", defaultConfig.Namespace,
 			"The namespace for validation test resources. "+
 				"It is recommended to set this to the namespace in which Rook's Ceph cluster will be installed.")
 
 		// VarPF() keeps the the specific var passed to it for setting at runtime, and the current
 		// val of that var when VarPF() is called is used as the default
-		validationConfig.ResourceTimeout = time.Duration(DefaultTimeoutMinutes) * time.Minute
+		validationConfig.ResourceTimeout = defaultConfig.ResourceTimeout
 		t := (*timeoutMinutes)(&validationConfig.ResourceTimeout)
 		subCommand.Flags().VarPF(t, "timeout-minutes", "", /* no shorthand */
 			"The time to wait for resources to change to the expected state. For example, for the "+
@@ -119,33 +109,51 @@ func init() {
 	}
 
 	// flags for 'validation run'
-	runCmd.Flags().StringVar(&validationConfig.PublicNetwork, "public-network", "",
+	runCmd.Flags().StringVar(&validationConfig.PublicNetwork, "public-network", defaultConfig.PublicNetwork,
 		"The name of the Network Attachment Definition (NAD) that will be used for Ceph's public network. "+
 			"This should be a namespaced name in the form <namespace>/<name> if the NAD is defined in a different namespace from the cluster namespace.")
-	runCmd.Flags().StringVar(&validationConfig.ClusterNetwork, "cluster-network", "",
+	runCmd.Flags().StringVar(&validationConfig.ClusterNetwork, "cluster-network", defaultConfig.ClusterNetwork,
 		"The name of the Network Attachment Definition (NAD) that will be used for Ceph's cluster network. "+
 			"This should be a namespaced name in the form <namespace>/<name> if the NAD is defined in a different namespace from the cluster namespace.")
-	runCmd.Flags().IntVar(&validationConfig.DaemonsPerNode, "daemons-per-node", DefaultDaemonsPerNode,
+	runCmd.Flags().IntVar(&validationConfig.DaemonsPerNode, "daemons-per-node", defaultConfig.DaemonsPerNode,
 		"The number of validation test daemons to run per node. "+
 			"It is recommended to set this to the maximum number of Ceph daemons that can run on any node in the worst case of node failure(s). "+
 			"The default value is set to the worst-case value for a Rook Ceph cluster with 3 portable OSDs, 3 portable monitors, "+
 			"and where all optional child resources have been created with 1 daemon such that they all might run on a single node in a failure scenario. "+
 			"If you aren't sure what to choose for this value, add 1 for each additional OSD beyond 3.")
-	runCmd.Flags().StringVar(&validationConfig.NginxImage, "nginx-image", "nginxinc/nginx-unprivileged:stable-alpine",
+	runCmd.Flags().StringVar(&validationConfig.NginxImage, "nginx-image", defaultConfig.NginxImage,
 		"The Nginx image used for the validation server and clients.")
 
+	runCmd.Flags().StringVarP(&validationConfigFile, "config", "c", "",
+		"The validation test config file to use. This cannot be used with other flags.")
+	runCmd.MarkFlagsMutuallyExclusive("config", "timeout-minutes")
+	runCmd.MarkFlagsMutuallyExclusive("config", "namespace")
+	runCmd.MarkFlagsMutuallyExclusive("config", "public-network")
+	runCmd.MarkFlagsMutuallyExclusive("config", "cluster-network")
+	runCmd.MarkFlagsMutuallyExclusive("config", "daemons-per-node")
+	runCmd.MarkFlagsMutuallyExclusive("config", "nginx-image")
+
 	// flags for 'validation cleanup'
 	// none
 }
 
 func runValidation(ctx context.Context) {
-	if validationConfig.PublicNetwork == "" && validationConfig.ClusterNetwork == "" {
-		fmt.Print(`at least one of "--public-network" or "--cluster-network" must be specified`)
-		os.Exit(22 /* EINVAL */)
+	if validationConfigFile != "" {
+		f, err := os.ReadFile(validationConfigFile)
+		if err != nil {
+			fmt.Printf("failed to read config file %q: %s\n", validationConfigFile, err)
+			os.Exit(1)
+		}
+		c, err := multus.ValidationTestConfigFromYAML(string(f))
+		if err != nil {
+			fmt.Printf("failed to parse config file %q: %s\n", validationConfigFile, err)
+			os.Exit(22 /* EINVAL */)
+		}
+		validationConfig.ValidationTestConfig = *c
 	}
 
-	if validationConfig.NginxImage == "" {
-		fmt.Print(`--nginx-image must be specified`)
+	if err := validationConfig.ValidationTestConfig.Validate(); err != nil {
+		fmt.Print(err.Error() + "\n")
 		os.Exit(22 /* EINVAL */)
 	}
 

pkg/daemon/multus/config.go

@@ -0,0 +1,131 @@
+/*
+Copyright 2023 The Rook Authors. All rights reserved.
+
+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 multus
+
+import (
+	_ "embed"
+	"fmt"
+	"os"
+	"strings"
+	"time"
+
+	"github.com/rook/rook/pkg/operator/k8sutil"
+	"gopkg.in/yaml.v2"
+)
+
+var (
+	//go:embed config.yaml
+	ConfigYaml string
+)
+
+var (
+	DefaultValidationNamespace = "rook-ceph"
+
+	// 1 mon, 3 osds, 1 mgr, 1 mds, 1 nfs, 1 rgw, 1 rbdmirror, 1 cephfsmirror,
+	// (1 csi provisioner, 1 csi plugin) x3 for rbd, cephfs, and nfs CSI drivers
+	DefaultValidationDaemonsPerNode = 16
+
+	DefaultValidationNginxImage = "nginxinc/nginx-unprivileged:stable-alpine"
+
+	DefaultValidationResourceTimeout = 3 * time.Minute
+)
+
+func init() {
+	// the default namespace is the current namespace the operator pod is running in if possible
+	ns := os.Getenv(k8sutil.PodNamespaceEnvVar)
+	if ns != "" {
+		DefaultValidationNamespace = ns
+	}
+}
+
+// ValidationTestConfig is a configuration for a Multus validation test. To prevent documentation
+// for this struct from getting out of date, see the output of ValidationTestConfig.ToYAML() for
+// usage text for each field.
+type ValidationTestConfig struct {
+	Namespace       string        `yaml:"namespace"`
+	PublicNetwork   string        `yaml:"publicNetwork"`
+	ClusterNetwork  string        `yaml:"clusterNetwork"`
+	DaemonsPerNode  int           `yaml:"daemonsPerNode"`
+	ResourceTimeout time.Duration `yaml:"resourceTimeout"`
+	NginxImage      string        `yaml:"nginxImage"`
+}
+
+// NewDefaultValidationTestConfig returns a new ValidationTestConfig with default values.
+func NewDefaultValidationTestConfig() *ValidationTestConfig {
+	c := &ValidationTestConfig{}
+	c.Namespace = DefaultValidationNamespace
+	c.DaemonsPerNode = DefaultValidationDaemonsPerNode
+	c.ResourceTimeout = DefaultValidationResourceTimeout
+	c.NginxImage = DefaultValidationNginxImage
+	return c
+}
+
+// ToYAML converts the validation test config into a YAML representation with user-readable comments
+// describing how to use the various parameters.
+func (c *ValidationTestConfig) ToYAML() (string, error) {
+	// No Go YAML libraries seem to support fields with default-comments attached to them. It would
+	// be silly to use some super-advanced reflection techniques or to extend our own YAML library,
+	// so it is at least straightforward to render the config file from a Go template.
+	t, err := loadTemplate("config.yaml", ConfigYaml, c)
+	if err != nil {
+		return "", fmt.Errorf("failed to load config into yaml template: %w", err)
+	}
+	return string(t), nil
+}
+
+// String implements the Stringer interface
+func (c *ValidationTestConfig) String() string {
+	out, err := yaml.Marshal(c)
+	if err != nil {
+		return "failed quick marshal of validation test config!"
+	}
+	return string(out)
+}
+
+// ValidationTestConfigFromYAML loads a YAML-formatted string into a new ValidationTestConfig.
+func ValidationTestConfigFromYAML(y string) (*ValidationTestConfig, error) {
+	c := &ValidationTestConfig{}
+	err := yaml.Unmarshal([]byte(y), c)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal config from yaml: %w", err)
+	}
+	return c, nil
+}
+
+// Validate reports any validation test configuration problems as errors.
+func (c *ValidationTestConfig) Validate() error {
+	errs := []string{}
+	if c.Namespace == "" {
+		errs = append(errs, "namespace must be specified")
+	}
+	if c.PublicNetwork == "" && c.ClusterNetwork == "" {
+		errs = append(errs, "at least one of publicNetwork and clusterNetwork must be specified")
+	}
+	if c.DaemonsPerNode == 0 {
+		errs = append(errs, "daemonsPerNode must be nonzero")
+	}
+	if c.ResourceTimeout < 1*time.Minute {
+		errs = append(errs, "resourceTimeout must be at least one minute (two or more are recommended)")
+	}
+	if c.NginxImage == "" {
+		errs = append(errs, "nginxImage must be specified")
+	}
+	if len(errs) > 0 {
+		return fmt.Errorf("validation test config is invalid: %s", strings.Join(errs, ", "))
+	}
+	return nil
+}

pkg/daemon/multus/config.yaml

@@ -0,0 +1,29 @@
+# The intended namespace where the validation test will be run. This is should be where the
+# Rook-Ceph cluster will be installed.
+namespace: "{{ .Namespace }}"
+
+# These fields should be set to the name of the Network Attachment Definition (NAD) which will be
+# used for the Ceph cluster's public or cluster network, respectively. This should be a namespaced
+# name in the form <namespace>/<name> if the NAD is defined in a different namespace from the
+# CephCluster namespace. One or both of the fields may be set.
+publicNetwork: "{{ .PublicNetwork }}"
+clusterNetwork: "{{ .ClusterNetwork }}"
+
+# The number of Multus-connected validation daemons that will run as part of the test. This should
+# be set to the largest number of Ceph daemons that might run on a node in the worst case. Remember
+# to consider failure cases where nodes or availability zones are offline and their workloads must
+# be rescheduled.
+# A Rook cluster with all resource types might run these daemons on one node under failure:
+# 1 mon, 3 OSDs, 1 mgr, 1 MDS, 1 NFS server, 1 RGW, 1 rbdmirror, 1 cephfsmirror,
+# plus 1 csi provisioner and 1 csi plugin for all 3 CSI types: RBD, CSI, and CephFS -- 16 daemons
+daemonsPerNode: {{ .DaemonsPerNode }}
+
+# This is the time to wait for resources to change to the expected state. For example, for the test
+# web server to start, for test clients to become ready, or for test resources to be deleted. At
+# longest, this may need to reflect the time it takes for client pods to get address assignments and
+# then for each client to determine that its network connection is stable.
+# This should be at least 1 minute. 2 minutes or more is recommended.
+resourceTimeout: "{{ .ResourceTimeout }}"
+
+# The Nginx image which will be used for the web server and clients.
+nginxImage: "{{ .NginxImage }}"

pkg/daemon/multus/config_test.go

@@ -0,0 +1,55 @@
+/*
+Copyright 2023 The Rook Authors. All rights reserved.
+
+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 multus
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidationTestConfig_YAML(t *testing.T) {
+	emptyValidationTestConfig := &ValidationTestConfig{}
+
+	type test struct {
+		name   string
+		config *ValidationTestConfig
+	}
+	tests := []test{
+		{"empty config", emptyValidationTestConfig},
+		{"default config", NewDefaultValidationTestConfig()},
+		{"full config", &ValidationTestConfig{
+			Namespace: "my-rook", PublicNetwork: "my-pub", ClusterNetwork: "my-priv",
+			DaemonsPerNode: 12, ResourceTimeout: 2 * time.Minute, NginxImage: "myorg/nginx:latest",
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			y, err := tt.config.ToYAML()
+			assert.NoError(t, err)
+			// basic test to ensure config yamls have comments
+			assert.Contains(t, y, "# The intended namespace where the validation test will be run.")
+
+			c2, err := ValidationTestConfigFromYAML(y)
+			assert.NoError(t, err)
+
+			// config survives round trip to and from yaml
+			assert.Equal(t, tt.config, c2)
+		})
+	}
+}

pkg/daemon/multus/statemachine.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2023 The Rook Authors. All rights reserved.
+
+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 multus
 
 import (

pkg/daemon/multus/util.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2023 The Rook Authors. All rights reserved.
+
+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 multus
 
 import (

pkg/daemon/multus/validation.go

@@ -39,40 +39,10 @@ const (
 type ValidationTest struct {
 	Clientset kubernetes.Interface
 
-	// Namespace is the intended namespace where the validation test will be run. This is
-	// recommended to be where the Rook-Ceph cluster will be installed.
-	Namespace string
-
-	// PublicNetwork is the name of the Network Attachment Definition (NAD) which will
-	// be used for the Ceph cluster's public network. This should be a namespaced name in the form
-	// <namespace>/<name> if the NAD is defined in a different namespace from the cluster namespace.
-	PublicNetwork string
-
-	// ClusterNetwork is the name of the Network Attachment Definition (NAD) which will
-	// be used for the Ceph cluster's cluster network. This should be a namespaced name in the form
-	// <namespace>/<name> if the NAD is defined in a different namespace from the cluster namespace.
-	ClusterNetwork string
-
-	// DaemonsPerNode is the number of Multus-connected validation daemons that will run as part of
-	// the test. This should be set to the largest number of Ceph daemons that may run on a node in
-	// the worst case. Remember to consider failure cases where nodes or availability zones are
-	// offline and their workloads must be rescheduled.
-	DaemonsPerNode int
-
-	// ResourceTimeout is the time to wait for resources to change to the expected state. For
-	// example, for the test web server to start, for test clients to become ready, or for test
-	// resources to be deleted. At longest, this may need to reflect the time it takes for client
-	// pods to to pull images, get address assignments, and then for each client to determine that
-	// its network connection is stable.
-	//
-	// This should be at least 1 minute. 2 minutes or more is recommended.
-	ResourceTimeout time.Duration
-
-	// Logger an instance of the basic log implementation used by this library.
+	// The Logger will be used to render ongoing status by this library.
 	Logger Logger
 
-	//NginxImage is the Nginx image which will be used for the web server and clients.
-	NginxImage string
+	ValidationTestConfig
 }
 
 // ValidationTestResults contains results from a validation test.
@@ -355,20 +325,10 @@ func (vt *ValidationTest) Run(ctx context.Context) (*ValidationTestResults, erro
 		vt.Logger = &SimpleStderrLogger{}
 		vt.Logger.Infof("no logger was specified; using a simple stderr logger")
 	}
-	vt.Logger.Infof("starting multus validation test with the following config:")
-	vt.Logger.Infof("  namespace: %q", vt.Namespace)
-	vt.Logger.Infof("  public network: %q", vt.PublicNetwork)
-	vt.Logger.Infof("  cluster network: %q", vt.ClusterNetwork)
-	vt.Logger.Infof("  daemons per node: %d", vt.DaemonsPerNode)
-	vt.Logger.Infof("  resource timeout: %v", vt.ResourceTimeout)
-	vt.Logger.Infof("  nginx image: %q", vt.NginxImage)
+	vt.Logger.Infof("starting multus validation test with the following config:\n%s", &vt.ValidationTestConfig)
 
-	if vt.PublicNetwork == "" && vt.ClusterNetwork == "" {
-		return nil, fmt.Errorf("at least one of 'public network' and 'cluster network' must be specified")
-	}
-
-	if vt.NginxImage == "" {
-		return nil, fmt.Errorf("'nginx-image' must be specified")
+	if err := vt.ValidationTestConfig.Validate(); err != nil {
+		return nil, err
 	}
 
 	testResults := &ValidationTestResults{