Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set a request timeout on kubectl commands #360

Merged
merged 3 commits into from
Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/chart/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
3 changes: 2 additions & 1 deletion pkg/chart/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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),
}
}

Expand Down
54 changes: 29 additions & 25 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"path/filepath"
"reflect"
"strings"
"time"

"github.com/mitchellh/go-homedir"

Expand All @@ -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" {
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"os"
"path/filepath"
"testing"
"time"

"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/config/test_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@
"skip-missing-values": true,
"namespace": "default",
"release-label": "release",
"exclude-deprecated": true
"exclude-deprecated": true,
"kubectl-timeout": "120s"
}
1 change: 1 addition & 0 deletions pkg/config/test_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ skip-missing-values: true
namespace: default
release-label: release
exclude-deprecated: true
kubectl-timeout: 120s
76 changes: 57 additions & 19 deletions pkg/tool/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,46 @@ 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
// namespace and, eventually, the namespace itself are force-deleted.
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)
}

if k.getNamespace(namespace) {
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)
}

Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -126,16 +146,20 @@ 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
}

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
}
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -183,23 +211,30 @@ 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
}
return strings.Fields(pods), nil
}

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) {
Expand All @@ -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
}
Expand Down