From 352fcb3b5b69e221c547cb383eb196ef46aa4f5a Mon Sep 17 00:00:00 2001 From: David Przybilla Date: Mon, 11 Jul 2022 15:11:12 +0900 Subject: [PATCH] Teardown logic shared for demo/sandbox cmds (#329) --- flytectl/cmd/demo/start.go | 1 - flytectl/cmd/demo/teardown.go | 37 ++------------------- flytectl/cmd/demo/teardown_test.go | 13 ++++++-- flytectl/cmd/sandbox/start.go | 1 - flytectl/cmd/sandbox/teardown.go | 38 ++------------------- flytectl/cmd/sandbox/teardown_test.go | 40 +++------------------- flytectl/pkg/sandbox/teardown.go | 39 ++++++++++++++++++++++ flytectl/pkg/sandbox/teardown_test.go | 48 +++++++++++++++++++++++++++ 8 files changed, 107 insertions(+), 110 deletions(-) create mode 100644 flytectl/pkg/sandbox/teardown.go create mode 100644 flytectl/pkg/sandbox/teardown_test.go diff --git a/flytectl/cmd/demo/start.go b/flytectl/cmd/demo/start.go index 247111ffca..a1e3113398 100644 --- a/flytectl/cmd/demo/start.go +++ b/flytectl/cmd/demo/start.go @@ -72,7 +72,6 @@ eg : for passing multiple environment variables Usage ` - demoContextName = "flyte-sandbox" ) func startDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error { diff --git a/flytectl/cmd/demo/teardown.go b/flytectl/cmd/demo/teardown.go index bd98b22e37..399a904ab7 100644 --- a/flytectl/cmd/demo/teardown.go +++ b/flytectl/cmd/demo/teardown.go @@ -2,17 +2,12 @@ package demo import ( "context" - "fmt" - "github.com/flyteorg/flytectl/pkg/configutil" + "github.com/flyteorg/flytectl/pkg/sandbox" "github.com/flyteorg/flytectl/pkg/docker" - "github.com/docker/docker/api/types" - "github.com/enescakir/emoji" - cmdCore "github.com/flyteorg/flytectl/cmd/core" - "github.com/flyteorg/flytectl/pkg/k8s" ) const ( @@ -33,33 +28,5 @@ func teardownDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.Comm if err != nil { return err } - - return tearDownDemo(ctx, cli) -} - -func tearDownDemo(ctx context.Context, cli docker.Docker) error { - c, err := docker.GetSandbox(ctx, cli) - if err != nil { - return err - } - if c != nil { - if err := cli.ContainerRemove(context.Background(), c.ID, types.ContainerRemoveOptions{ - Force: true, - }); err != nil { - return err - } - } - if err := configutil.ConfigCleanup(); err != nil { - fmt.Printf("Config cleanup failed. Which Failed due to %v \n ", err) - } - if err := removeDemoKubeContext(); err != nil { - fmt.Printf("Kubecontext cleanup failed. Which Failed due to %v \n ", err) - } - fmt.Printf("%v %v Demo cluster is removed successfully. \n", emoji.Broom, emoji.Broom) - return nil -} - -func removeDemoKubeContext() error { - k8sCtxMgr := k8s.NewK8sContextManager() - return k8sCtxMgr.RemoveContext(demoContextName) + return sandbox.Teardown(ctx, cli) } diff --git a/flytectl/cmd/demo/teardown_test.go b/flytectl/cmd/demo/teardown_test.go index 7741272a2b..34a25f23f5 100644 --- a/flytectl/cmd/demo/teardown_test.go +++ b/flytectl/cmd/demo/teardown_test.go @@ -12,6 +12,7 @@ import ( "github.com/flyteorg/flytectl/pkg/docker/mocks" "github.com/flyteorg/flytectl/pkg/k8s" k8sMocks "github.com/flyteorg/flytectl/pkg/k8s/mocks" + "github.com/flyteorg/flytectl/pkg/sandbox" "github.com/flyteorg/flytectl/pkg/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -36,7 +37,7 @@ func TestTearDownFunc(t *testing.T) { mockK8sContextMgr := &k8sMocks.ContextOps{} k8s.ContextMgr = mockK8sContextMgr mockK8sContextMgr.OnRemoveContextMatch(mock.Anything).Return(nil) - err := tearDownDemo(ctx, mockDocker) + err := sandbox.Teardown(ctx, mockDocker) assert.Nil(t, err) }) t.Run("Error", func(t *testing.T) { @@ -44,7 +45,15 @@ func TestTearDownFunc(t *testing.T) { mockDocker := &mocks.Docker{} mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) mockDocker.OnContainerRemove(ctx, mock.Anything, types.ContainerRemoveOptions{Force: true}).Return(fmt.Errorf("err")) - err := tearDownDemo(ctx, mockDocker) + err := sandbox.Teardown(ctx, mockDocker) + assert.NotNil(t, err) + }) + + t.Run("Error", func(t *testing.T) { + ctx := context.Background() + mockDocker := &mocks.Docker{} + mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(nil, fmt.Errorf("err")) + err := sandbox.Teardown(ctx, mockDocker) assert.NotNil(t, err) }) diff --git a/flytectl/cmd/sandbox/start.go b/flytectl/cmd/sandbox/start.go index d409637032..1996a7f17d 100644 --- a/flytectl/cmd/sandbox/start.go +++ b/flytectl/cmd/sandbox/start.go @@ -76,7 +76,6 @@ eg : for passing multiple environment variables Usage ` - sandboxContextName = "flyte-sandbox" ) func startSandboxCluster(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error { diff --git a/flytectl/cmd/sandbox/teardown.go b/flytectl/cmd/sandbox/teardown.go index 9280e303a5..4e209a19c1 100644 --- a/flytectl/cmd/sandbox/teardown.go +++ b/flytectl/cmd/sandbox/teardown.go @@ -2,17 +2,11 @@ package sandbox import ( "context" - "fmt" - - "github.com/flyteorg/flytectl/pkg/configutil" "github.com/flyteorg/flytectl/pkg/docker" - - "github.com/docker/docker/api/types" - "github.com/enescakir/emoji" + "github.com/flyteorg/flytectl/pkg/sandbox" cmdCore "github.com/flyteorg/flytectl/cmd/core" - "github.com/flyteorg/flytectl/pkg/k8s" ) const ( @@ -33,33 +27,5 @@ func teardownSandboxCluster(ctx context.Context, args []string, cmdCtx cmdCore.C if err != nil { return err } - - return tearDownSandbox(ctx, cli) -} - -func tearDownSandbox(ctx context.Context, cli docker.Docker) error { - c, err := docker.GetSandbox(ctx, cli) - if err != nil { - return err - } - if c != nil { - if err := cli.ContainerRemove(context.Background(), c.ID, types.ContainerRemoveOptions{ - Force: true, - }); err != nil { - return err - } - } - if err := configutil.ConfigCleanup(); err != nil { - fmt.Printf("Config cleanup failed. Which Failed due to %v \n ", err) - } - if err := removeSandboxKubeContext(); err != nil { - fmt.Printf("Kubecontext cleanup failed. Which Failed due to %v \n ", err) - } - fmt.Printf("%v %v Sandbox cluster is removed successfully. \n", emoji.Broom, emoji.Broom) - return nil -} - -func removeSandboxKubeContext() error { - k8sCtxMgr := k8s.NewK8sContextManager() - return k8sCtxMgr.RemoveContext(sandboxContextName) + return sandbox.Teardown(ctx, cli) } diff --git a/flytectl/cmd/sandbox/teardown_test.go b/flytectl/cmd/sandbox/teardown_test.go index 0664342979..63509c22ec 100644 --- a/flytectl/cmd/sandbox/teardown_test.go +++ b/flytectl/cmd/sandbox/teardown_test.go @@ -1,8 +1,6 @@ package sandbox import ( - "context" - "fmt" "testing" "github.com/docker/docker/api/types" @@ -17,40 +15,8 @@ import ( "github.com/stretchr/testify/mock" ) -var containers []types.Container - -func TestTearDownFunc(t *testing.T) { - container1 := types.Container{ - ID: "FlyteSandboxClusterName", - Names: []string{ - docker.FlyteSandboxClusterName, - }, - } - containers = append(containers, container1) - - t.Run("Success", func(t *testing.T) { - ctx := context.Background() - mockDocker := &mocks.Docker{} - mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) - mockDocker.OnContainerRemove(ctx, mock.Anything, types.ContainerRemoveOptions{Force: true}).Return(nil) - mockK8sContextMgr := &k8sMocks.ContextOps{} - k8s.ContextMgr = mockK8sContextMgr - mockK8sContextMgr.OnRemoveContextMatch(mock.Anything).Return(nil) - err := tearDownSandbox(ctx, mockDocker) - assert.Nil(t, err) - }) - t.Run("Error", func(t *testing.T) { - ctx := context.Background() - mockDocker := &mocks.Docker{} - mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) - mockDocker.OnContainerRemove(ctx, mock.Anything, types.ContainerRemoveOptions{Force: true}).Return(fmt.Errorf("err")) - err := tearDownSandbox(ctx, mockDocker) - assert.NotNil(t, err) - }) - -} - func TestTearDownClusterFunc(t *testing.T) { + var containers []types.Container _ = util.SetupFlyteDir() _ = util.WriteIntoFile([]byte("data"), configutil.FlytectlConfig) s := testutils.Setup() @@ -58,6 +24,10 @@ func TestTearDownClusterFunc(t *testing.T) { mockDocker := &mocks.Docker{} mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) mockDocker.OnContainerRemove(ctx, mock.Anything, types.ContainerRemoveOptions{Force: true}).Return(nil) + mockK8sContextMgr := &k8sMocks.ContextOps{} + mockK8sContextMgr.OnRemoveContext(mock.Anything).Return(nil) + k8s.ContextMgr = mockK8sContextMgr + docker.Client = mockDocker err := teardownSandboxCluster(ctx, []string{}, s.CmdCtx) assert.Nil(t, err) diff --git a/flytectl/pkg/sandbox/teardown.go b/flytectl/pkg/sandbox/teardown.go new file mode 100644 index 0000000000..633d026833 --- /dev/null +++ b/flytectl/pkg/sandbox/teardown.go @@ -0,0 +1,39 @@ +package sandbox + +import ( + "context" + "fmt" + + "github.com/docker/docker/api/types" + "github.com/enescakir/emoji" + "github.com/flyteorg/flytectl/pkg/configutil" + "github.com/flyteorg/flytectl/pkg/docker" + "github.com/flyteorg/flytectl/pkg/k8s" +) + +func Teardown(ctx context.Context, cli docker.Docker) error { + c, err := docker.GetSandbox(ctx, cli) + if err != nil { + return err + } + if c != nil { + if err := cli.ContainerRemove(context.Background(), c.ID, types.ContainerRemoveOptions{ + Force: true, + }); err != nil { + return err + } + } + if err := configutil.ConfigCleanup(); err != nil { + fmt.Printf("Config cleanup failed. Which Failed due to %v \n ", err) + } + if err := removeSandboxKubeContext(); err != nil { + fmt.Printf("Kubecontext cleanup failed. Which Failed due to %v \n ", err) + } + fmt.Printf("%v %v Sandbox cluster is removed successfully. \n", emoji.Broom, emoji.Broom) + return nil +} + +func removeSandboxKubeContext() error { + k8sCtxMgr := k8s.NewK8sContextManager() + return k8sCtxMgr.RemoveContext(sandboxContextName) +} diff --git a/flytectl/pkg/sandbox/teardown_test.go b/flytectl/pkg/sandbox/teardown_test.go new file mode 100644 index 0000000000..9047ee7614 --- /dev/null +++ b/flytectl/pkg/sandbox/teardown_test.go @@ -0,0 +1,48 @@ +package sandbox + +import ( + "context" + "fmt" + "testing" + + "github.com/docker/docker/api/types" + "github.com/flyteorg/flytectl/pkg/docker" + "github.com/flyteorg/flytectl/pkg/docker/mocks" + "github.com/flyteorg/flytectl/pkg/k8s" + k8sMocks "github.com/flyteorg/flytectl/pkg/k8s/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestTearDownFunc(t *testing.T) { + var containers []types.Container + container1 := types.Container{ + ID: "FlyteSandboxClusterName", + Names: []string{ + docker.FlyteSandboxClusterName, + }, + } + containers = append(containers, container1) + ctx := context.Background() + + mockDocker := &mocks.Docker{} + mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) + mockDocker.OnContainerRemove(ctx, mock.Anything, types.ContainerRemoveOptions{Force: true}).Return(fmt.Errorf("err")) + err := Teardown(ctx, mockDocker) + assert.NotNil(t, err) + + mockDocker = &mocks.Docker{} + mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(nil, fmt.Errorf("err")) + err = Teardown(ctx, mockDocker) + assert.NotNil(t, err) + + mockDocker = &mocks.Docker{} + mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) + mockDocker.OnContainerRemove(ctx, mock.Anything, types.ContainerRemoveOptions{Force: true}).Return(nil) + mockK8sContextMgr := &k8sMocks.ContextOps{} + mockK8sContextMgr.OnRemoveContext(mock.Anything).Return(nil) + k8s.ContextMgr = mockK8sContextMgr + err = Teardown(ctx, mockDocker) + assert.Nil(t, err) + +}