Merge "Refactor document pull command"

cmd/document/document.go

@@ -17,6 +17,7 @@ package document
 import (
 	"github.com/spf13/cobra"
 
+	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/environment"
 )
 
@@ -27,7 +28,9 @@ func NewDocumentCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Com
 		Short: "Manage deployment documents",
 	}
 
-	documentRootCmd.AddCommand(NewPullCommand(rootSettings))
+	cfgFactory := config.CreateFactory(&rootSettings.AirshipConfigPath, &rootSettings.KubeConfigPath)
+
+	documentRootCmd.AddCommand(NewPullCommand(cfgFactory))
 	documentRootCmd.AddCommand(NewPluginCommand())
 
 	return documentRootCmd

cmd/document/document_test.go

@@ -18,15 +18,17 @@ import (
 	"testing"
 
 	"opendev.org/airship/airshipctl/cmd/document"
+	"opendev.org/airship/airshipctl/pkg/environment"
 	"opendev.org/airship/airshipctl/testutil"
 )
 
 func TestDocument(t *testing.T) {
+	rootSettings := &environment.AirshipCTLSettings{}
 	tests := []*testutil.CmdTest{
 		{
 			Name:    "document-with-help",
 			CmdLine: "-h",
-			Cmd:     document.NewDocumentCommand(nil),
+			Cmd:     document.NewDocumentCommand(rootSettings),
 		},
 		{
 			Name:    "document-plugin-with-help",

cmd/document/pull.go

@@ -17,21 +17,17 @@ package document
 import (
 	"github.com/spf13/cobra"
 
+	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document/pull"
-	"opendev.org/airship/airshipctl/pkg/environment"
 )
 
 // NewPullCommand creates a new command for pulling airship document repositories
-func NewPullCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command {
+func NewPullCommand(cfgFactory config.Factory) *cobra.Command {
-	settings := pull.Settings{AirshipCTLSettings: rootSettings}
 	documentPullCmd := &cobra.Command{
 		Use:   "pull",
 		Short: "Pulls documents from remote git repository",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// Load or Initialize airship Config
+			return pull.Pull(cfgFactory)
-			rootSettings.InitConfig()
-
-			return settings.Pull()
 		},
 	}
 

pkg/document/pull/pull.go

@@ -15,34 +15,35 @@
 package pull
 
 import (
+	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document/repo"
-	"opendev.org/airship/airshipctl/pkg/environment"
 	"opendev.org/airship/airshipctl/pkg/log"
 )
 
 // Settings is a reference to environment.AirshipCTLSettings
 // AirshipCTLSettings is a container for all of the settings needed by airshipctl
 type Settings struct {
-	*environment.AirshipCTLSettings
+	*config.Config
 }
 
 // Pull clones repositories
-func (s *Settings) Pull() error {
+func Pull(cfgFactory config.Factory) error {
-	if err := s.Config.EnsureComplete(); err != nil {
+	cfg, err := cfgFactory()
-		return err
-	}
-	err := s.cloneRepositories()
 	if err != nil {
 		return err
 	}
 
+	settings := &Settings{cfg}
+	if err = settings.cloneRepositories(); err != nil {
+		return err
+	}
 	return nil
 }
 
 func (s *Settings) cloneRepositories() error {
 	// Clone main repository
-	currentManifest, err := s.Config.CurrentContextManifest()
+	currentManifest, err := s.CurrentContextManifest()
-	log.Debugf("Reading current context manifest information from %s", s.AirshipConfigPath)
+	log.Debugf("Reading current context manifest information from %s", s.LoadedConfigPath())
 	if err != nil {
 		return err
 	}

pkg/document/pull/pull_test.go

@@ -12,10 +12,11 @@
  limitations under the License.
 */
 
-package pull
+package pull_test
 
 import (
 	"io/ioutil"
+
 	"path"
 	"strings"
 	"testing"
@@ -25,17 +26,36 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"opendev.org/airship/airshipctl/pkg/config"
+	"opendev.org/airship/airshipctl/pkg/document/pull"
 	"opendev.org/airship/airshipctl/pkg/document/repo"
-	"opendev.org/airship/airshipctl/pkg/environment"
 	"opendev.org/airship/airshipctl/pkg/util"
 	"opendev.org/airship/airshipctl/testutil"
 )
 
-func getDummyPullSettings() *Settings {
+func mockConfigFactory(t *testing.T, testGitDir string, chkOutOpts *config.RepoCheckout, tmpDir string) config.Factory {
-	return &Settings{
+	return func() (*config.Config, error) {
-		AirshipCTLSettings: &environment.AirshipCTLSettings{
+		cfg := testutil.DummyConfig()
-			Config: testutil.DummyConfig(),
+		currentManifest, err := cfg.CurrentContextManifest()
-		},
+		require.NoError(t, err)
+		currentManifest.Repositories = map[string]*config.Repository{
+			currentManifest.PrimaryRepositoryName: {
+				URLString:       testGitDir,
+				CheckoutOptions: chkOutOpts,
+				Auth: &config.RepoAuth{
+					Type: "http-basic",
+				},
+			},
+		}
+
+		currentManifest.TargetPath = tmpDir
+
+		_, err = repo.NewRepository(
+			".",
+			currentManifest.Repositories[currentManifest.PrimaryRepositoryName],
+		)
+		require.NoError(t, err)
+
+		return cfg, nil
 	}
 }
 
@@ -79,9 +99,6 @@ func TestPull(t *testing.T) {
 			error: config.ErrMutuallyExclusiveCheckout{},
 		},
 	}
-	dummyPullSettings := getDummyPullSettings()
-	currentManifest, err := dummyPullSettings.Config.CurrentContextManifest()
-	require.NoError(err)
 
 	testGitDir := fixtures.Basic().One().DotGit().Root()
 	dirNameFromURL := util.GitDirNameFromURL(testGitDir)
@@ -93,25 +110,13 @@ func TestPull(t *testing.T) {
 		expectedErr := tt.error
 		chkOutOpts := tt.checkoutOpts
 		t.Run(tt.name, func(t *testing.T) {
-			currentManifest.Repositories = map[string]*config.Repository{
+			cfgFactory := mockConfigFactory(t, testGitDir, tt.checkoutOpts, tmpDir)
-				currentManifest.PrimaryRepositoryName: {
+			cfg, err := cfgFactory()
-					URLString:       testGitDir,
+			require.NoError(err)
-					CheckoutOptions: chkOutOpts,
+			currentManifest, err := cfg.CurrentContextManifest()
-					Auth: &config.RepoAuth{
-						Type: "http-basic",
-					},
-				},
-			}
-
-			currentManifest.TargetPath = tmpDir
-
-			_, err = repo.NewRepository(
-				".",
-				currentManifest.Repositories[currentManifest.PrimaryRepositoryName],
-			)
 			require.NoError(err)
 
-			err = dummyPullSettings.Pull()
+			err = pull.Pull(cfgFactory)
 			if expectedErr != nil {
 				assert.NotNil(err)
 				assert.Equal(expectedErr, err)