Refactor (flyteorg#275)

* refactor packages Signed-off-by: Yuvraj <[email protected]>
eapolinario · Feb 17, 2022 · 6e80250 · 6e80250
1 parent fc7e267
commit 6e80250
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 40 deletions.
diff --git a/cmd/register/register_util.go b/cmd/register/register_util.go
@@ -15,7 +15,7 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/flyteorg/flytectl/pkg/util/githubutil"
+	"github.com/flyteorg/flytectl/pkg/githubutil"
 
 	"github.com/flyteorg/flytestdlib/contextutils"
 	"github.com/flyteorg/flytestdlib/promutils"

diff --git a/cmd/sandbox/start.go b/cmd/sandbox/start.go
@@ -9,10 +9,10 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/flyteorg/flytectl/pkg/util/githubutil"
+	"github.com/flyteorg/flytectl/clierrors"
+	"github.com/flyteorg/flytectl/pkg/githubutil"
 
 	"github.com/avast/retry-go"
-	"github.com/flyteorg/flytectl/clierrors"
 	"github.com/olekukonko/tablewriter"
 	corev1api "k8s.io/api/core/v1"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -52,11 +52,19 @@ Run specific version of Flyte. FlyteCTL sandbox only supports Flyte version avai
 
 Note: FlyteCTL sandbox is only supported for Flyte versions > v0.10.0
 
+Run latest pre release of  Flyte.
+::
+
+ flytectl sandbox start  --pre
+
+Note: pre release flag will be ignore if user pass version flag, In that case Flytectl will use specific version. 
+
 Specify a Flyte Sandbox compliant image with the registry. This is useful in case you want to use an image from your registry.
 ::
 
   flytectl sandbox start --image docker.io/my-override:latest
 
+Note: If image flag is passed then Flytectl will ignore version and pre flags.
 	
 Specify a Flyte Sandbox image pull policy. Possible pull policy values are Always, IfNotPresent, or Never:
 ::
@@ -70,7 +78,7 @@ Usage
 	taintEffect          = "NoSchedule"
 	sandboxContextName   = "flyte-sandbox"
 	sandboxDockerContext = "default"
-	imageName            = "cr.flyte.org/flyteorg/flyte-sandbox"
+	sandboxImageName     = "cr.flyte.org/flyteorg/flyte-sandbox"
 )
 
 type ExecResult struct {
@@ -159,8 +167,8 @@ func startSandbox(ctx context.Context, cli docker.Docker, reader io.Reader) (*bu
 		volumes = append(volumes, *vol)
 	}
 	sandboxImage := sandboxConfig.DefaultConfig.Image
-	if len(sandboxConfig.DefaultConfig.Image) == 0 {
-		image, version, err := githubutil.GetSandboxImage(sandboxConfig.DefaultConfig.Version, imageName)
+	if len(sandboxImage) == 0 {
+		image, version, err := githubutil.GetFullyQualifiedImageName(sandboxConfig.DefaultConfig.Version, sandboxImageName, sandboxConfig.DefaultConfig.Prerelease)
 		if err != nil {
 			return nil, err
 		}

diff --git a/cmd/sandbox/start_test.go b/cmd/sandbox/start_test.go
@@ -10,7 +10,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/flyteorg/flytectl/pkg/util/githubutil"
+	"github.com/flyteorg/flytectl/pkg/githubutil"
 
 	"github.com/flyteorg/flytectl/pkg/k8s"
 
@@ -89,7 +89,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		errCh := make(chan error)
 		sandboxConfig.DefaultConfig.Version = "v0.19.1"
 		bodyStatus := make(chan container.ContainerWaitOKBody)
-		image, _, err := githubutil.GetSandboxImage(sandboxConfig.DefaultConfig.Version, imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName(sandboxConfig.DefaultConfig.Version, sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -120,7 +120,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		ctx := context.Background()
 		mockDocker := &mocks.Docker{}
 		errCh := make(chan error)
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		bodyStatus := make(chan container.ContainerWaitOKBody)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
@@ -169,7 +169,7 @@ func TestStartSandboxFunc(t *testing.T) {
 			Source: sandboxConfig.DefaultConfig.Source,
 			Target: docker.Source,
 		})
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -211,7 +211,7 @@ func TestStartSandboxFunc(t *testing.T) {
 			Source: absPath,
 			Target: docker.Source,
 		})
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -246,7 +246,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		sandboxConfig.DefaultConfig.Version = "v0.18.0"
 		sandboxConfig.DefaultConfig.Source = ""
 
-		image, _, err := githubutil.GetSandboxImage(sandboxConfig.DefaultConfig.Version, imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName(sandboxConfig.DefaultConfig.Version, sandboxImageName, false)
 		assert.Nil(t, err)
 		volumes := docker.Volumes
 		mockDocker.OnContainerCreate(ctx, &container.Config{
@@ -281,7 +281,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		mockDocker := &mocks.Docker{}
 		sandboxConfig.DefaultConfig.Version = "v0.1444.0"
 		sandboxConfig.DefaultConfig.Source = ""
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		volumes := docker.Volumes
 		mockDocker.OnContainerCreate(ctx, &container.Config{
@@ -314,7 +314,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		errCh := make(chan error)
 		bodyStatus := make(chan container.ContainerWaitOKBody)
 		mockDocker := &mocks.Docker{}
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		sandboxConfig.DefaultConfig.Source = f.UserHomeDir()
 		volumes := docker.Volumes
@@ -360,7 +360,7 @@ func TestStartSandboxFunc(t *testing.T) {
 			Source: sandboxConfig.DefaultConfig.Source,
 			Target: docker.Source,
 		})
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -402,7 +402,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		mockDocker := &mocks.Docker{}
 		sandboxConfig.DefaultConfig.Source = ""
 		sandboxConfig.DefaultConfig.Version = ""
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -441,7 +441,7 @@ func TestStartSandboxFunc(t *testing.T) {
 			Source: sandboxConfig.DefaultConfig.Source,
 			Target: docker.Source,
 		})
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -481,7 +481,7 @@ func TestStartSandboxFunc(t *testing.T) {
 			Source: sandboxConfig.DefaultConfig.Source,
 			Target: docker.Source,
 		})
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -525,7 +525,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		if err != nil {
 			t.Error(err)
 		}
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		bodyStatus := make(chan container.ContainerWaitOKBody)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
@@ -565,7 +565,7 @@ func TestStartSandboxFunc(t *testing.T) {
 		mockDocker := &mocks.Docker{}
 		errCh := make(chan error)
 		bodyStatus := make(chan container.ContainerWaitOKBody)
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		mockDocker.OnContainerCreate(ctx, &container.Config{
 			Env:          docker.Environment,
@@ -700,26 +700,26 @@ func TestGetNodeTaintStatus(t *testing.T) {
 
 func TestGetSandboxImage(t *testing.T) {
 	t.Run("Get Latest sandbox", func(t *testing.T) {
-		image, _, err := githubutil.GetSandboxImage("", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("", sandboxImageName, false)
 		assert.Nil(t, err)
 		assert.Equal(t, true, strings.HasPrefix(image, "cr.flyte.org/flyteorg/flyte-sandbox:dind-"))
 	})
 
 	t.Run("Get sandbox image with version ", func(t *testing.T) {
-		image, _, err := githubutil.GetSandboxImage("v0.14.0", imageName)
+		image, _, err := githubutil.GetFullyQualifiedImageName("v0.14.0", sandboxImageName, false)
 		assert.Nil(t, err)
-		assert.Equal(t, true, strings.HasPrefix(image, imageName))
+		assert.Equal(t, true, strings.HasPrefix(image, sandboxImageName))
 	})
 	t.Run("Get sandbox image with wrong version ", func(t *testing.T) {
-		_, _, err := githubutil.GetSandboxImage("v100.1.0", imageName)
+		_, _, err := githubutil.GetFullyQualifiedImageName("v100.1.0", sandboxImageName, false)
 		assert.NotNil(t, err)
 	})
 	t.Run("Get sandbox image with wrong version ", func(t *testing.T) {
-		_, _, err := githubutil.GetSandboxImage("aaaaaa", imageName)
+		_, _, err := githubutil.GetFullyQualifiedImageName("aaaaaa", sandboxImageName, false)
 		assert.NotNil(t, err)
 	})
 	t.Run("Get sandbox image with version that is not supported", func(t *testing.T) {
-		_, _, err := githubutil.GetSandboxImage("v0.10.0", imageName)
+		_, _, err := githubutil.GetFullyQualifiedImageName("v0.10.0", sandboxImageName, false)
 		assert.NotNil(t, err)
 	})
 

diff --git a/cmd/upgrade/upgrade.go b/cmd/upgrade/upgrade.go
@@ -12,13 +12,13 @@ import (
 
 	stdlibversion "github.com/flyteorg/flytestdlib/version"
 
-	"github.com/flyteorg/flytectl/pkg/util/githubutil"
+	"github.com/flyteorg/flytectl/pkg/githubutil"
 
 	"github.com/flyteorg/flytestdlib/logger"
 	"github.com/mouuff/go-rocket-update/pkg/updater"
 
 	cmdCore "github.com/flyteorg/flytectl/cmd/core"
-	"github.com/flyteorg/flytectl/pkg/util/platformutil"
+	"github.com/flyteorg/flytectl/pkg/platformutil"
 	"github.com/spf13/cobra"
 )
 

diff --git a/cmd/upgrade/upgrade_test.go b/cmd/upgrade/upgrade_test.go
@@ -5,10 +5,10 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/flyteorg/flytectl/pkg/githubutil"
 	"github.com/flyteorg/flytectl/pkg/util"
-	"github.com/flyteorg/flytectl/pkg/util/githubutil"
 
-	"github.com/flyteorg/flytectl/pkg/util/platformutil"
+	"github.com/flyteorg/flytectl/pkg/platformutil"
 
 	"github.com/flyteorg/flyteidl/clients/go/admin/mocks"
 	stdlibversion "github.com/flyteorg/flytestdlib/version"

diff --git a/cmd/version/version.go b/cmd/version/version.go
@@ -6,9 +6,9 @@ import (
 	"fmt"
 	"runtime"
 
-	"github.com/flyteorg/flytectl/pkg/util/githubutil"
+	"github.com/flyteorg/flytectl/pkg/githubutil"
 
-	"github.com/flyteorg/flytectl/pkg/util/platformutil"
+	"github.com/flyteorg/flytectl/pkg/platformutil"
 
 	cmdCore "github.com/flyteorg/flytectl/cmd/core"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"

diff --git a/pkg/util/githubutil/githubutil.go → pkg/githubutil/githubutil.go b/pkg/util/githubutil/githubutil.go → pkg/githubutil/githubutil.go
@@ -8,15 +8,13 @@ import (
 	"runtime"
 	"strings"
 
-	sandboxConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox"
-
 	"github.com/flyteorg/flytectl/pkg/util"
 
 	"github.com/flyteorg/flytestdlib/logger"
 
 	"golang.org/x/oauth2"
 
-	"github.com/flyteorg/flytectl/pkg/util/platformutil"
+	"github.com/flyteorg/flytectl/pkg/platformutil"
 	stdlibversion "github.com/flyteorg/flytestdlib/version"
 	"github.com/mouuff/go-rocket-update/pkg/provider"
 	"github.com/mouuff/go-rocket-update/pkg/updater"
@@ -105,7 +103,7 @@ func GetSandboxImageSha(version string, pre bool) (string, string, error) {
 				break
 			}
 		}
-		logger.Infof(context.Background(), "sandbox started with release %s", release.GetTagName())
+		logger.Infof(context.Background(), "starting with release %s", release.GetTagName())
 	} else if len(version) > 0 {
 		r, err := CheckVersionExist(version, flyte)
 		if err != nil {
@@ -213,11 +211,12 @@ func CheckBrewInstall(goos platformutil.Platform) (string, error) {
 	return "", nil
 }
 
-// GetSandboxImage Returns the alternate image if specified, else
+// GetFullyQualifiedImageName Returns the sandbox image, version and error
 // if no version is specified then the Latest release of cr.flyte.org/flyteorg/flyte-sandbox:dind-{SHA} is used
 // else cr.flyte.org/flyteorg/flyte-sandbox:dind-{SHA}, where sha is derived from the version.
-func GetSandboxImage(version, image string) (string, string, error) {
-	sha, version, err := GetSandboxImageSha(version, sandboxConfig.DefaultConfig.Prerelease)
+// If pre release is true then use latest pre release of Flyte, In that case User don't need to pass version
+func GetFullyQualifiedImageName(version, image string, pre bool) (string, string, error) {
+	sha, version, err := GetSandboxImageSha(version, pre)
 	if err != nil {
 		return "", version, err
 	}

diff --git a/pkg/util/githubutil/githubutil_test.go → pkg/githubutil/githubutil_test.go b/pkg/util/githubutil/githubutil_test.go → pkg/githubutil/githubutil_test.go
@@ -8,11 +8,13 @@ import (
 
 	stdlibversion "github.com/flyteorg/flytestdlib/version"
 
-	"github.com/flyteorg/flytectl/pkg/util/platformutil"
+	"github.com/flyteorg/flytectl/pkg/platformutil"
 
 	"github.com/stretchr/testify/assert"
 )
 
+var sandboxImageName = "cr.flyte.org/flyteorg/flyte-sandbox"
+
 func TestGetLatestVersion(t *testing.T) {
 	t.Run("Get latest release with wrong url", func(t *testing.T) {
 		_, err := GetLatestVersion("fl")
@@ -42,6 +44,27 @@ func TestCheckVersionExist(t *testing.T) {
 	})
 }
 
+func TestGetFullyQualifiedImageName(t *testing.T) {
+	t.Run("Get tFully Qualified Image Name ", func(t *testing.T) {
+		image, tag, err := GetFullyQualifiedImageName("", sandboxImageName, false)
+		assert.Nil(t, err)
+		assert.Equal(t, true, strings.HasPrefix(tag, "v"))
+		assert.Equal(t, true, strings.HasPrefix(image, sandboxImageName))
+	})
+	t.Run("Get tFully Qualified Image Name with pre release", func(t *testing.T) {
+		image, tag, err := GetFullyQualifiedImageName("", sandboxImageName, true)
+		assert.Nil(t, err)
+		assert.Equal(t, true, strings.HasPrefix(tag, "v"))
+		assert.Equal(t, true, strings.HasPrefix(image, sandboxImageName))
+	})
+	t.Run("Get tFully Qualified Image Name with specific version", func(t *testing.T) {
+		image, tag, err := GetFullyQualifiedImageName("v0.19.0", sandboxImageName, true)
+		assert.Nil(t, err)
+		assert.Equal(t, "v0.19.0", tag)
+		assert.Equal(t, true, strings.HasPrefix(image, sandboxImageName))
+	})
+}
+
 func TestGetSHAFromVersion(t *testing.T) {
 	t.Run("Invalid Tag", func(t *testing.T) {
 		_, err := GetSHAFromVersion("v100.0.0", "flyte")

diff --git a/pkg/util/platformutil/platformutil.go → pkg/platformutil/platformutil.go b/pkg/util/platformutil/platformutil.go → pkg/platformutil/platformutil.go
diff --git a/pkg/util/platformutil/platformutil_test.go → pkg/platformutil/platformutil_test.go b/pkg/util/platformutil/platformutil_test.go → pkg/platformutil/platformutil_test.go