From fb1f3bf852ecdfbb61738943614f6aa831246ebc Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 9 Feb 2023 14:01:38 -0800 Subject: [PATCH] Support new sandbox-bundled features (#381) * Create a docker volume to persist database and object store Signed-off-by: Jeev B * Cleanup sandbox configuration directory and mount Signed-off-by: Jeev B * Add logic for backward-compatible demo reload Signed-off-by: Jeev B * Fix working directory for exec Signed-off-by: Jeev B * Generate mocks Signed-off-by: Jeev B * Get existing tests passing Signed-off-by: Jeev B * Cleanup volume creation and add a dryRun message Signed-off-by: Jeev B * Use fmt.Errorf in place of errors.New Signed-off-by: Jeev B * Fix build Signed-off-by: Jeev B * Add tests for volume creation Signed-off-by: Jeev B * Add test for volume teardown Signed-off-by: Jeev B * Include code path for volume creation in sandbox start test Signed-off-by: Jeev B * Add tests for demo reload with backward compatibility Signed-off-by: Jeev B * Suppress output of `which` when testing for sandbox version during demo reload --------- Signed-off-by: Jeev B --- .../cmd/config/subcommand/sandbox/teardown.go | 21 ++++ flytectl/cmd/demo/demo.go | 6 +- flytectl/cmd/demo/reload.go | 65 +++++++++- flytectl/cmd/demo/reload_test.go | 72 ++++++++++- flytectl/cmd/demo/teardown.go | 6 +- flytectl/cmd/demo/teardown_test.go | 29 ++++- flytectl/cmd/sandbox/teardown.go | 3 +- flytectl/pkg/docker/docker.go | 5 + flytectl/pkg/docker/docker_util.go | 65 ++++++++-- flytectl/pkg/docker/docker_util_test.go | 27 +++++ flytectl/pkg/docker/mocks/docker.go | 114 ++++++++++++++++++ flytectl/pkg/sandbox/start.go | 24 +++- flytectl/pkg/sandbox/start_test.go | 8 +- flytectl/pkg/sandbox/teardown.go | 16 ++- flytectl/pkg/sandbox/teardown_test.go | 7 +- flytectl/pkg/util/util.go | 3 +- 16 files changed, 427 insertions(+), 44 deletions(-) create mode 100644 flytectl/cmd/config/subcommand/sandbox/teardown.go diff --git a/flytectl/cmd/config/subcommand/sandbox/teardown.go b/flytectl/cmd/config/subcommand/sandbox/teardown.go new file mode 100644 index 0000000000..0e315d6bd0 --- /dev/null +++ b/flytectl/cmd/config/subcommand/sandbox/teardown.go @@ -0,0 +1,21 @@ +package sandbox + +import ( + "fmt" + + "github.com/spf13/pflag" +) + +type TeardownFlags struct { + Volume bool +} + +var ( + DefaultTeardownFlags = &TeardownFlags{} +) + +func (f *TeardownFlags) GetPFlagSet(prefix string) *pflag.FlagSet { + cmdFlags := pflag.NewFlagSet("TeardownFlags", pflag.ExitOnError) + cmdFlags.BoolVarP(&f.Volume, fmt.Sprintf("%v%v", prefix, "volume"), "v", f.Volume, "Optional. Clean up Docker volume. This will result in a permanent loss of all data within the database and object store. Use with caution!") + return cmdFlags +} diff --git a/flytectl/cmd/demo/demo.go b/flytectl/cmd/demo/demo.go index 23052175bb..a26b06e657 100644 --- a/flytectl/cmd/demo/demo.go +++ b/flytectl/cmd/demo/demo.go @@ -56,8 +56,10 @@ func CreateDemoCommand() *cobra.Command { Short: reloadShort, Long: reloadLong, PFlagProvider: sandboxCmdConfig.DefaultConfig, DisableFlyteClient: true}, "teardown": {CmdFunc: teardownDemoCluster, Aliases: []string{}, ProjectDomainNotRequired: true, - Short: teardownShort, - Long: teardownLong, DisableFlyteClient: true}, + Short: teardownShort, + Long: teardownLong, + PFlagProvider: sandboxCmdConfig.DefaultTeardownFlags, + DisableFlyteClient: true}, "status": {CmdFunc: demoClusterStatus, Aliases: []string{}, ProjectDomainNotRequired: true, Short: statusShort, Long: statusLong}, diff --git a/flytectl/cmd/demo/reload.go b/flytectl/cmd/demo/reload.go index f7441cf7c6..e7100802de 100644 --- a/flytectl/cmd/demo/reload.go +++ b/flytectl/cmd/demo/reload.go @@ -12,7 +12,8 @@ import ( ) const ( - labelSelector = "app=flyte" + internalBootstrapAgent = "flyte-sandbox-bootstrap" + labelSelector = "app.kubernetes.io/name=flyte-binary" ) const ( reloadShort = "Power cycle the Flyte executable pod, effectively picking up an updated config." @@ -28,8 +29,68 @@ Usage ` ) -// reloadDemoCluster will kill the flyte binary pod so the new one can pick up a new config file +func isLegacySandbox(ctx context.Context, cli docker.Docker, containerID string) (bool, error) { + var result bool + + // Check if sandbox is compatible with new bootstrap mechanism + exec, err := docker.ExecCommend( + ctx, + cli, + containerID, + []string{"sh", "-c", fmt.Sprintf("which %s > /dev/null", internalBootstrapAgent)}, + ) + if err != nil { + return result, err + } + if err = docker.InspectExecResp(ctx, cli, exec.ID); err != nil { + return result, err + } + res, err := cli.ContainerExecInspect(ctx, exec.ID) + if err != nil { + return result, err + } + + result = res.ExitCode != 0 + return result, nil +} + func reloadDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error { + cli, err := docker.GetDockerClient() + if err != nil { + return err + } + c, err := docker.GetSandbox(ctx, cli) + if err != nil { + return err + } + if c == nil { + return fmt.Errorf("reload failed - could not find an active sandbox") + } + + // Working with a legacy sandbox - fallback to legacy reload mechanism + useLegacyMethod, err := isLegacySandbox(ctx, cli, c.ID) + if err != nil { + return err + } + if useLegacyMethod { + return legacyReloadDemoCluster(ctx) + } + + // At this point we know that we are on a modern sandbox, and we can use the + // internal bootstrap agent to reload the cluster + exec, err := docker.ExecCommend(ctx, cli, c.ID, []string{internalBootstrapAgent}) + if err != nil { + return err + } + if err = docker.InspectExecResp(ctx, cli, exec.ID); err != nil { + return err + } + + return nil +} + +// legacyReloadDemoCluster will kill the flyte binary pod so the new one can pick up a new config file +func legacyReloadDemoCluster(ctx context.Context) error { k8sClient, err := k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint) if err != nil { fmt.Println("Could not get K8s client") diff --git a/flytectl/cmd/demo/reload_test.go b/flytectl/cmd/demo/reload_test.go index 35ceea040a..ef50033dc7 100644 --- a/flytectl/cmd/demo/reload_test.go +++ b/flytectl/cmd/demo/reload_test.go @@ -1,10 +1,16 @@ package demo import ( + "bufio" + "bytes" "context" + "fmt" "testing" + "github.com/docker/docker/api/types" cmdCore "github.com/flyteorg/flytectl/cmd/core" + "github.com/flyteorg/flytectl/pkg/docker" + "github.com/flyteorg/flytectl/pkg/docker/mocks" "github.com/flyteorg/flytectl/pkg/k8s" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -20,14 +26,76 @@ var fakePod = corev1.Pod{ }, ObjectMeta: metav1.ObjectMeta{ Name: "dummyflytepod", - Labels: map[string]string{"app": "flyte"}, + Labels: map[string]string{"app.kubernetes.io/name": "flyte-binary"}, }, } -func TestDemoReload(t *testing.T) { +func sandboxSetup(ctx context.Context, legacy bool) { + mockDocker := &mocks.Docker{} + docker.Client = mockDocker + mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return([]types.Container{ + { + ID: docker.FlyteSandboxClusterName, + Names: []string{ + docker.FlyteSandboxClusterName, + }, + }, + }, nil) + + // This first set of mocks is for the check for the bootstrap agent. This is + // Expected to fail in legacy sandboxes + var checkLegacySandboxExecExitCode int + if legacy { + checkLegacySandboxExecExitCode = 1 + } + mockDocker.OnContainerExecCreateMatch( + ctx, + docker.FlyteSandboxClusterName, + types.ExecConfig{ + AttachStderr: true, + Tty: true, + WorkingDir: "/", + AttachStdout: true, + Cmd: []string{"sh", "-c", fmt.Sprintf("which %s > /dev/null", internalBootstrapAgent)}, + }, + ).Return(types.IDResponse{ID: "0"}, nil) + mockDocker.OnContainerExecAttachMatch(ctx, "0", types.ExecStartCheck{}).Return(types.HijackedResponse{ + Reader: bufio.NewReader(bytes.NewReader([]byte{})), + }, nil) + mockDocker.OnContainerExecInspectMatch(ctx, "0").Return(types.ContainerExecInspect{ExitCode: checkLegacySandboxExecExitCode}, nil) + + // Register additional mocks for the actual execution of the bootstrap agent + // in non-legacy sandboxes + if !legacy { + mockDocker.OnContainerExecCreateMatch( + ctx, + docker.FlyteSandboxClusterName, + types.ExecConfig{ + AttachStderr: true, + Tty: true, + WorkingDir: "/", + AttachStdout: true, + Cmd: []string{internalBootstrapAgent}, + }, + ).Return(types.IDResponse{ID: "1"}, nil) + mockDocker.OnContainerExecAttachMatch(ctx, "1", types.ExecStartCheck{}).Return(types.HijackedResponse{ + Reader: bufio.NewReader(bytes.NewReader([]byte{})), + }, nil) + } +} + +func TestReloadLegacy(t *testing.T) { ctx := context.Background() commandCtx := cmdCore.CommandContext{} + sandboxSetup(ctx, false) + err := reloadDemoCluster(ctx, []string{}, commandCtx) + assert.Nil(t, err) +} +func TestDemoReloadLegacy(t *testing.T) { + ctx := context.Background() + commandCtx := cmdCore.CommandContext{} + sandboxSetup(ctx, true) t.Run("No errors", func(t *testing.T) { client := testclient.NewSimpleClientset() _, err := client.CoreV1().Pods("flyte").Create(ctx, &fakePod, v1.CreateOptions{}) diff --git a/flytectl/cmd/demo/teardown.go b/flytectl/cmd/demo/teardown.go index 399a904ab7..2fc59769be 100644 --- a/flytectl/cmd/demo/teardown.go +++ b/flytectl/cmd/demo/teardown.go @@ -3,10 +3,10 @@ package demo import ( "context" - "github.com/flyteorg/flytectl/pkg/sandbox" - "github.com/flyteorg/flytectl/pkg/docker" + "github.com/flyteorg/flytectl/pkg/sandbox" + sandboxCmdConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" cmdCore "github.com/flyteorg/flytectl/cmd/core" ) @@ -28,5 +28,5 @@ func teardownDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.Comm if err != nil { return err } - return sandbox.Teardown(ctx, cli) + return sandbox.Teardown(ctx, cli, sandboxCmdConfig.DefaultTeardownFlags) } diff --git a/flytectl/cmd/demo/teardown_test.go b/flytectl/cmd/demo/teardown_test.go index 34a25f23f5..cfe8bcfea7 100644 --- a/flytectl/cmd/demo/teardown_test.go +++ b/flytectl/cmd/demo/teardown_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/docker/docker/api/types" + sandboxCmdConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" "github.com/flyteorg/flytectl/cmd/testutils" "github.com/flyteorg/flytectl/pkg/configutil" "github.com/flyteorg/flytectl/pkg/docker" @@ -29,7 +30,7 @@ func TestTearDownFunc(t *testing.T) { } containers = append(containers, container1) - t.Run("Success", func(t *testing.T) { + t.Run("SuccessKeepVolume", func(t *testing.T) { ctx := context.Background() mockDocker := &mocks.Docker{} mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(containers, nil) @@ -37,23 +38,39 @@ func TestTearDownFunc(t *testing.T) { mockK8sContextMgr := &k8sMocks.ContextOps{} k8s.ContextMgr = mockK8sContextMgr mockK8sContextMgr.OnRemoveContextMatch(mock.Anything).Return(nil) - err := sandbox.Teardown(ctx, mockDocker) + err := sandbox.Teardown(ctx, mockDocker, sandboxCmdConfig.DefaultTeardownFlags) assert.Nil(t, err) }) - t.Run("Error", func(t *testing.T) { + t.Run("SuccessRemoveVolume", 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) + mockDocker.OnVolumeRemove(ctx, docker.FlyteSandboxVolumeName, true).Return(nil) + mockK8sContextMgr := &k8sMocks.ContextOps{} + k8s.ContextMgr = mockK8sContextMgr + mockK8sContextMgr.OnRemoveContextMatch(mock.Anything).Return(nil) + err := sandbox.Teardown( + ctx, + mockDocker, + &sandboxCmdConfig.TeardownFlags{Volume: true}, + ) + assert.Nil(t, err) + }) + t.Run("ErrorOnContainerRemove", 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 := sandbox.Teardown(ctx, mockDocker) + err := sandbox.Teardown(ctx, mockDocker, sandboxCmdConfig.DefaultTeardownFlags) assert.NotNil(t, err) }) - t.Run("Error", func(t *testing.T) { + t.Run("ErrorOnContainerList", 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) + err := sandbox.Teardown(ctx, mockDocker, sandboxCmdConfig.DefaultTeardownFlags) assert.NotNil(t, err) }) diff --git a/flytectl/cmd/sandbox/teardown.go b/flytectl/cmd/sandbox/teardown.go index 4e209a19c1..4b2fcd0469 100644 --- a/flytectl/cmd/sandbox/teardown.go +++ b/flytectl/cmd/sandbox/teardown.go @@ -6,6 +6,7 @@ import ( "github.com/flyteorg/flytectl/pkg/docker" "github.com/flyteorg/flytectl/pkg/sandbox" + sandboxCmdConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" cmdCore "github.com/flyteorg/flytectl/cmd/core" ) @@ -27,5 +28,5 @@ func teardownSandboxCluster(ctx context.Context, args []string, cmdCtx cmdCore.C if err != nil { return err } - return sandbox.Teardown(ctx, cli) + return sandbox.Teardown(ctx, cli, sandboxCmdConfig.DefaultTeardownFlags) } diff --git a/flytectl/pkg/docker/docker.go b/flytectl/pkg/docker/docker.go index 46ea3ea141..9d3ccb4efe 100644 --- a/flytectl/pkg/docker/docker.go +++ b/flytectl/pkg/docker/docker.go @@ -6,7 +6,9 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/volume" "github.com/docker/docker/client" specs "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -27,6 +29,9 @@ type Docker interface { ImageList(ctx context.Context, listOption types.ImageListOptions) ([]types.ImageSummary, error) ContainerStatPath(ctx context.Context, containerID, path string) (types.ContainerPathStat, error) CopyFromContainer(ctx context.Context, containerID, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) + VolumeCreate(ctx context.Context, options volume.VolumeCreateBody) (types.Volume, error) + VolumeList(ctx context.Context, filter filters.Args) (volume.VolumeListOKBody, error) + VolumeRemove(ctx context.Context, volumeID string, force bool) error } type FlyteDocker struct { diff --git a/flytectl/pkg/docker/docker_util.go b/flytectl/pkg/docker/docker_util.go index cf8901e558..221fb32eab 100644 --- a/flytectl/pkg/docker/docker_util.go +++ b/flytectl/pkg/docker/docker_util.go @@ -16,7 +16,9 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/api/types/volume" "github.com/docker/docker/pkg/stdcopy" "github.com/docker/go-connections/nat" cmdUtil "github.com/flyteorg/flytectl/pkg/commandutils" @@ -24,17 +26,20 @@ import ( ) var ( - FlyteStateDir = f.FilePathJoin(f.UserHomeDir(), ".flyte", "state") - Kubeconfig = f.FilePathJoin(FlyteStateDir, "kubeconfig") - SandboxKubeconfig = f.FilePathJoin(f.UserHomeDir(), ".flyte", "k3s", "k3s.yaml") - SuccessMessage = "Deploying Flyte..." - FlyteSandboxClusterName = "flyte-sandbox" - Environment = []string{"SANDBOX=1", "KUBERNETES_API_PORT=30086", "FLYTE_HOST=localhost:30081", "FLYTE_AWS_ENDPOINT=http://localhost:30084"} - Source = "/root" - StateDirMountDest = "/srv/flyte" - K3sDir = "/etc/rancher/" - Client Docker - Volumes = []mount.Mount{ + FlyteSandboxConfigDir = f.FilePathJoin(f.UserHomeDir(), ".flyte", "sandbox") + Kubeconfig = f.FilePathJoin(FlyteSandboxConfigDir, "kubeconfig") + SandboxKubeconfig = f.FilePathJoin(f.UserHomeDir(), ".flyte", "k3s", "k3s.yaml") + SuccessMessage = "Deploying Flyte..." + FlyteSandboxClusterName = "flyte-sandbox" + FlyteSandboxVolumeName = "flyte-sandbox" + FlyteSandboxInternalDir = "/var/lib/flyte" + FlyteSandboxInternalConfigDir = f.FilePathJoin(FlyteSandboxInternalDir, "config") + FlyteSandboxInternalStorageDir = f.FilePathJoin(FlyteSandboxInternalDir, "storage") + Environment = []string{"SANDBOX=1", "KUBERNETES_API_PORT=30086", "FLYTE_HOST=localhost:30081", "FLYTE_AWS_ENDPOINT=http://localhost:30084"} + Source = "/root" + K3sDir = "/etc/rancher/" + Client Docker + Volumes = []mount.Mount{ { Type: mount.TypeBind, Source: f.FilePathJoin(f.UserHomeDir(), ".flyte"), @@ -44,7 +49,7 @@ var ( ExecConfig = types.ExecConfig{ AttachStderr: true, Tty: true, - WorkingDir: Source, + WorkingDir: "/", AttachStdout: true, Cmd: []string{}, } @@ -75,7 +80,7 @@ func GetSandbox(ctx context.Context, cli Docker) (*types.Container, error) { return nil, err } for _, v := range containers { - if strings.Contains(v.Names[0], FlyteSandboxClusterName) { + if strings.TrimLeft(v.Names[0], "/") == FlyteSandboxClusterName { return &v, nil } } @@ -339,3 +344,37 @@ func InspectExecResp(ctx context.Context, cli Docker, containerID string) error } return nil } + +func PrintCreateVolume(name string) { + fmt.Printf("%v Run the following command to create a volume\n", emoji.Sparkle) + fmt.Printf(" docker volume create %v\n", name) +} + +func GetOrCreateVolume( + ctx context.Context, cli Docker, volumeName string, dryRun bool, +) (*types.Volume, error) { + if dryRun { + PrintCreateVolume(volumeName) + return nil, nil + } + + resp, err := cli.VolumeList(ctx, filters.NewArgs( + filters.KeyValuePair{Key: "name", Value: fmt.Sprintf("^%s$", volumeName)}, + )) + if err != nil { + return nil, err + } + switch len(resp.Volumes) { + case 0: + v, err := cli.VolumeCreate(ctx, volume.VolumeCreateBody{Name: volumeName}) + if err != nil { + return nil, err + } + return &v, nil + case 1: + return resp.Volumes[0], nil + default: + // We don't expect to ever arrive at this point + return nil, fmt.Errorf("unexpected error - found multiple volumes with name: %s", volumeName) + } +} diff --git a/flytectl/pkg/docker/docker_util_test.go b/flytectl/pkg/docker/docker_util_test.go index 79e7dae72e..f1b8f7b33a 100644 --- a/flytectl/pkg/docker/docker_util_test.go +++ b/flytectl/pkg/docker/docker_util_test.go @@ -14,6 +14,8 @@ import ( f "github.com/flyteorg/flytectl/pkg/filesystemutils" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/volume" "github.com/flyteorg/flytectl/pkg/docker/mocks" "github.com/stretchr/testify/mock" @@ -375,6 +377,31 @@ func TestInspectExecResp(t *testing.T) { } +func TestGetOrCreateVolume(t *testing.T) { + t.Run("VolumeExists", func(t *testing.T) { + ctx := context.Background() + mockDocker := &mocks.Docker{} + expected := &types.Volume{Name: "test"} + + mockDocker.OnVolumeList(ctx, filters.NewArgs(filters.KeyValuePair{Key: "name", Value: "^test$"})).Return(volume.VolumeListOKBody{Volumes: []*types.Volume{expected}}, nil) + actual, err := GetOrCreateVolume(ctx, mockDocker, "test", false) + assert.Equal(t, expected, actual, "volumes should match") + assert.Nil(t, err) + }) + t.Run("VolumeDoesNotExist", func(t *testing.T) { + ctx := context.Background() + mockDocker := &mocks.Docker{} + expected := types.Volume{Name: "test"} + + mockDocker.OnVolumeList(ctx, filters.NewArgs(filters.KeyValuePair{Key: "name", Value: "^test$"})).Return(volume.VolumeListOKBody{Volumes: []*types.Volume{}}, nil) + mockDocker.OnVolumeCreate(ctx, volume.VolumeCreateBody{Name: "test"}).Return(expected, nil) + actual, err := GetOrCreateVolume(ctx, mockDocker, "test", false) + assert.Equal(t, expected, *actual, "volumes should match") + assert.Nil(t, err) + }) + +} + func TestDemoPorts(t *testing.T) { _, ports, _ := GetDemoPorts() assert.Equal(t, 5, len(ports)) diff --git a/flytectl/pkg/docker/mocks/docker.go b/flytectl/pkg/docker/mocks/docker.go index a2ddc27cb7..b5361fc957 100644 --- a/flytectl/pkg/docker/mocks/docker.go +++ b/flytectl/pkg/docker/mocks/docker.go @@ -7,6 +7,8 @@ import ( container "github.com/docker/docker/api/types/container" + filters "github.com/docker/docker/api/types/filters" + io "io" mock "github.com/stretchr/testify/mock" @@ -16,6 +18,8 @@ import ( types "github.com/docker/docker/api/types" v1 "github.com/opencontainers/image-spec/specs-go/v1" + + volume "github.com/docker/docker/api/types/volume" ) // Docker is an autogenerated mock type for the Docker type @@ -536,3 +540,113 @@ func (_m *Docker) ImagePull(ctx context.Context, refStr string, options types.Im return r0, r1 } + +type Docker_VolumeCreate struct { + *mock.Call +} + +func (_m Docker_VolumeCreate) Return(_a0 types.Volume, _a1 error) *Docker_VolumeCreate { + return &Docker_VolumeCreate{Call: _m.Call.Return(_a0, _a1)} +} + +func (_m *Docker) OnVolumeCreate(ctx context.Context, options volume.VolumeCreateBody) *Docker_VolumeCreate { + c_call := _m.On("VolumeCreate", ctx, options) + return &Docker_VolumeCreate{Call: c_call} +} + +func (_m *Docker) OnVolumeCreateMatch(matchers ...interface{}) *Docker_VolumeCreate { + c_call := _m.On("VolumeCreate", matchers...) + return &Docker_VolumeCreate{Call: c_call} +} + +// VolumeCreate provides a mock function with given fields: ctx, options +func (_m *Docker) VolumeCreate(ctx context.Context, options volume.VolumeCreateBody) (types.Volume, error) { + ret := _m.Called(ctx, options) + + var r0 types.Volume + if rf, ok := ret.Get(0).(func(context.Context, volume.VolumeCreateBody) types.Volume); ok { + r0 = rf(ctx, options) + } else { + r0 = ret.Get(0).(types.Volume) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, volume.VolumeCreateBody) error); ok { + r1 = rf(ctx, options) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type Docker_VolumeList struct { + *mock.Call +} + +func (_m Docker_VolumeList) Return(_a0 volume.VolumeListOKBody, _a1 error) *Docker_VolumeList { + return &Docker_VolumeList{Call: _m.Call.Return(_a0, _a1)} +} + +func (_m *Docker) OnVolumeList(ctx context.Context, filter filters.Args) *Docker_VolumeList { + c_call := _m.On("VolumeList", ctx, filter) + return &Docker_VolumeList{Call: c_call} +} + +func (_m *Docker) OnVolumeListMatch(matchers ...interface{}) *Docker_VolumeList { + c_call := _m.On("VolumeList", matchers...) + return &Docker_VolumeList{Call: c_call} +} + +// VolumeList provides a mock function with given fields: ctx, filter +func (_m *Docker) VolumeList(ctx context.Context, filter filters.Args) (volume.VolumeListOKBody, error) { + ret := _m.Called(ctx, filter) + + var r0 volume.VolumeListOKBody + if rf, ok := ret.Get(0).(func(context.Context, filters.Args) volume.VolumeListOKBody); ok { + r0 = rf(ctx, filter) + } else { + r0 = ret.Get(0).(volume.VolumeListOKBody) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, filters.Args) error); ok { + r1 = rf(ctx, filter) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type Docker_VolumeRemove struct { + *mock.Call +} + +func (_m Docker_VolumeRemove) Return(_a0 error) *Docker_VolumeRemove { + return &Docker_VolumeRemove{Call: _m.Call.Return(_a0)} +} + +func (_m *Docker) OnVolumeRemove(ctx context.Context, volumeID string, force bool) *Docker_VolumeRemove { + c_call := _m.On("VolumeRemove", ctx, volumeID, force) + return &Docker_VolumeRemove{Call: c_call} +} + +func (_m *Docker) OnVolumeRemoveMatch(matchers ...interface{}) *Docker_VolumeRemove { + c_call := _m.On("VolumeRemove", matchers...) + return &Docker_VolumeRemove{Call: c_call} +} + +// VolumeRemove provides a mock function with given fields: ctx, volumeID, force +func (_m *Docker) VolumeRemove(ctx context.Context, volumeID string, force bool) error { + ret := _m.Called(ctx, volumeID, force) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, bool) error); ok { + r0 = rf(ctx, volumeID, force) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/flytectl/pkg/sandbox/start.go b/flytectl/pkg/sandbox/start.go index f7ac5ed9e9..744bc0a923 100644 --- a/flytectl/pkg/sandbox/start.go +++ b/flytectl/pkg/sandbox/start.go @@ -39,7 +39,7 @@ const ( sandboxImageName = "cr.flyte.org/flyteorg/flyte-sandbox" demoImageName = "cr.flyte.org/flyteorg/flyte-sandbox-bundled" DefaultFlyteConfig = "/opt/flyte/defaults.flyte.yaml" - k3sKubeConfigEnvVar = "K3S_KUBECONFIG_OUTPUT=/srv/flyte/kubeconfig" + k3sKubeConfigEnvVar = "K3S_KUBECONFIG_OUTPUT=/var/lib/flyte/config/kubeconfig" ) func isNodeTainted(ctx context.Context, client corev1.CoreV1Interface) (bool, error) { @@ -184,11 +184,11 @@ func startSandbox(ctx context.Context, cli docker.Docker, g github.GHRepoService volumes = append(volumes, *vol) } - // This is the state directory mount, flyte will write the kubeconfig here. May hold more in future releases + // This is the sandbox configuration directory mount, flyte will write the kubeconfig here. May hold more in future releases // To be interoperable with the old sandbox, only mount if the directory exists, should've created by StartCluster - if fileInfo, err := os.Stat(docker.FlyteStateDir); err == nil { + if fileInfo, err := os.Stat(docker.FlyteSandboxConfigDir); err == nil { if fileInfo.IsDir() { - if vol, err := MountVolume(docker.FlyteStateDir, docker.StateDirMountDest); err != nil { + if vol, err := MountVolume(docker.FlyteSandboxConfigDir, docker.FlyteSandboxInternalConfigDir); err != nil { return nil, err } else if vol != nil { volumes = append(volumes, *vol) @@ -196,6 +196,22 @@ func startSandbox(ctx context.Context, cli docker.Docker, g github.GHRepoService } } + // Create and mount a docker volume that will be used to persist data + // across sandbox clusters + if _, err := docker.GetOrCreateVolume( + ctx, + cli, + docker.FlyteSandboxVolumeName, + sandboxConfig.DryRun, + ); err != nil { + return nil, err + } + volumes = append(volumes, mount.Mount{ + Type: mount.TypeVolume, + Source: docker.FlyteSandboxVolumeName, + Target: docker.FlyteSandboxInternalStorageDir, + }) + sandboxImage := sandboxConfig.Image if len(sandboxImage) == 0 { image, version, err := github.GetFullyQualifiedImageName(defaultImagePrefix, sandboxConfig.Version, defaultImageName, sandboxConfig.Prerelease, g) diff --git a/flytectl/pkg/sandbox/start_test.go b/flytectl/pkg/sandbox/start_test.go index 7bc204ac34..a09797d363 100644 --- a/flytectl/pkg/sandbox/start_test.go +++ b/flytectl/pkg/sandbox/start_test.go @@ -19,6 +19,8 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/volume" sandboxCmdConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" "github.com/google/go-github/v42/github" "github.com/stretchr/testify/assert" @@ -86,6 +88,8 @@ func sandboxSetup() { bodyStatus := make(chan container.ContainerWaitOKBody) githubMock = &ghMocks.GHRepoService{} sandboxCmdConfig.DefaultConfig.Image = "dummyimage" + mockDocker.OnVolumeList(ctx, filters.NewArgs(filters.KeyValuePair{Key: "name", Value: fmt.Sprintf("^%s$", docker.FlyteSandboxVolumeName)})).Return(volume.VolumeListOKBody{Volumes: []*types.Volume{}}, nil) + mockDocker.OnVolumeCreate(ctx, volume.VolumeCreateBody{Name: docker.FlyteSandboxVolumeName}).Return(types.Volume{Name: docker.FlyteSandboxVolumeName}, nil) mockDocker.OnContainerCreateMatch(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(container.ContainerCreateCreatedBody{ ID: "Hello", }, nil) @@ -119,7 +123,8 @@ func TestStartFunc(t *testing.T) { Timestamps: true, Follow: true, }).Return(nil, nil) - + mockDocker.OnVolumeList(ctx, filters.NewArgs(filters.KeyValuePair{Key: mock.Anything, Value: mock.Anything})).Return(volume.VolumeListOKBody{Volumes: []*types.Volume{}}, nil) + mockDocker.OnVolumeCreate(ctx, volume.VolumeCreateBody{Name: mock.Anything}).Return(types.Volume{}, nil) _, err := startSandbox(ctx, mockDocker, githubMock, os.Stdin, config, sandboxImageName, defaultImagePrefix, exposedPorts, portBindings, util.SandBoxConsolePort) assert.Nil(t, err) }) @@ -295,7 +300,6 @@ func TestStartFunc(t *testing.T) { mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return([]types.Container{}, nil) mockDocker.OnImagePullMatch(mock.Anything, mock.Anything, mock.Anything).Return(os.Stdin, nil) mockDocker.OnContainerStart(ctx, "Hello", types.ContainerStartOptions{}).Return(nil) - stringReader := strings.NewReader(docker.SuccessMessage) reader := ioutil.NopCloser(stringReader) mockDocker.OnContainerLogsMatch(ctx, mock.Anything, types.ContainerLogsOptions{ diff --git a/flytectl/pkg/sandbox/teardown.go b/flytectl/pkg/sandbox/teardown.go index 633d026833..3881617db6 100644 --- a/flytectl/pkg/sandbox/teardown.go +++ b/flytectl/pkg/sandbox/teardown.go @@ -6,12 +6,13 @@ import ( "github.com/docker/docker/api/types" "github.com/enescakir/emoji" + sandboxCmdConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" "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 { +func Teardown(ctx context.Context, cli docker.Docker, teardownFlags *sandboxCmdConfig.TeardownFlags) error { c, err := docker.GetSandbox(ctx, cli) if err != nil { return err @@ -24,12 +25,19 @@ func Teardown(ctx context.Context, cli docker.Docker) error { } } if err := configutil.ConfigCleanup(); err != nil { - fmt.Printf("Config cleanup failed. Which Failed due to %v \n ", err) + 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("Kubecontext cleanup failed. Which Failed due to %v\n", err) } - fmt.Printf("%v %v Sandbox cluster is removed successfully. \n", emoji.Broom, emoji.Broom) + // Teardown volume if option is specified + if teardownFlags.Volume { + if err := cli.VolumeRemove(ctx, docker.FlyteSandboxVolumeName, true); err != nil { + fmt.Printf("Volume 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 } diff --git a/flytectl/pkg/sandbox/teardown_test.go b/flytectl/pkg/sandbox/teardown_test.go index 9047ee7614..3f3702c9ad 100644 --- a/flytectl/pkg/sandbox/teardown_test.go +++ b/flytectl/pkg/sandbox/teardown_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/docker/docker/api/types" + sandboxCmdConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" "github.com/flyteorg/flytectl/pkg/docker" "github.com/flyteorg/flytectl/pkg/docker/mocks" "github.com/flyteorg/flytectl/pkg/k8s" @@ -28,12 +29,12 @@ 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 := Teardown(ctx, mockDocker) + err := Teardown(ctx, mockDocker, sandboxCmdConfig.DefaultTeardownFlags) assert.NotNil(t, err) mockDocker = &mocks.Docker{} mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return(nil, fmt.Errorf("err")) - err = Teardown(ctx, mockDocker) + err = Teardown(ctx, mockDocker, sandboxCmdConfig.DefaultTeardownFlags) assert.NotNil(t, err) mockDocker = &mocks.Docker{} @@ -42,7 +43,7 @@ func TestTearDownFunc(t *testing.T) { mockK8sContextMgr := &k8sMocks.ContextOps{} mockK8sContextMgr.OnRemoveContext(mock.Anything).Return(nil) k8s.ContextMgr = mockK8sContextMgr - err = Teardown(ctx, mockDocker) + err = Teardown(ctx, mockDocker, sandboxCmdConfig.DefaultTeardownFlags) assert.Nil(t, err) } diff --git a/flytectl/pkg/util/util.go b/flytectl/pkg/util/util.go index ece882279a..a851ac4e9d 100644 --- a/flytectl/pkg/util/util.go +++ b/flytectl/pkg/util/util.go @@ -11,7 +11,6 @@ import ( "github.com/flyteorg/flytectl/pkg/configutil" "github.com/flyteorg/flytectl/pkg/docker" - f "github.com/flyteorg/flytectl/pkg/filesystemutils" "github.com/enescakir/emoji" hversion "github.com/hashicorp/go-version" @@ -57,7 +56,7 @@ func CreatePathAndFile(pathToConfig string) error { // SetupFlyteDir will create .flyte dir if not exist func SetupFlyteDir() error { - if err := os.MkdirAll(f.FilePathJoin(f.UserHomeDir(), ".flyte", "state"), os.ModePerm); err != nil { + if err := os.MkdirAll(docker.FlyteSandboxConfigDir, os.ModePerm); err != nil { return err }