From 285aeeba8c4aa48226bda5c662372148714c09f5 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Thu, 27 Aug 2020 17:23:12 -0500 Subject: [PATCH] Refactor document pull command This command was refactored for usage with new config factory. Config object now is being initialized in pkg module on demand, which provides more flexibility and cleaner code. Change-Id: Iff75eb66db2fee922e6db1bf2930892c02c3b601 Signed-off-by: Ruslan Aliev Relates-To: #327 --- cmd/document/document.go | 5 ++- cmd/document/document_test.go | 4 ++- cmd/document/pull.go | 10 ++---- pkg/document/pull/pull.go | 19 +++++------ pkg/document/pull/pull_test.go | 59 ++++++++++++++++++---------------- 5 files changed, 52 insertions(+), 45 deletions(-) diff --git a/cmd/document/document.go b/cmd/document/document.go index 288d145c3..bda9d93ea 100644 --- a/cmd/document/document.go +++ b/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 diff --git a/cmd/document/document_test.go b/cmd/document/document_test.go index db910d00e..187f0cdb4 100644 --- a/cmd/document/document_test.go +++ b/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", diff --git a/cmd/document/pull.go b/cmd/document/pull.go index 22f155885..b3faa3a9b 100644 --- a/cmd/document/pull.go +++ b/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 { - settings := pull.Settings{AirshipCTLSettings: rootSettings} +func NewPullCommand(cfgFactory config.Factory) *cobra.Command { 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 - rootSettings.InitConfig() - - return settings.Pull() + return pull.Pull(cfgFactory) }, } diff --git a/pkg/document/pull/pull.go b/pkg/document/pull/pull.go index a8c708de4..5a1fe3369 100644 --- a/pkg/document/pull/pull.go +++ b/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 { - if err := s.Config.EnsureComplete(); err != nil { - return err - } - err := s.cloneRepositories() +func Pull(cfgFactory config.Factory) error { + cfg, err := cfgFactory() 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() - log.Debugf("Reading current context manifest information from %s", s.AirshipConfigPath) + currentManifest, err := s.CurrentContextManifest() + log.Debugf("Reading current context manifest information from %s", s.LoadedConfigPath()) if err != nil { return err } diff --git a/pkg/document/pull/pull_test.go b/pkg/document/pull/pull_test.go index 1cbdf5a98..44e84e1fa 100644 --- a/pkg/document/pull/pull_test.go +++ b/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 { - return &Settings{ - AirshipCTLSettings: &environment.AirshipCTLSettings{ - Config: testutil.DummyConfig(), - }, +func mockConfigFactory(t *testing.T, testGitDir string, chkOutOpts *config.RepoCheckout, tmpDir string) config.Factory { + return func() (*config.Config, error) { + cfg := 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{ - currentManifest.PrimaryRepositoryName: { - URLString: testGitDir, - CheckoutOptions: chkOutOpts, - Auth: &config.RepoAuth{ - Type: "http-basic", - }, - }, - } - - currentManifest.TargetPath = tmpDir - - _, err = repo.NewRepository( - ".", - currentManifest.Repositories[currentManifest.PrimaryRepositoryName], - ) + cfgFactory := mockConfigFactory(t, testGitDir, tt.checkoutOpts, tmpDir) + cfg, err := cfgFactory() + require.NoError(err) + currentManifest, err := cfg.CurrentContextManifest() require.NoError(err) - err = dummyPullSettings.Pull() + err = pull.Pull(cfgFactory) if expectedErr != nil { assert.NotNil(err) assert.Equal(expectedErr, err)