fix(kumactl): make check timeout configurable

tests were flaky because we were modifying a global variable. Extracted this global var in a configuration parameter as it may be useful anyway Signed-off-by: Charly Molter <charly.molter@konghq.com>
kumahq · Jan 17, 2022 · 4d06c2e · 4d06c2e
1 parent 624bf49
commit 4d06c2e
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 15 deletions.
diff --git a/app/kumactl/cmd/completion/testdata/bash.golden b/app/kumactl/cmd/completion/testdata/bash.golden
@@ -552,6 +552,10 @@ _kumactl_config_control-planes_add()
     two_word_flags+=("--ca-cert-file")
     local_nonpersistent_flags+=("--ca-cert-file")
     local_nonpersistent_flags+=("--ca-cert-file=")
+    flags+=("--check-timeout=")
+    two_word_flags+=("--check-timeout")
+    local_nonpersistent_flags+=("--check-timeout")
+    local_nonpersistent_flags+=("--check-timeout=")
     flags+=("--client-cert-file=")
     two_word_flags+=("--client-cert-file")
     local_nonpersistent_flags+=("--client-cert-file")

diff --git a/app/kumactl/cmd/config/config_control_planes_add.go b/app/kumactl/cmd/config/config_control_planes_add.go
@@ -2,6 +2,7 @@ package config
 
 import (
 	net_url "net/url"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -24,6 +25,7 @@ type controlPlaneAddArgs struct {
 	headers        map[string]string
 	authType       string
 	authConf       map[string]string
+	checkTimeout   time.Duration
 }
 
 func newConfigControlPlanesAddCmd(pctx *kumactl_cmd.RootContext) *cobra.Command {
@@ -60,7 +62,7 @@ func newConfigControlPlanesAddCmd(pctx *kumactl_cmd.RootContext) *cobra.Command
 				cp.Coordinates.ApiServer.Headers = append(cp.Coordinates.ApiServer.Headers, header)
 			}
 			cfg := pctx.Config()
-			if err := config.ValidateCpCoordinates(cp); err != nil {
+			if err := config.ValidateCpCoordinates(cp, args.checkTimeout); err != nil {
 				return err
 			}
 			if !cfg.AddControlPlane(cp, args.overwrite) {
@@ -95,6 +97,7 @@ func newConfigControlPlanesAddCmd(pctx *kumactl_cmd.RootContext) *cobra.Command
 	cmd.Flags().StringToStringVar(&args.headers, "headers", args.headers, "add these headers while communicating to control plane, format key=value")
 	cmd.Flags().StringVar(&args.authType, "auth-type", args.authType, `authentication type (for example: "tokens")`)
 	cmd.Flags().StringToStringVar(&args.authConf, "auth-conf", args.authConf, "authentication configuration for defined authentication type format key=value")
+	cmd.Flags().DurationVar(&args.checkTimeout, "check-timeout", time.Second * 5, "the timeout of the first api call to check connectivity to the api server (probably never needs to be updated)")
 	return cmd
 }
 

diff --git a/app/kumactl/cmd/config/config_control_planes_add_test.go b/app/kumactl/cmd/config/config_control_planes_add_test.go
@@ -18,7 +18,6 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/spf13/cobra"
 
-	"github.com/kumahq/kuma/app/kumactl/pkg/config"
 	"github.com/kumahq/kuma/pkg/api-server/types"
 	"github.com/kumahq/kuma/pkg/util/test"
 	"github.com/kumahq/kuma/pkg/version"
@@ -118,22 +117,17 @@ var _ = Describe("kumactl config control-planes add", func() {
 
 		It("should fail when CP timeouts", func() {
 			// setup
-			currentTimeout := config.DefaultApiServerTimeout
-			config.DefaultApiServerTimeout = 10 * time.Millisecond
-			defer func() {
-				config.DefaultApiServerTimeout = currentTimeout
-			}()
-			timeout := config.DefaultApiServerTimeout * 5 // so we are sure we exceed the timeout
 			server, port := setupCpServer(func(writer http.ResponseWriter, req *http.Request) {
-				time.Sleep(timeout)
+				time.Sleep(time.Millisecond * 20)
 			})
 			defer server.Close()
 
 			// given
 			rootCmd.SetArgs([]string{"--config-file", configFile.Name(),
 				"config", "control-planes", "add",
 				"--name", "example",
-				"--address", fmt.Sprintf("http://localhost:%d", port)})
+				"--address", fmt.Sprintf("http://localhost:%d", port),
+				"--check-timeout", "10ms"})
 			// when
 			err := rootCmd.Execute()
 

diff --git a/app/kumactl/pkg/config/validate.go b/app/kumactl/pkg/config/validate.go
@@ -14,16 +14,13 @@ import (
 	"github.com/kumahq/kuma/pkg/version"
 )
 
-// overridden by tests
-var DefaultApiServerTimeout = 5 * time.Second
-
-func ValidateCpCoordinates(cp *kumactl_config.ControlPlane) error {
+func ValidateCpCoordinates(cp *kumactl_config.ControlPlane, timeout time.Duration) error {
 	req, err := http.NewRequest("GET", cp.Coordinates.ApiServer.Url, nil)
 	if err != nil {
 		return errors.Wrap(err, "could not construct the request")
 	}
 	client := http.Client{
-		Timeout:   DefaultApiServerTimeout,
+		Timeout:   timeout,
 		Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}},
 	}
 	for _, h := range cp.Coordinates.ApiServer.Headers {