Skip to content

Commit

Permalink
#minor Documentation fix and refactoring in sandbox (#131)
Browse files Browse the repository at this point in the history
* Documentation fix

Signed-off-by: Yuvraj <[email protected]>
  • Loading branch information
yindia authored Jul 5, 2021
1 parent a87e20b commit b5d78aa
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 32 deletions.
28 changes: 17 additions & 11 deletions flytectl/.github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -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/<number>
https://github.com/flyteorg/flyte/issues/<number>

## Follow-up issue
_NA_
OR
_https://github.com/lyft/flyte/issues/<number>_
_https://github.com/flyteorg/flyte/issues/<number>_
4 changes: 2 additions & 2 deletions flytectl/cmd/sandbox/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 16 additions & 3 deletions flytectl/cmd/sandbox/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`
)

Expand Down
2 changes: 1 addition & 1 deletion flytectl/cmd/sandbox/sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 7 additions & 5 deletions flytectl/cmd/sandbox/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`
Expand Down Expand Up @@ -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,
})
}

Expand Down
53 changes: 46 additions & 7 deletions flytectl/cmd/sandbox/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion flytectl/cmd/sandbox/teardown.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions flytectl/pkg/docker/docker_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -43,7 +43,7 @@ var (
ExecConfig = types.ExecConfig{
AttachStderr: true,
Tty: true,
WorkingDir: FlyteSnackDir,
WorkingDir: Source,
AttachStdout: true,
Cmd: []string{},
}
Expand Down

0 comments on commit b5d78aa

Please sign in to comment.