From 8cc5354253e520584519f1c3b60303223cb27623 Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Mon, 5 Jul 2021 13:15:34 +0530 Subject: [PATCH] #minor Documentation fix and refactoring in sandbox (#131) * Documentation fix Signed-off-by: Yuvraj --- flytectl/.github/PULL_REQUEST_TEMPLATE.md | 28 +++++++----- flytectl/cmd/sandbox/exec.go | 4 +- flytectl/cmd/sandbox/sandbox.go | 19 ++++++-- flytectl/cmd/sandbox/sandbox_test.go | 2 +- flytectl/cmd/sandbox/start.go | 12 ++--- flytectl/cmd/sandbox/start_test.go | 53 ++++++++++++++++++++--- flytectl/cmd/sandbox/teardown.go | 2 +- flytectl/pkg/docker/docker_util.go | 4 +- 8 files changed, 92 insertions(+), 32 deletions(-) diff --git a/flytectl/.github/PULL_REQUEST_TEMPLATE.md b/flytectl/.github/PULL_REQUEST_TEMPLATE.md index d2becf38b7..97321913f9 100644 --- a/flytectl/.github/PULL_REQUEST_TEMPLATE.md +++ b/flytectl/.github/PULL_REQUEST_TEMPLATE.md @@ -1,26 +1,32 @@ +## Read then delete + +- Make sure to use a concise title for the pull-request. +- Use #patch, #minor #majora or #none in the pull-request title to bump the corresponding version. Otherwise, the patch version + will be bumped. [More details](https://github.com/marketplace/actions/github-tag-bump) + # TL;DR _Please replace this text with a description of what this PR accomplishes._ ## Type - - [ ] Bug Fix - - [ ] Feature - - [ ] Plugin +- [ ] Bug Fix +- [ ] Feature +- [ ] Plugin ## Are all requirements met? - - [ ] Code completed - - [ ] Smoke tested - - [ ] Unit tests added - - [ ] Code documentation added - - [ ] Any pending items have an associated Issue +- [ ] Code completed +- [ ] Smoke tested +- [ ] Unit tests added +- [ ] Code documentation added +- [ ] Any pending items have an associated Issue ## Complete description - _How did you fix the bug, make the feature etc. Link to any design docs etc_ +_How did you fix the bug, make the feature etc. Link to any design docs etc_ ## Tracking Issue -https://github.com/lyft/flyte/issues/ +https://github.com/flyteorg/flyte/issues/ ## Follow-up issue _NA_ OR -_https://github.com/lyft/flyte/issues/_ +_https://github.com/flyteorg/flyte/issues/_ diff --git a/flytectl/cmd/sandbox/exec.go b/flytectl/cmd/sandbox/exec.go index 804d54b6c6..63a17e8884 100644 --- a/flytectl/cmd/sandbox/exec.go +++ b/flytectl/cmd/sandbox/exec.go @@ -9,9 +9,9 @@ import ( ) const ( - execShort = "Execute any command in sandbox" + execShort = "Execute non-interactive command inside the sandbox container" execLong = ` -Execute command will Will run non-interactive commands and return immediately with the output. +Execute command will run non-interactive command inside the sandbox container and return immediately with the output.By default flytectl exec in /root directory inside the sandbox container :: bin/flytectl sandbox exec -- ls -al diff --git a/flytectl/cmd/sandbox/sandbox.go b/flytectl/cmd/sandbox/sandbox.go index 233f4f0eba..9be32bbba4 100644 --- a/flytectl/cmd/sandbox/sandbox.go +++ b/flytectl/cmd/sandbox/sandbox.go @@ -8,18 +8,31 @@ import ( // Long descriptions are whitespace sensitive when generating docs using sphinx. const ( - sandboxShort = `Used for testing flyte sandbox.` + sandboxShort = `Used for sandbox interactions like start/teardown/status/exec.` sandboxLong = ` -Example Create sandbox cluster. +The Flyte Sandbox is a fully standalone minimal environment for running Flyte. provides a simplified way of running flyte-sandbox as a single Docker container running locally. + +Create sandbox cluster. :: bin/flytectl sandbox start -Example Remove sandbox cluster. +Remove sandbox cluster. :: bin/flytectl sandbox teardown + + +Check status of sandbox container. +:: + + bin/flytectl sandbox status + +Execute command inside sandbox container. +:: + + bin/flytectl sandbox exec -- pwd ` ) diff --git a/flytectl/cmd/sandbox/sandbox_test.go b/flytectl/cmd/sandbox/sandbox_test.go index 640390c137..ee615c08ce 100644 --- a/flytectl/cmd/sandbox/sandbox_test.go +++ b/flytectl/cmd/sandbox/sandbox_test.go @@ -11,7 +11,7 @@ import ( func TestCreateSandboxCommand(t *testing.T) { sandboxCommand := CreateSandboxCommand() assert.Equal(t, sandboxCommand.Use, "sandbox") - assert.Equal(t, sandboxCommand.Short, "Used for testing flyte sandbox.") + assert.Equal(t, sandboxCommand.Short, "Used for sandbox interactions like start/teardown/status/exec.") fmt.Println(sandboxCommand.Commands()) assert.Equal(t, len(sandboxCommand.Commands()), 4) cmdNouns := sandboxCommand.Commands() diff --git a/flytectl/cmd/sandbox/start.go b/flytectl/cmd/sandbox/start.go index 6a78d639e4..5853ee5060 100644 --- a/flytectl/cmd/sandbox/start.go +++ b/flytectl/cmd/sandbox/start.go @@ -17,17 +17,19 @@ import ( ) const ( - startShort = "Start the flyte sandbox" + startShort = "Start the flyte sandbox cluster" startLong = ` -Start will run the flyte sandbox cluster inside a docker container and setup the config that is required +The Flyte Sandbox is a fully standalone minimal environment for running Flyte. provides a simplified way of running flyte-sandbox as a single Docker container running locally. + +Start sandbox cluster without any source code :: bin/flytectl sandbox start -Mount your flytesnacks repository code inside sandbox +Mount your source code repository inside sandbox :: - bin/flytectl sandbox start --sourcesPath=$HOME/flyteorg/flytesnacks + bin/flytectl sandbox start --source=$HOME/flyteorg/flytesnacks Usage ` @@ -75,7 +77,7 @@ func startSandbox(ctx context.Context, cli docker.Docker, reader io.Reader) (*bu docker.Volumes = append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) } diff --git a/flytectl/cmd/sandbox/start_test.go b/flytectl/cmd/sandbox/start_test.go index a0bef27c8d..042860ed60 100644 --- a/flytectl/cmd/sandbox/start_test.go +++ b/flytectl/cmd/sandbox/start_test.go @@ -6,6 +6,7 @@ import ( "io" "io/ioutil" "os" + "path/filepath" "strings" "testing" @@ -56,7 +57,7 @@ func TestStartSandboxFunc(t *testing.T) { _, err := startSandbox(ctx, mockDocker, os.Stdin) assert.Nil(t, err) }) - t.Run("Successfully run sandbox cluster with flytesnacks", func(t *testing.T) { + t.Run("Successfully run sandbox cluster with source code", func(t *testing.T) { ctx := context.Background() errCh := make(chan error) bodyStatus := make(chan container.ContainerWaitOKBody) @@ -65,7 +66,7 @@ func TestStartSandboxFunc(t *testing.T) { volumes := append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: sandboxConfig.DefaultConfig.Source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) mockDocker.OnContainerCreate(ctx, &container.Config{ Env: docker.Environment, @@ -92,6 +93,44 @@ func TestStartSandboxFunc(t *testing.T) { _, err := startSandbox(ctx, mockDocker, os.Stdin) assert.Nil(t, err) }) + t.Run("Successfully run sandbox cluster with abs path of source code", func(t *testing.T) { + ctx := context.Background() + errCh := make(chan error) + bodyStatus := make(chan container.ContainerWaitOKBody) + mockDocker := &mocks.Docker{} + sandboxConfig.DefaultConfig.Source = "../" + absPath, err := filepath.Abs(sandboxConfig.DefaultConfig.Source) + assert.Nil(t, err) + volumes := append(docker.Volumes, mount.Mount{ + Type: mount.TypeBind, + Source: absPath, + Target: docker.Source, + }) + mockDocker.OnContainerCreate(ctx, &container.Config{ + Env: docker.Environment, + Image: docker.ImageName, + Tty: false, + ExposedPorts: p1, + }, &container.HostConfig{ + Mounts: volumes, + PortBindings: p2, + Privileged: true, + }, nil, nil, mock.Anything).Return(container.ContainerCreateCreatedBody{ + ID: "Hello", + }, nil) + mockDocker.OnContainerStart(ctx, "Hello", types.ContainerStartOptions{}).Return(nil) + mockDocker.OnContainerList(ctx, types.ContainerListOptions{All: true}).Return([]types.Container{}, nil) + mockDocker.OnImagePullMatch(ctx, mock.Anything, types.ImagePullOptions{}).Return(os.Stdin, nil) + mockDocker.OnContainerLogsMatch(ctx, mock.Anything, types.ContainerLogsOptions{ + ShowStderr: true, + ShowStdout: true, + Timestamps: true, + Follow: true, + }).Return(nil, nil) + mockDocker.OnContainerWaitMatch(ctx, mock.Anything, container.WaitConditionNotRunning).Return(bodyStatus, errCh) + _, err = startSandbox(ctx, mockDocker, os.Stdin) + assert.Nil(t, err) + }) t.Run("Error in pulling image", func(t *testing.T) { ctx := context.Background() errCh := make(chan error) @@ -101,7 +140,7 @@ func TestStartSandboxFunc(t *testing.T) { volumes := append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: sandboxConfig.DefaultConfig.Source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) mockDocker.OnContainerCreate(ctx, &container.Config{ Env: docker.Environment, @@ -137,7 +176,7 @@ func TestStartSandboxFunc(t *testing.T) { volumes := append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: sandboxConfig.DefaultConfig.Source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) mockDocker.OnContainerCreate(ctx, &container.Config{ Env: docker.Environment, @@ -181,7 +220,7 @@ func TestStartSandboxFunc(t *testing.T) { volumes := append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: sandboxConfig.DefaultConfig.Source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) mockDocker.OnContainerCreate(ctx, &container.Config{ Env: docker.Environment, @@ -217,7 +256,7 @@ func TestStartSandboxFunc(t *testing.T) { volumes := append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: sandboxConfig.DefaultConfig.Source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) mockDocker.OnContainerCreate(ctx, &container.Config{ Env: docker.Environment, @@ -253,7 +292,7 @@ func TestStartSandboxFunc(t *testing.T) { volumes := append(docker.Volumes, mount.Mount{ Type: mount.TypeBind, Source: sandboxConfig.DefaultConfig.Source, - Target: docker.FlyteSnackDir, + Target: docker.Source, }) mockDocker.OnContainerCreate(ctx, &container.Config{ Env: docker.Environment, diff --git a/flytectl/cmd/sandbox/teardown.go b/flytectl/cmd/sandbox/teardown.go index 9d05a2581b..1c9e634351 100644 --- a/flytectl/cmd/sandbox/teardown.go +++ b/flytectl/cmd/sandbox/teardown.go @@ -15,7 +15,7 @@ import ( const ( teardownShort = "Teardown will cleanup the sandbox environment" teardownLong = ` -Teardown will remove docker container and all the flyte config +Teardown will remove sandbox cluster and all the flyte config created by sandbox start :: bin/flytectl sandbox teardown diff --git a/flytectl/pkg/docker/docker_util.go b/flytectl/pkg/docker/docker_util.go index 03e220879f..c1b39bd182 100644 --- a/flytectl/pkg/docker/docker_util.go +++ b/flytectl/pkg/docker/docker_util.go @@ -30,7 +30,7 @@ var ( ImageName = "cr.flyte.org/flyteorg/flyte-sandbox:dind" FlyteSandboxClusterName = "flyte-sandbox" Environment = []string{"SANDBOX=1", "KUBERNETES_API_PORT=30086", "FLYTE_HOST=localhost:30081", "FLYTE_AWS_ENDPOINT=http://localhost:30084"} - FlyteSnackDir = "/usr/src" + Source = "/root" K3sDir = "/etc/rancher/" Client Docker Volumes = []mount.Mount{ @@ -43,7 +43,7 @@ var ( ExecConfig = types.ExecConfig{ AttachStderr: true, Tty: true, - WorkingDir: FlyteSnackDir, + WorkingDir: Source, AttachStdout: true, Cmd: []string{}, }