Skip to content

Commit

Permalink
fix custom label for kubectl strategy (#281)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
rsvihladremio authored Jan 9, 2025
1 parent bbcb421 commit d5a0c02
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 10 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 7 additions & 4 deletions cmd/root/kubectl/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 7 additions & 5 deletions cmd/root/kubectl/kubectl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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])
}
Expand Down Expand Up @@ -148,6 +149,7 @@ func TestKubectCopyFromWindowsHost(t *testing.T) {
}
k := CliK8sActions{
cli: cli,
labelSelector: "role=abc",
kubectlPath: "kubectl",
namespace: namespace,
k8sContext: k8sContext,
Expand Down

0 comments on commit d5a0c02

Please sign in to comment.