From d5a0c021469ea3ea4034d6eada9809fa4d5a0376 Mon Sep 17 00:00:00 2001 From: Ryan Svihla <105286284+rsvihladremio@users.noreply.github.com> Date: Thu, 9 Jan 2025 19:40:50 +0100 Subject: [PATCH] fix custom label for kubectl strategy (#281) - kubectl strategy had hard-coded the default role of role=dremio-cluster-pod, this is now fixed and uses the same kubectl args that is used by the kubernetes api strategy fixes #280 - added test around custom kubectl -l flag --- CHANGELOG.md | 7 +++++++ cmd/root.go | 2 +- cmd/root/kubectl/kubectl.go | 11 +++++++---- cmd/root/kubectl/kubectl_test.go | 12 +++++++----- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 932de48..5a1baaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [3.3.1] - 2024-01-09 + +### Fixed + +* -l selector was not being applied when kubectl was available + ## [3.3.0] - 2024-01-03 ### Added @@ -838,6 +844,7 @@ someone has added the PAT which is always available - able to capture logs, configuration and diagnostic data from Dremio clusters deployed on Kubernetes and on-prem +[3.3.1]: https://github.com/dremio/dremio-diagnostic-collector/compare/v3.3.0...v3.3.1 [3.3.0]: https://github.com/dremio/dremio-diagnostic-collector/compare/v3.2.8...v3.3.0 [3.2.8]: https://github.com/dremio/dremio-diagnostic-collector/compare/v3.2.7...v3.2.8 [3.2.7]: https://github.com/dremio/dremio-diagnostic-collector/compare/v3.2.6...v3.2.7 diff --git a/cmd/root.go b/cmd/root.go index 7fc95e8..70713cd 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -183,7 +183,7 @@ func RemoteCollect(collectionArgs collection.Args, sshArgs ssh.Args, kubeArgs ku return err } if !disableKubeCtl { - potentialStrategy, err := kubectl.NewKubectlK8sActions(hook, kubeArgs.Namespace, kubeArgs.K8SContext) + potentialStrategy, err := kubectl.NewKubectlK8sActions(hook, kubeArgs) if err != nil { simplelog.Warningf("kubectl not available failling back to kubeapi: %v", err) } else { diff --git a/cmd/root/kubectl/kubectl.go b/cmd/root/kubectl/kubectl.go index 5fd1857..5af50f1 100644 --- a/cmd/root/kubectl/kubectl.go +++ b/cmd/root/kubectl/kubectl.go @@ -27,6 +27,7 @@ import ( "time" "github.com/dremio/dremio-diagnostic-collector/v3/cmd/root/cli" + "github.com/dremio/dremio-diagnostic-collector/v3/cmd/root/kubernetes" "github.com/dremio/dremio-diagnostic-collector/v3/pkg/consoleprint" "github.com/dremio/dremio-diagnostic-collector/v3/pkg/shutdown" "github.com/dremio/dremio-diagnostic-collector/v3/pkg/simplelog" @@ -38,13 +39,13 @@ type KubeArgs struct { // NewKubectlK8sActions is the only supported way to initialize the KubectlK8sActions struct // one must pass the path to kubectl -func NewKubectlK8sActions(hook shutdown.CancelHook, namespace, k8sContext string) (*CliK8sActions, error) { +func NewKubectlK8sActions(hook shutdown.CancelHook, kubeArgs kubernetes.KubeArgs) (*CliK8sActions, error) { kubectl, err := exec.LookPath("kubectl") if err != nil { return &CliK8sActions{}, fmt.Errorf("no kubectl found: %w", err) } cliInstance := cli.NewCli(hook) - + k8sContext := kubeArgs.K8SContext if k8sContext == "" { k8sContextRaw, err := cliInstance.Execute(false, kubectl, "config", "current-context") if err != nil { @@ -59,7 +60,8 @@ func NewKubectlK8sActions(hook shutdown.CancelHook, namespace, k8sContext string return &CliK8sActions{ cli: cliInstance, kubectlPath: kubectl, - namespace: namespace, + labelSelector: kubeArgs.LabelSelector, + namespace: kubeArgs.Namespace, k8sContext: k8sContext, pidHosts: make(map[string]string), retriesEnabled: retriesEnabled, @@ -69,6 +71,7 @@ func NewKubectlK8sActions(hook shutdown.CancelHook, namespace, k8sContext string // CliK8sActions provides a way to collect and copy files using kubectl type CliK8sActions struct { cli cli.CmdExecutor + labelSelector string kubectlPath string namespace string k8sContext string @@ -205,7 +208,7 @@ func (c *CliK8sActions) GetCoordinators() (podName []string, err error) { } func (c *CliK8sActions) SearchPods(compare func(container string) bool) (podName []string, err error) { - out, err := c.cli.Execute(false, c.kubectlPath, "get", "pods", "-n", c.namespace, "--context", c.k8sContext, "-l", "role=dremio-cluster-pod", "-o", "name") + out, err := c.cli.Execute(false, c.kubectlPath, "get", "pods", "-n", c.namespace, "--context", c.k8sContext, "-l", c.labelSelector, "-o", "name") if err != nil { return []string{}, err } diff --git a/cmd/root/kubectl/kubectl_test.go b/cmd/root/kubectl/kubectl_test.go index ff47728..bf340f5 100644 --- a/cmd/root/kubectl/kubectl_test.go +++ b/cmd/root/kubectl/kubectl_test.go @@ -75,10 +75,11 @@ func TestKubectlSearch(t *testing.T) { StoredErrors: []error{nil, nil, nil, nil}, } k := CliK8sActions{ - cli: cli, - kubectlPath: "kubectl", - namespace: namespace, - k8sContext: k8sContext, + cli: cli, + labelSelector: "role=dremio-pods", + kubectlPath: "kubectl", + namespace: namespace, + k8sContext: k8sContext, } podNames, err := k.GetCoordinators() if err != nil { @@ -95,7 +96,7 @@ func TestKubectlSearch(t *testing.T) { if len(calls) != 4 { t.Errorf("expected 4 call but got %v", len(calls)) } - expectedCall := []string{"kubectl", "get", "pods", "-n", namespace, "--context", k8sContext, "-l", "role=dremio-cluster-pod", "-o", "name"} + expectedCall := []string{"kubectl", "get", "pods", "-n", namespace, "--context", k8sContext, "-l", "role=dremio-pods", "-o", "name"} if !reflect.DeepEqual(calls[0], expectedCall) { t.Errorf("\nexpected call\n%v\nbut got\n%v", expectedCall, calls[0]) } @@ -148,6 +149,7 @@ func TestKubectCopyFromWindowsHost(t *testing.T) { } k := CliK8sActions{ cli: cli, + labelSelector: "role=abc", kubectlPath: "kubectl", namespace: namespace, k8sContext: k8sContext,