From 24710f743e8cac29290ca38a4aff1975d39a833f Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Fri, 3 Sep 2021 12:19:52 +0530 Subject: [PATCH] Hotfix register example (#172) * fix register example Signed-off-by: Yuvraj --- .../subcommand/register/files_config.go | 3 +- .../subcommand/register/filesconfig_flags.go | 3 +- .../register/filesconfig_flags_test.go | 14 ++++ flytectl/cmd/register/examples.go | 33 ++++++++-- flytectl/cmd/register/examples_test.go | 4 +- flytectl/cmd/register/files.go | 3 + flytectl/cmd/register/register_util.go | 65 +++++++++---------- flytectl/cmd/register/register_util_test.go | 33 ++++------ 8 files changed, 92 insertions(+), 66 deletions(-) diff --git a/flytectl/cmd/config/subcommand/register/files_config.go b/flytectl/cmd/config/subcommand/register/files_config.go index c489dd38ed..8aea9f099b 100644 --- a/flytectl/cmd/config/subcommand/register/files_config.go +++ b/flytectl/cmd/config/subcommand/register/files_config.go @@ -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."` diff --git a/flytectl/cmd/config/subcommand/register/filesconfig_flags.go b/flytectl/cmd/config/subcommand/register/filesconfig_flags.go index cd19e456cc..5727138104 100755 --- a/flytectl/cmd/config/subcommand/register/filesconfig_flags.go +++ b/flytectl/cmd/config/subcommand/register/filesconfig_flags.go @@ -54,7 +54,8 @@ func (cfg FilesConfig) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.BoolVar(&DefaultFilesConfig.ContinueOnError, fmt.Sprintf("%v%v", prefix, "continueOnError"), DefaultFilesConfig.ContinueOnError, "continue on error when registering files.") cmdFlags.BoolVar(&DefaultFilesConfig.Archive, fmt.Sprintf("%v%v", prefix, "archive"), DefaultFilesConfig.Archive, "pass in archive file either an http link or local path.") cmdFlags.StringVar(&DefaultFilesConfig.AssumableIamRole, fmt.Sprintf("%v%v", prefix, "assumableIamRole"), DefaultFilesConfig.AssumableIamRole, " custom assumable iam auth role to register launch plans with.") - cmdFlags.StringVar(&DefaultFilesConfig.K8ServiceAccount, fmt.Sprintf("%v%v", prefix, "k8ServiceAccount"), DefaultFilesConfig.K8ServiceAccount, " custom kubernetes service account auth role to register launch plans with.") + cmdFlags.StringVar(&DefaultFilesConfig.K8sServiceAccount, fmt.Sprintf("%v%v", prefix, "k8sServiceAccount"), DefaultFilesConfig.K8sServiceAccount, " custom kubernetes service account auth role to register launch plans with.") + cmdFlags.StringVar(&DefaultFilesConfig.K8ServiceAccount, fmt.Sprintf("%v%v", prefix, "k8ServiceAccount"), DefaultFilesConfig.K8ServiceAccount, " deprecated. Please use --K8sServiceAccount") cmdFlags.StringVar(&DefaultFilesConfig.OutputLocationPrefix, fmt.Sprintf("%v%v", prefix, "outputLocationPrefix"), DefaultFilesConfig.OutputLocationPrefix, " custom output location prefix for offloaded types (files/schemas).") cmdFlags.StringVar(&DefaultFilesConfig.SourceUploadPath, fmt.Sprintf("%v%v", prefix, "sourceUploadPath"), DefaultFilesConfig.SourceUploadPath, " Location for source code in storage.") cmdFlags.BoolVar(&DefaultFilesConfig.DryRun, fmt.Sprintf("%v%v", prefix, "dryRun"), DefaultFilesConfig.DryRun, "execute command without making any modifications.") diff --git a/flytectl/cmd/config/subcommand/register/filesconfig_flags_test.go b/flytectl/cmd/config/subcommand/register/filesconfig_flags_test.go index 951cfbaecc..6f46245d9f 100755 --- a/flytectl/cmd/config/subcommand/register/filesconfig_flags_test.go +++ b/flytectl/cmd/config/subcommand/register/filesconfig_flags_test.go @@ -155,6 +155,20 @@ func TestFilesConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_k8sServiceAccount", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("k8sServiceAccount", testValue) + if vString, err := cmdFlags.GetString("k8sServiceAccount"); err == nil { + testDecodeJson_FilesConfig(t, fmt.Sprintf("%v", vString), &actual.K8sServiceAccount) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_k8ServiceAccount", func(t *testing.T) { t.Run("Override", func(t *testing.T) { diff --git a/flytectl/cmd/register/examples.go b/flytectl/cmd/register/examples.go index 0952571c12..264324876d 100644 --- a/flytectl/cmd/register/examples.go +++ b/flytectl/cmd/register/examples.go @@ -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" ) @@ -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) diff --git a/flytectl/cmd/register/examples_test.go b/flytectl/cmd/register/examples_test.go index 1c004a74a9..5b38de8196 100644 --- a/flytectl/cmd/register/examples_test.go +++ b/flytectl/cmd/register/examples_test.go @@ -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" } diff --git a/flytectl/cmd/register/files.go b/flytectl/cmd/register/files.go index 210bb1bba2..99d8a99090 100644 --- a/flytectl/cmd/register/files.go +++ b/flytectl/cmd/register/files.go @@ -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 { diff --git a/flytectl/cmd/register/register_util.go b/flytectl/cmd/register/register_util.go index 07f62d6246..fa51cc29c8 100644 --- a/flytectl/cmd/register/register_util.go +++ b/flytectl/cmd/register/register_util.go @@ -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() { @@ -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 { @@ -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 } @@ -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 + } +} diff --git a/flytectl/cmd/register/register_util_test.go b/flytectl/cmd/register/register_util_test.go index b4b11563a4..bb85c1c22f 100644 --- a/flytectl/cmd/register/register_util_test.go +++ b/flytectl/cmd/register/register_util_test.go @@ -53,7 +53,7 @@ func registerFilesSetup() { cmdCtx = cmdCore.NewCommandContext(mockAdminClient, u.MockOutStream) rconfig.DefaultFilesConfig.AssumableIamRole = "" - rconfig.DefaultFilesConfig.K8ServiceAccount = "" + rconfig.DefaultFilesConfig.K8sServiceAccount = "" rconfig.DefaultFilesConfig.OutputLocationPrefix = "" } @@ -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", @@ -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() @@ -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) }) }