Fixed the non-zero return code issue in archive path (flyteorg#194)

robert-ulbrich-mercedes-benz · Jul 2, 2024 · 2850004 · 2850004
1 parent c2d6697
commit 2850004
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 6 deletions.
diff --git a/flytectl/cmd/register/files.go b/flytectl/cmd/register/files.go
@@ -96,7 +96,7 @@ func registerFromFilesFunc(ctx context.Context, args []string, cmdCtx cmdCore.Co
 }
 
 func Register(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error {
-	var _err error
+	var regErr error
 	var dataRefs []string
 
 	// Deprecated checks for --k8Service
@@ -131,17 +131,18 @@ func Register(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext)
 
 	var registerResults []Result
 	fastFail := rconfig.DefaultFilesConfig.ContinueOnError
-	for i := 0; i < len(validProto) && !(fastFail && _err != nil); i++ {
-		registerResults, _err = registerFile(ctx, validProto[i], sourceCodeName, registerResults, cmdCtx)
+	for i := 0; i < len(validProto) && !(fastFail && regErr != nil); i++ {
+		registerResults, regErr = registerFile(ctx, validProto[i], sourceCodeName, registerResults, cmdCtx)
 	}
 
 	payload, _ := json.Marshal(registerResults)
 	registerPrinter := printer.Printer{}
 	_ = registerPrinter.JSONToTable(payload, projectColumns)
 	if tmpDir != "" {
-		if _err = os.RemoveAll(tmpDir); _err != nil {
+		if _err := os.RemoveAll(tmpDir); _err != nil {
 			logger.Errorf(ctx, "unable to delete temp dir %v due to %v", tmpDir, _err)
+			return _err
 		}
 	}
-	return _err
+	return regErr
 }
diff --git a/flytectl/cmd/register/files_test.go b/flytectl/cmd/register/files_test.go
@@ -1,6 +1,7 @@
 package register
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/flyteorg/flytestdlib/contextutils"
@@ -71,7 +72,7 @@ func TestRegisterFromFiles(t *testing.T) {
 		err = Register(ctx, args, cmdCtx)
 		assert.Nil(t, err)
 	})
-	t.Run("Failed registeration because of invalid files", func(t *testing.T) {
+	t.Run("Failed registration because of invalid files", func(t *testing.T) {
 		setup()
 		registerFilesSetup()
 		testScope := promutils.NewTestScope()
@@ -90,7 +91,28 @@ func TestRegisterFromFiles(t *testing.T) {
 		err = registerFromFilesFunc(ctx, args, cmdCtx)
 		assert.NotNil(t, err)
 	})
+	t.Run("Failure registration of fast serialize", func(t *testing.T) {
+		setup()
+		registerFilesSetup()
+		testScope := promutils.NewTestScope()
+		labeled.SetMetricKeys(contextutils.AppNameKey, contextutils.ProjectKey, contextutils.DomainKey)
+		rconfig.DefaultFilesConfig.Archive = true
 
+		rconfig.DefaultFilesConfig.OutputLocationPrefix = s3Output
+		rconfig.DefaultFilesConfig.SourceUploadPath = s3Output
+		s, err := storage.NewDataStore(&storage.Config{
+			Type: storage.TypeMemory,
+		}, testScope.NewSubScope("flytectl"))
+		Client = s
+		assert.Nil(t, err)
+		args = []string{"testdata/flytesnacks-core.tgz"}
+		mockAdminClient.OnCreateTaskMatch(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("failed"))
+		mockAdminClient.OnCreateWorkflowMatch(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("failed"))
+		mockAdminClient.OnCreateLaunchPlanMatch(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("failed"))
+		err = registerFromFilesFunc(ctx, args, cmdCtx)
+		assert.NotNil(t, err)
+		assert.Equal(t, fmt.Errorf("failed"), err)
+	})
 	t.Run("Valid registration of fast serialize", func(t *testing.T) {
 		setup()
 		registerFilesSetup()