Merge pull request #12414 from rook/mergify/bp/release-1.11/pr-12396

multus: add config file for validation tool (backport #12396)

.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{