Skip to content

Commit

Permalink
Fix kube.Exec to not include passed cmd two times (#789)
Browse files Browse the repository at this point in the history
* Fix kube.Exec to not include passed cmd two times

Per current implementation if we pass a command to
kube.Exec it is being passed twice in query parameter
to exec subresource to pod.
This PR tries to change that and we will now just have
the command once.

* Add test for kopia command

* Apply suggestions from code review

* Add nolint

Co-authored-by: Pavan Navarathna <pavan@kasten.io>
  • Loading branch information
viveksinghggits and pavannd1 committed Oct 12, 2020
1 parent 18ee57f commit 7ab0607
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 7 deletions.
4 changes: 1 addition & 3 deletions pkg/kube/exec.go
Expand Up @@ -70,9 +70,7 @@ func ExecWithOptions(kubeCli kubernetes.Interface, options ExecOptions) (string,
if len(options.ContainerName) != 0 {
req.Param("container", options.ContainerName)
}
for _, c := range options.Command {
req.Param("command", c)
}

req.VersionedParams(&v1.PodExecOptions{
Container: options.ContainerName,
Command: options.Command,
Expand Down
52 changes: 48 additions & 4 deletions pkg/kube/exec_test.go
Expand Up @@ -19,6 +19,7 @@ package kube
import (
"bytes"
"context"
"strings"
"time"

. "gopkg.in/check.v1"
Expand All @@ -36,6 +37,7 @@ type ExecSuite struct {
var _ = Suite(&ExecSuite{})

func (s *ExecSuite) SetUpSuite(c *C) {
ctx := context.Background()
var err error
s.cli, err = NewClient()
c.Assert(err, IsNil)
Expand All @@ -44,7 +46,7 @@ func (s *ExecSuite) SetUpSuite(c *C) {
GenerateName: "exectest-",
},
}
ns, err = s.cli.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{})
ns, err = s.cli.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{})
c.Assert(err, IsNil)
s.namespace = ns.Name
pod := &v1.Pod{
Expand All @@ -59,11 +61,11 @@ func (s *ExecSuite) SetUpSuite(c *C) {
},
},
}
s.pod, err = s.cli.CoreV1().Pods(s.namespace).Create(context.TODO(), pod, metav1.CreateOptions{})
s.pod, err = s.cli.CoreV1().Pods(s.namespace).Create(ctx, pod, metav1.CreateOptions{})
c.Assert(err, IsNil)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
ctxTimeout, cancel := context.WithTimeout(ctx, 5*time.Minute)
defer cancel()
c.Assert(WaitForPodReady(ctx, s.cli, s.namespace, s.pod.Name), IsNil)
c.Assert(WaitForPodReady(ctxTimeout, s.cli, s.namespace, s.pod.Name), IsNil)
s.pod, err = s.cli.CoreV1().Pods(s.namespace).Get(ctx, s.pod.Name, metav1.GetOptions{})
c.Assert(err, IsNil)
}
Expand Down Expand Up @@ -96,3 +98,45 @@ func (s *ExecSuite) TestExecEchoDefaultContainer(c *C) {
c.Assert(stdout, Equals, "badabing")
c.Assert(stderr, Equals, "")
}

func (s *ExecSuite) TestLSWithoutStdIn(c *C) {
cmd := []string{"ls", "-l", "/home"}
c.Assert(s.pod.Status.Phase, Equals, v1.PodRunning)
c.Assert(len(s.pod.Status.ContainerStatuses) > 0, Equals, true)
stdout, stderr, err := Exec(s.cli, s.pod.Namespace, s.pod.Name, "", cmd, nil)
c.Assert(err, IsNil)
c.Assert(stdout, Equals, "total 0")
c.Assert(stderr, Equals, "")
}

func (s *ExecSuite) TestKopiaCommand(c *C) {
ctx := context.Background()
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "kopia-pod",
Namespace: s.namespace,
},
Spec: v1.PodSpec{
Containers: []v1.Container{
v1.Container{
Name: "kanister-sidecar",
Image: "kanisterio/kanister-tools:0.37.0",
},
},
},
}
p, err := s.cli.CoreV1().Pods(s.namespace).Create(ctx, pod, metav1.CreateOptions{})
c.Assert(err, IsNil)
defer s.cli.CoreV1().Pods(s.namespace).Delete(ctx, pod.Name, metav1.DeleteOptions{}) // nolint: errcheck
ctxT, cancel := context.WithTimeout(ctx, 5*time.Minute)
defer cancel()
c.Assert(WaitForPodReady(ctxT, s.cli, s.namespace, p.Name), IsNil)
// up until now below is how we were used to run kopia commands
// "bash" "-c" "kopia repository create filesystem --path=$HOME/kopia_repo --password=newpass"
// but now we don't want `bash -c`
cmd := []string{"kopia", "repository", "create", "filesystem", "--path=$HOME/kopia_repo", "--password=newpass"}
stdout, stderr, err := Exec(s.cli, pod.Namespace, pod.Name, "", cmd, nil)
c.Assert(err, IsNil)
c.Assert(strings.Contains(stdout, "Policy for (global):"), Equals, true)
c.Assert(strings.Contains(stderr, "Initializing repository with:"), Equals, true)
}

0 comments on commit 7ab0607

Please sign in to comment.