From 048064130dbaedfd2bc04f2ed0a52f17245c9f7c Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 26 Jul 2022 23:16:31 +0800 Subject: [PATCH] Set a request timeout on kubectl commands (#360) * feat: add kubectl timeout value to configuration Signed-off-by: Scott Leggett * fix: set a request timeout on kubectl commands Simple kubectl commands such as get, describe, and logs should return quickly, but kubectl does not set a timeout on requests by default. This can mean kubectl hangs forever if the kubernetes API stops responding during a request, which is quite possible in CI environments where jobs may cancelled and kubernetes clusters torn down while ct is running. This change sets a configurable timeout on such kubectl commands, with a default value of 30s. Signed-off-by: Scott Leggett * chore: update integration test for new kubectl API Signed-off-by: Scott Leggett --- pkg/chart/chart.go | 2 +- pkg/chart/integration_test.go | 3 +- pkg/config/config.go | 54 +++++++++++++------------ pkg/config/config_test.go | 2 + pkg/config/test_config.json | 3 +- pkg/config/test_config.yaml | 1 + pkg/tool/kubectl.go | 76 ++++++++++++++++++++++++++--------- 7 files changed, 94 insertions(+), 47 deletions(-) diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index b222f39d..c66c8e0b 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -263,7 +263,7 @@ func NewTesting(config config.Configuration, extraSetArgs string) (Testing, erro config: config, helm: tool.NewHelm(procExec, extraArgs, strings.Fields(extraSetArgs)), git: tool.NewGit(procExec), - kubectl: tool.NewKubectl(procExec), + kubectl: tool.NewKubectl(procExec, config.KubectlTimeout), linter: tool.NewLinter(procExec), cmdExecutor: tool.NewCmdTemplateExecutor(procExec), accountValidator: tool.AccountValidator{}, diff --git a/pkg/chart/integration_test.go b/pkg/chart/integration_test.go index 63fa5d62..9d5f881f 100644 --- a/pkg/chart/integration_test.go +++ b/pkg/chart/integration_test.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" "testing" + "time" "github.com/helm/chart-testing/v3/pkg/config" "github.com/helm/chart-testing/v3/pkg/exec" @@ -41,7 +42,7 @@ func newTestingHelmIntegration(cfg config.Configuration, extraSetArgs string) Te accountValidator: fakeAccountValidator{}, linter: fakeMockLinter, helm: tool.NewHelm(procExec, extraArgs, strings.Fields(extraSetArgs)), - kubectl: tool.NewKubectl(procExec), + kubectl: tool.NewKubectl(procExec, 30*time.Second), } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 33f2acee..7113761d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -20,6 +20,7 @@ import ( "path/filepath" "reflect" "strings" + "time" "github.com/mitchellh/go-homedir" @@ -41,36 +42,39 @@ var ( ) type Configuration struct { - Remote string `mapstructure:"remote"` - TargetBranch string `mapstructure:"target-branch"` - Since string `mapstructure:"since"` - BuildId string `mapstructure:"build-id"` - LintConf string `mapstructure:"lint-conf"` - ChartYamlSchema string `mapstructure:"chart-yaml-schema"` - ValidateMaintainers bool `mapstructure:"validate-maintainers"` - ValidateChartSchema bool `mapstructure:"validate-chart-schema"` - ValidateYaml bool `mapstructure:"validate-yaml"` - AdditionalCommands []string `mapstructure:"additional-commands"` - CheckVersionIncrement bool `mapstructure:"check-version-increment"` - ProcessAllCharts bool `mapstructure:"all"` - Charts []string `mapstructure:"charts"` - ChartRepos []string `mapstructure:"chart-repos"` - ChartDirs []string `mapstructure:"chart-dirs"` - ExcludedCharts []string `mapstructure:"excluded-charts"` - HelmExtraArgs string `mapstructure:"helm-extra-args"` - HelmRepoExtraArgs []string `mapstructure:"helm-repo-extra-args"` - HelmDependencyExtraArgs []string `mapstructure:"helm-dependency-extra-args"` - Debug bool `mapstructure:"debug"` - Upgrade bool `mapstructure:"upgrade"` - SkipMissingValues bool `mapstructure:"skip-missing-values"` - Namespace string `mapstructure:"namespace"` - ReleaseLabel string `mapstructure:"release-label"` - ExcludeDeprecated bool `mapstructure:"exclude-deprecated"` + Remote string `mapstructure:"remote"` + TargetBranch string `mapstructure:"target-branch"` + Since string `mapstructure:"since"` + BuildId string `mapstructure:"build-id"` + LintConf string `mapstructure:"lint-conf"` + ChartYamlSchema string `mapstructure:"chart-yaml-schema"` + ValidateMaintainers bool `mapstructure:"validate-maintainers"` + ValidateChartSchema bool `mapstructure:"validate-chart-schema"` + ValidateYaml bool `mapstructure:"validate-yaml"` + AdditionalCommands []string `mapstructure:"additional-commands"` + CheckVersionIncrement bool `mapstructure:"check-version-increment"` + ProcessAllCharts bool `mapstructure:"all"` + Charts []string `mapstructure:"charts"` + ChartRepos []string `mapstructure:"chart-repos"` + ChartDirs []string `mapstructure:"chart-dirs"` + ExcludedCharts []string `mapstructure:"excluded-charts"` + HelmExtraArgs string `mapstructure:"helm-extra-args"` + HelmRepoExtraArgs []string `mapstructure:"helm-repo-extra-args"` + HelmDependencyExtraArgs []string `mapstructure:"helm-dependency-extra-args"` + Debug bool `mapstructure:"debug"` + Upgrade bool `mapstructure:"upgrade"` + SkipMissingValues bool `mapstructure:"skip-missing-values"` + Namespace string `mapstructure:"namespace"` + ReleaseLabel string `mapstructure:"release-label"` + ExcludeDeprecated bool `mapstructure:"exclude-deprecated"` + KubectlTimeout time.Duration `mapstructure:"kubectl-timeout"` } func LoadConfiguration(cfgFile string, cmd *cobra.Command, printConfig bool) (*Configuration, error) { v := viper.New() + v.SetDefault("kubectl-timeout", time.Duration(30*time.Second)) + cmd.Flags().VisitAll(func(flag *flag.Flag) { flagName := flag.Name if flagName != "config" && flagName != "help" { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 66e4a287..00871925 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -57,6 +58,7 @@ func loadAndAssertConfigFromFile(t *testing.T, configFile string) { require.Equal(t, "default", cfg.Namespace) require.Equal(t, "release", cfg.ReleaseLabel) require.Equal(t, true, cfg.ExcludeDeprecated) + require.Equal(t, 120*time.Second, cfg.KubectlTimeout) } func Test_findConfigFile(t *testing.T) { diff --git a/pkg/config/test_config.json b/pkg/config/test_config.json index ba5659cb..861f8b78 100644 --- a/pkg/config/test_config.json +++ b/pkg/config/test_config.json @@ -29,5 +29,6 @@ "skip-missing-values": true, "namespace": "default", "release-label": "release", - "exclude-deprecated": true + "exclude-deprecated": true, + "kubectl-timeout": "120s" } diff --git a/pkg/config/test_config.yaml b/pkg/config/test_config.yaml index 5af7bf9a..56f48477 100644 --- a/pkg/config/test_config.yaml +++ b/pkg/config/test_config.yaml @@ -25,3 +25,4 @@ skip-missing-values: true namespace: default release-label: release exclude-deprecated: true +kubectl-timeout: 120s diff --git a/pkg/tool/kubectl.go b/pkg/tool/kubectl.go index 13fa70d6..d2021e58 100644 --- a/pkg/tool/kubectl.go +++ b/pkg/tool/kubectl.go @@ -14,19 +14,23 @@ import ( ) type Kubectl struct { - exec exec.ProcessExecutor + exec exec.ProcessExecutor + timeout time.Duration } -func NewKubectl(exec exec.ProcessExecutor) Kubectl { +func NewKubectl(exec exec.ProcessExecutor, timeout time.Duration) Kubectl { return Kubectl{ - exec: exec, + exec: exec, + timeout: timeout, } } // CreateNamespace creates a new namespace with the given name. func (k Kubectl) CreateNamespace(namespace string) error { fmt.Printf("Creating namespace '%s'...\n", namespace) - return k.exec.RunProcess("kubectl", "create", "namespace", namespace) + return k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "create", "namespace", namespace) } // DeleteNamespace deletes the specified namespace. If the namespace does not terminate within 120s, pods running in the @@ -34,7 +38,10 @@ func (k Kubectl) CreateNamespace(namespace string) error { func (k Kubectl) DeleteNamespace(namespace string) { fmt.Printf("Deleting namespace '%s'...\n", namespace) timeoutSec := "180s" - if err := k.exec.RunProcess("kubectl", "delete", "namespace", namespace, "--timeout", timeoutSec); err != nil { + err := k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "delete", "namespace", namespace, "--timeout", timeoutSec) + if err != nil { fmt.Printf("Namespace '%s' did not terminate after %s.\n", namespace, timeoutSec) } @@ -42,7 +49,11 @@ func (k Kubectl) DeleteNamespace(namespace string) { fmt.Printf("Namespace '%s' did not terminate after %s.\n", namespace, timeoutSec) fmt.Println("Force-deleting everything...") - if err := k.exec.RunProcess("kubectl", "delete", "all", "--namespace", namespace, "--all", "--force", "--grace-period=0"); err != nil { + err = k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "delete", "all", "--namespace", namespace, "--all", "--force", + "--grace-period=0") + if err != nil { fmt.Printf("Error deleting everything in the namespace %v: %v", namespace, err) } @@ -59,7 +70,9 @@ func (k Kubectl) DeleteNamespace(namespace string) { func (k Kubectl) forceNamespaceDeletion(namespace string) error { // Getting the namespace json to remove the finalizer - cmdOutput, err := k.exec.RunProcessAndCaptureOutput("kubectl", "get", "namespace", namespace, "--output=json") + cmdOutput, err := k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "namespace", namespace, "--output=json") if err != nil { fmt.Println("Error getting namespace json:", err) return err @@ -111,13 +124,20 @@ func (k Kubectl) forceNamespaceDeletion(namespace string) error { time.Sleep(5 * time.Second) // Check again - if _, err := k.exec.RunProcessAndCaptureOutput("kubectl", "get", "namespace", namespace); err != nil { + _, err = k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "namespace", namespace) + if err != nil { fmt.Printf("Namespace '%s' terminated.\n", namespace) return nil } fmt.Printf("Force-deleting namespace '%s'...\n", namespace) - if err := k.exec.RunProcess("kubectl", "delete", "namespace", namespace, "--force", "--grace-period=0", "--ignore-not-found=true"); err != nil { + err = k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "delete", "namespace", namespace, "--force", "--grace-period=0", + "--ignore-not-found=true") + if err != nil { fmt.Println("Error deleting namespace:", err) return err } @@ -126,8 +146,10 @@ func (k Kubectl) forceNamespaceDeletion(namespace string) error { } func (k Kubectl) WaitForDeployments(namespace string, selector string) error { - output, err := k.exec.RunProcessAndCaptureOutput( - "kubectl", "get", "deployments", "--namespace", namespace, "--selector", selector, "--output", "jsonpath={.items[*].metadata.name}") + output, err := k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "deployments", "--namespace", namespace, "--selector", selector, + "--output", "jsonpath={.items[*].metadata.name}") if err != nil { return err } @@ -135,7 +157,9 @@ func (k Kubectl) WaitForDeployments(namespace string, selector string) error { deployments := strings.Fields(output) for _, deployment := range deployments { deployment = strings.Trim(deployment, "'") - err := k.exec.RunProcess("kubectl", "rollout", "status", "deployment", deployment, "--namespace", namespace) + err = k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "rollout", "status", "deployment", deployment, "--namespace", namespace) if err != nil { return err } @@ -145,7 +169,9 @@ func (k Kubectl) WaitForDeployments(namespace string, selector string) error { // // Just after rollout, pods from the previous deployment revision may still be in a // terminating state. - unavailable, err := k.exec.RunProcessAndCaptureOutput("kubectl", "get", "deployment", deployment, "--namespace", namespace, "--output", + unavailable, err := k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "deployment", deployment, "--namespace", namespace, "--output", `jsonpath={.status.unavailableReplicas}`) if err != nil { return err @@ -159,7 +185,9 @@ func (k Kubectl) WaitForDeployments(namespace string, selector string) error { } func (k Kubectl) GetPodsforDeployment(namespace string, deployment string) ([]string, error) { - jsonString, _ := k.exec.RunProcessAndCaptureOutput("kubectl", "get", "deployment", deployment, "--namespace", namespace, "--output=json") + jsonString, _ := k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "deployment", deployment, "--namespace", namespace, "--output=json") var deploymentMap map[string]interface{} err := json.Unmarshal([]byte(jsonString), &deploymentMap) if err != nil { @@ -183,7 +211,8 @@ func (k Kubectl) GetPodsforDeployment(namespace string, deployment string) ([]st func (k Kubectl) GetPods(args ...string) ([]string, error) { kubectlArgs := []string{"get", "pods"} kubectlArgs = append(kubectlArgs, args...) - pods, err := k.exec.RunProcessAndCaptureOutput("kubectl", kubectlArgs) + pods, err := k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), kubectlArgs) if err != nil { return nil, err } @@ -191,15 +220,21 @@ func (k Kubectl) GetPods(args ...string) ([]string, error) { } func (k Kubectl) GetEvents(namespace string) error { - return k.exec.RunProcess("kubectl", "get", "events", "--output", "wide", "--namespace", namespace) + return k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "events", "--output", "wide", "--namespace", namespace) } func (k Kubectl) DescribePod(namespace string, pod string) error { - return k.exec.RunProcess("kubectl", "describe", "pod", pod, "--namespace", namespace) + return k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "describe", "pod", pod, "--namespace", namespace) } func (k Kubectl) Logs(namespace string, pod string, container string) error { - return k.exec.RunProcess("kubectl", "logs", pod, "--namespace", namespace, "--container", container) + return k.exec.RunProcess("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "logs", pod, "--namespace", namespace, "--container", container) } func (k Kubectl) GetInitContainers(namespace string, pod string) ([]string, error) { @@ -211,7 +246,10 @@ func (k Kubectl) GetContainers(namespace string, pod string) ([]string, error) { } func (k Kubectl) getNamespace(namespace string) bool { - if _, err := k.exec.RunProcessAndCaptureOutput("kubectl", "get", "namespace", namespace); err != nil { + _, err := k.exec.RunProcessAndCaptureOutput("kubectl", + fmt.Sprintf("--request-timeout=%s", k.timeout), + "get", "namespace", namespace) + if err != nil { fmt.Printf("Namespace '%s' terminated.\n", namespace) return false }