Skip to content

Commit

Permalink
Hotfix register example (flyteorg#172)
Browse files Browse the repository at this point in the history
* fix register example

Signed-off-by: Yuvraj <code@evalsocket.dev>
  • Loading branch information
yindia committed Sep 3, 2021
1 parent 6a8bb24 commit 24710f7
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 66 deletions.
3 changes: 2 additions & 1 deletion flytectl/cmd/config/subcommand/register/files_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ type FilesConfig struct {
ContinueOnError bool `json:"continueOnError" pflag:",continue on error when registering files."`
Archive bool `json:"archive" pflag:",pass in archive file either an http link or local path."`
AssumableIamRole string `json:"assumableIamRole" pflag:", custom assumable iam auth role to register launch plans with."`
K8ServiceAccount string `json:"k8ServiceAccount" pflag:", custom kubernetes service account auth role to register launch plans with."`
K8sServiceAccount string `json:"k8sServiceAccount" pflag:", custom kubernetes service account auth role to register launch plans with."`
K8ServiceAccount string `json:"k8ServiceAccount" pflag:", deprecated. Please use --K8sServiceAccount"`
OutputLocationPrefix string `json:"outputLocationPrefix" pflag:", custom output location prefix for offloaded types (files/schemas)."`
SourceUploadPath string `json:"sourceUploadPath" pflag:", Location for source code in storage."`
DryRun bool `json:"dryRun" pflag:",execute command without making any modifications."`
Expand Down
3 changes: 2 additions & 1 deletion flytectl/cmd/config/subcommand/register/filesconfig_flags.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions flytectl/cmd/config/subcommand/register/filesconfig_flags_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 26 additions & 7 deletions flytectl/cmd/register/examples.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import (
"context"
"fmt"

"github.com/flyteorg/flytestdlib/logger"

"github.com/google/go-github/github"

rconfig "github.com/flyteorg/flytectl/cmd/config/subcommand/register"
cmdCore "github.com/flyteorg/flytectl/cmd/core"
)
Expand All @@ -16,27 +20,42 @@ Registers all latest flytesnacks example
bin/flytectl register examples -d development -p flytesnacks
Registers specific release of flytesnacks example
::
bin/flytectl register examples -d development -p flytesnacks v0.2.176
Note: register command automatically override the version with release version
Usage
`
)

var (
githubOrg = "flyteorg"
githubRepository = "flytesnacks"
snackReleaseURL = "https://github.com/flyteorg/flytesnacks/releases/download/%s/flytesnacks-%s.tgz"
flyteManifest = "https://github.com/flyteorg/flytesnacks/releases/download/%s/flyte_tests_manifest.json"
githubOrg = "flyteorg"
flytesnacksRepository = "flytesnacks"
)

func registerExamplesFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error {
flytesnacks, tag, err := getFlyteTestManifest(githubOrg, githubRepository)
var examples []github.ReleaseAsset
var release string

// Deprecated checks for --k8Service
deprecatedCheck(ctx)

if len(args) == 1 {
release = args[0]
}
examples, tag, err := getAllFlytesnacksExample(githubOrg, flytesnacksRepository, release)
if err != nil {
return err
}

logger.Infof(ctx, "Register started for %s %s release https://github.com/%s/%s/releases/tag/%s", flytesnacksRepository, tag, githubOrg, flytesnacksRepository, tag)
rconfig.DefaultFilesConfig.Archive = true
for _, v := range flytesnacks {
rconfig.DefaultFilesConfig.Version = tag
for _, v := range examples {
args := []string{
fmt.Sprintf(snackReleaseURL, tag, v.Name),
*v.BrowserDownloadURL,
}
if err := Register(ctx, args, cmdCtx); err != nil {
return fmt.Errorf("Example %v failed to register %v", v.Name, err)
Expand Down
4 changes: 2 additions & 2 deletions flytectl/cmd/register/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ func TestRegisterExamplesFunc(t *testing.T) {
func TestRegisterExamplesFuncErr(t *testing.T) {
setup()
registerFilesSetup()
githubRepository = "testingsnacks"
flytesnacksRepository = "testingsnacks"
args = []string{""}

err := registerExamplesFunc(ctx, args, cmdCtx)
// TODO (Yuvraj) make test to success after fixing flytesnacks bug
assert.NotNil(t, err)
githubRepository = "flytesnacks"
flytesnacksRepository = "flytesnacks"
}
3 changes: 3 additions & 0 deletions flytectl/cmd/register/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ func Register(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext)
var _err error
var dataRefs []string

// Deprecated checks for --k8Service
deprecatedCheck(ctx)

// getSerializeOutputFiles will return you all proto and source code compress file in sorted order
dataRefs, tmpDir, err := getSerializeOutputFiles(ctx, args)
if err != nil {
Expand Down
65 changes: 30 additions & 35 deletions flytectl/cmd/register/register_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,8 @@ type HTTPClient interface {
Do(req *http.Request) (*http.Response, error)
}

var FlyteSnacksRelease []FlyteSnack
var Client *storage.DataStore

// FlyteSnack Defines flyte test manifest structure
type FlyteSnack struct {
Name string `json:"name"`
Priority string `json:"priority"`
Path string `json:"path"`
ExitCondition Condition `json:"exitCondition"`
}

type Condition struct {
ExitSuccess bool `json:"exit_success"`
ExitMessage string `json:"exit_message"`
}

var httpClient HTTPClient

func init() {
Expand Down Expand Up @@ -242,12 +228,12 @@ func hydrateTaskSpec(task *admin.TaskSpec, sourceCode string) error {

func hydrateLaunchPlanSpec(lpSpec *admin.LaunchPlanSpec) {
assumableIamRole := len(rconfig.DefaultFilesConfig.AssumableIamRole) > 0
k8ServiceAcct := len(rconfig.DefaultFilesConfig.K8ServiceAccount) > 0
k8sServiceAcct := len(rconfig.DefaultFilesConfig.K8sServiceAccount) > 0
outputLocationPrefix := len(rconfig.DefaultFilesConfig.OutputLocationPrefix) > 0
if assumableIamRole || k8ServiceAcct {
if assumableIamRole || k8sServiceAcct {
lpSpec.AuthRole = &admin.AuthRole{
AssumableIamRole: rconfig.DefaultFilesConfig.AssumableIamRole,
KubernetesServiceAccount: rconfig.DefaultFilesConfig.K8ServiceAccount,
KubernetesServiceAccount: rconfig.DefaultFilesConfig.K8sServiceAccount,
}
}
if outputLocationPrefix {
Expand Down Expand Up @@ -468,32 +454,34 @@ func getJSONSpec(message proto.Message) string {
return jsonSpec
}

func getFlyteTestManifest(org, repository string) ([]FlyteSnack, string, error) {
func filterExampleFromRelease(releases github.RepositoryRelease) []github.ReleaseAsset {
var assets []github.ReleaseAsset
for _, v := range releases.Assets {
if strings.HasSuffix(*v.Name, ".tgz") {
assets = append(assets, v)
}
}
return assets
}

func getAllFlytesnacksExample(org, repository, release string) ([]github.ReleaseAsset, string, error) {
c := github.NewClient(nil)
opt := &github.ListOptions{Page: 1, PerPage: 1}
if len(release) > 0 {
releases, _, err := c.Repositories.GetReleaseByTag(context.Background(), org, repository, release)
if err != nil {
return nil, "", err
}
return filterExampleFromRelease(*releases), release, nil
}
releases, _, err := c.Repositories.ListReleases(context.Background(), org, repository, opt)
if err != nil {
return nil, "", err
}
if len(releases) == 0 {
return nil, "", fmt.Errorf("Repository doesn't have any release")
return nil, "", fmt.Errorf("repository doesn't have any release")
}
response, err := http.Get(fmt.Sprintf(flyteManifest, *releases[0].TagName))
if err != nil {
return nil, "", err
}
defer response.Body.Close()

data, err := ioutil.ReadAll(response.Body)
if err != nil {
return nil, "", err
}

err = json.Unmarshal(data, &FlyteSnacksRelease)
if err != nil {
return nil, "", err
}
return FlyteSnacksRelease, *releases[0].TagName, nil
return filterExampleFromRelease(*releases[0]), *releases[0].TagName, nil

}

Expand Down Expand Up @@ -580,3 +568,10 @@ func segregateSourceAndProtos(dataRefs []string) (string, []string, []string) {
}
return sourceCode, validProto, InvalidFiles
}

func deprecatedCheck(ctx context.Context) {
if len(rconfig.DefaultFilesConfig.K8ServiceAccount) > 0 {
logger.Warning(ctx, "--K8ServiceAccount is deprecated, Please use --K8sServiceAccount")
rconfig.DefaultFilesConfig.K8sServiceAccount = rconfig.DefaultFilesConfig.K8ServiceAccount
}
}
33 changes: 13 additions & 20 deletions flytectl/cmd/register/register_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func registerFilesSetup() {
cmdCtx = cmdCore.NewCommandContext(mockAdminClient, u.MockOutStream)

rconfig.DefaultFilesConfig.AssumableIamRole = ""
rconfig.DefaultFilesConfig.K8ServiceAccount = ""
rconfig.DefaultFilesConfig.K8sServiceAccount = ""
rconfig.DefaultFilesConfig.OutputLocationPrefix = ""
}

Expand Down Expand Up @@ -278,19 +278,19 @@ func TestHydrateLaunchPlanSpec(t *testing.T) {
hydrateLaunchPlanSpec(lpSpec)
assert.Equal(t, &admin.AuthRole{AssumableIamRole: "iamRole"}, lpSpec.AuthRole)
})
t.Run("k8Service account override", func(t *testing.T) {
t.Run("k8sService account override", func(t *testing.T) {
setup()
registerFilesSetup()
rconfig.DefaultFilesConfig.K8ServiceAccount = "k8Account"
rconfig.DefaultFilesConfig.K8sServiceAccount = "k8Account"
lpSpec := &admin.LaunchPlanSpec{}
hydrateLaunchPlanSpec(lpSpec)
assert.Equal(t, &admin.AuthRole{KubernetesServiceAccount: "k8Account"}, lpSpec.AuthRole)
})
t.Run("Both k8Service and IamRole", func(t *testing.T) {
t.Run("Both k8sService and IamRole", func(t *testing.T) {
setup()
registerFilesSetup()
rconfig.DefaultFilesConfig.AssumableIamRole = "iamRole"
rconfig.DefaultFilesConfig.K8ServiceAccount = "k8Account"
rconfig.DefaultFilesConfig.K8sServiceAccount = "k8Account"
lpSpec := &admin.LaunchPlanSpec{}
hydrateLaunchPlanSpec(lpSpec)
assert.Equal(t, &admin.AuthRole{AssumableIamRole: "iamRole",
Expand All @@ -306,13 +306,6 @@ func TestHydrateLaunchPlanSpec(t *testing.T) {
})
}

func TestFlyteManifest(t *testing.T) {
_, tag, err := getFlyteTestManifest(githubOrg, githubRepository)
assert.Nil(t, err)
assert.Contains(t, tag, "v")
assert.NotEmpty(t, tag)
}

func TestUploadFastRegisterArtifact(t *testing.T) {
t.Run("Successful upload", func(t *testing.T) {
testScope := promutils.NewTestScope()
Expand Down Expand Up @@ -358,22 +351,22 @@ func TestGetStorageClient(t *testing.T) {
})
}

func TestGetFlyteTestManifest(t *testing.T) {
func TestGetAllFlytesnacksExample(t *testing.T) {
t.Run("Failed to get manifest with wrong name", func(t *testing.T) {
_, tag, err := getFlyteTestManifest("no////ne", "no////ne")
_, tag, err := getAllFlytesnacksExample("no////ne", "no////ne", "")
assert.NotNil(t, err)
assert.Equal(t, len(tag), 0)
})
t.Run("Failed to get release", func(t *testing.T) {
_, tag, err := getFlyteTestManifest("flyteorg", "homebrew-tap")
_, tag, err := getAllFlytesnacksExample("flyteorg", "homebrew-tap", "")
assert.NotNil(t, err)
assert.Equal(t, len(tag), 0)
})
t.Run("Failed to get manifest", func(t *testing.T) {
flyteManifest = ""
_, tag, err := getFlyteTestManifest("flyteorg", "flytesnacks")
assert.NotNil(t, err)
assert.Equal(t, len(tag), 0)
t.Run("Successfully get examples", func(t *testing.T) {
assets, tag, err := getAllFlytesnacksExample("flyteorg", "flytesnacks", "v0.2.175")
assert.Nil(t, err)
assert.Greater(t, len(tag), 0)
assert.Greater(t, len(assets), 0)
})
}

Expand Down

0 comments on commit 24710f7

Please sign in to comment.