From 233bbda0e0ae7a0d1e9c270383a9479b51ce3aac Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Sat, 29 Aug 2020 19:39:36 -0500 Subject: [PATCH] Remove pkg/environment module This module is no longer needed since all config preloading actions were moved to config module and global flags are stored within root level cmd. Change-Id: I411f6717e5b3d2998706c35a82f1e7f1b2aef3a8 Signed-off-by: Ruslan Aliev Relates-To: #327 --- cmd/cluster/init_test.go | 12 +-- pkg/clusterctl/cmd/command_test.go | 20 ++--- pkg/environment/constants.go | 18 ---- pkg/environment/settings.go | 119 ------------------------- pkg/environment/settings_test.go | 138 ----------------------------- pkg/k8s/poller/poller_test.go | 8 +- pkg/phase/apply/apply_test.go | 12 +-- pkg/phase/phase_test.go | 42 +++++---- 8 files changed, 34 insertions(+), 335 deletions(-) delete mode 100644 pkg/environment/constants.go delete mode 100644 pkg/environment/settings.go delete mode 100644 pkg/environment/settings_test.go diff --git a/cmd/cluster/init_test.go b/cmd/cluster/init_test.go index d615c3ef0..ed944be90 100755 --- a/cmd/cluster/init_test.go +++ b/cmd/cluster/init_test.go @@ -18,25 +18,15 @@ import ( "testing" "opendev.org/airship/airshipctl/cmd/cluster" - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/testutil" ) func TestNewClusterInitCmd(t *testing.T) { - fakeRootSettings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "../../testdata/k8s/config.yaml", - KubeConfigPath: "../../testdata/k8s/kubeconfig.yaml", - } - fakeRootSettings.InitConfig() - tests := []*testutil.CmdTest{ { Name: "cluster-init-cmd-with-help", CmdLine: "--help", - Cmd: cluster.NewInitCommand(func() (*config.Config, error) { - return fakeRootSettings.Config, nil - }), + Cmd: cluster.NewInitCommand(nil), }, } for _, testcase := range tests { diff --git a/pkg/clusterctl/cmd/command_test.go b/pkg/clusterctl/cmd/command_test.go index bc13ee4fe..db3f65176 100644 --- a/pkg/clusterctl/cmd/command_test.go +++ b/pkg/clusterctl/cmd/command_test.go @@ -21,7 +21,6 @@ import ( "github.com/stretchr/testify/require" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" ) const ( @@ -31,11 +30,11 @@ const ( // TODO (kkalynovskyi) expand test cases func TestNewCommand(t *testing.T) { - rs := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/airshipconfig.yaml", - KubeConfigPath: "testdata/kubeconfig.yaml", - } - rs.InitConfig() + airshipConfigPath := "testdata/airshipconfig.yaml" + kubeConfigPath := "testdata/kubeconfig.yaml" + cfg, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) + tests := []struct { name string expectErr bool @@ -112,16 +111,15 @@ func TestNewCommand(t *testing.T) { }, } for _, tt := range tests { - rs.InitConfig() expectErr := tt.expectErr manifests := tt.manifests - rs.Config.Manifests = manifests + cfg.Manifests = manifests context := tt.currentConext t.Run(tt.name, func(t *testing.T) { - rs.Config.Manifests = manifests - rs.Config.CurrentContext = context + cfg.Manifests = manifests + cfg.CurrentContext = context command, err := NewCommand(func() (*config.Config, error) { - return rs.Config, nil + return cfg, nil }) if expectErr { assert.Error(t, err) diff --git a/pkg/environment/constants.go b/pkg/environment/constants.go deleted file mode 100644 index fa926ee89..000000000 --- a/pkg/environment/constants.go +++ /dev/null @@ -1,18 +0,0 @@ -/* - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - https://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package environment - -// HomeEnvVar holds value of HOME directory from env -const HomeEnvVar = "$HOME" diff --git a/pkg/environment/settings.go b/pkg/environment/settings.go deleted file mode 100644 index d3e3dee03..000000000 --- a/pkg/environment/settings.go +++ /dev/null @@ -1,119 +0,0 @@ -/* - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - https://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package environment - -import ( - "os" - "path/filepath" - - "github.com/spf13/cobra" - - "k8s.io/client-go/tools/clientcmd" - - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/log" - "opendev.org/airship/airshipctl/pkg/util" -) - -// AirshipCTLSettings is a container for all of the settings needed by airshipctl -type AirshipCTLSettings struct { - AirshipConfigPath string - KubeConfigPath string - Create bool - Config *config.Config -} - -// InitFlags adds the default settings flags to cmd -func (a *AirshipCTLSettings) InitFlags(cmd *cobra.Command) { - flags := cmd.PersistentFlags() - - defaultAirshipConfigDir := filepath.Join(HomeEnvVar, config.AirshipConfigDir) - - defaultAirshipConfigPath := filepath.Join(defaultAirshipConfigDir, config.AirshipConfig) - flags.StringVar( - &a.AirshipConfigPath, - "airshipconf", - "", - `Path to file for airshipctl configuration. (default "`+defaultAirshipConfigPath+`")`) - - defaultKubeConfigPath := filepath.Join(defaultAirshipConfigDir, config.AirshipKubeConfig) - flags.StringVar( - &a.KubeConfigPath, - clientcmd.RecommendedConfigPathFlag, - "", - `Path to kubeconfig associated with airshipctl configuration. (default "`+defaultKubeConfigPath+`")`) - - a.Create = false -} - -// InitConfig - Initializes and loads Config if exists. -// TODO (raliev) remove this function after completely switching to new approach of Config loading -func (a *AirshipCTLSettings) InitConfig() { - a.Config = config.NewConfig() - - a.InitAirshipConfigPath() - a.InitKubeConfigPath() - - err := a.Config.LoadConfig(a.AirshipConfigPath, a.KubeConfigPath, a.Create) - if err != nil { - // Should stop airshipctl - log.Fatal("Failed to load or initialize config: ", err) - } -} - -// InitAirshipConfigPath - Initializes AirshipConfigPath variable for Config object -// TODO (raliev) remove this function after completely switching to new approach of Config loading -func (a *AirshipCTLSettings) InitAirshipConfigPath() { - // The airshipConfigPath may already have been received as a command line argument - if a.AirshipConfigPath != "" { - return - } - - // Otherwise, we can check if we got the path via ENVIRONMENT variable - a.AirshipConfigPath = os.Getenv(config.AirshipConfigEnv) - if a.AirshipConfigPath != "" { - return - } - - // Otherwise, we'll try putting it in the home directory - homeDir := util.UserHomeDir() - a.AirshipConfigPath = filepath.Join(homeDir, config.AirshipConfigDir, config.AirshipConfig) -} - -// InitKubeConfigPath - Initializes KubeConfigPath variable for Config object -// TODO (raliev) remove this function after completely switching to new approach of Config loading -func (a *AirshipCTLSettings) InitKubeConfigPath() { - // NOTE(howell): This function will set the kubeConfigPath to the - // default location under the airship directory unless the user - // *explicitly* specifies a different location, either by setting the - // ENVIRONMENT variable or by passing a command line argument. - // This avoids messing up the user's kubeconfig if they didn't - // explicitly want airshipctl to use it. - - // The kubeConfigPath may already have been received as a command line argument - if a.KubeConfigPath != "" { - return - } - - // Otherwise, we can check if we got the path via ENVIRONMENT variable - a.KubeConfigPath = os.Getenv(config.AirshipKubeConfigEnv) - if a.KubeConfigPath != "" { - return - } - - // Otherwise, we'll try putting it in the home directory - homeDir := util.UserHomeDir() - a.KubeConfigPath = filepath.Join(homeDir, config.AirshipConfigDir, config.AirshipKubeConfig) -} diff --git a/pkg/environment/settings_test.go b/pkg/environment/settings_test.go deleted file mode 100644 index 9aa030e33..000000000 --- a/pkg/environment/settings_test.go +++ /dev/null @@ -1,138 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package environment_test - -import ( - "os" - "path/filepath" - "testing" - - "github.com/spf13/cobra" - "github.com/stretchr/testify/assert" - - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" - "opendev.org/airship/airshipctl/testutil" -) - -func TestInitFlags(t *testing.T) { - // Get the Environment - settings := &environment.AirshipCTLSettings{} - testCmd := &cobra.Command{} - settings.InitFlags(testCmd) - assert.True(t, testCmd.HasPersistentFlags()) -} - -func TestInitConfig(t *testing.T) { - t.Run("DefaultToHomeDirectory", func(subTest *testing.T) { - // Set up a fake $HOME directory - testDir, cleanup := testutil.TempDir(t, "test-home") - defer cleanup(t) - defer setHome(testDir)() - - var testSettings environment.AirshipCTLSettings - expectedAirshipConfig := filepath.Join(testDir, config.AirshipConfigDir, config.AirshipConfig) - expectedKubeConfig := filepath.Join(testDir, config.AirshipConfigDir, config.AirshipKubeConfig) - - testSettings.InitAirshipConfigPath() - testSettings.InitKubeConfigPath() - assert.Equal(t, expectedAirshipConfig, testSettings.AirshipConfigPath) - assert.Equal(t, expectedKubeConfig, testSettings.KubeConfigPath) - }) - - t.Run("PreferEnvToDefault", func(subTest *testing.T) { - // Set up a fake $HOME directory - testDir, cleanup := testutil.TempDir(t, "test-home") - defer cleanup(t) - defer setHome(testDir)() - - var testSettings environment.AirshipCTLSettings - expectedAirshipConfig := filepath.Join(testDir, "airshipEnv") - expectedKubeConfig := filepath.Join(testDir, "kubeEnv") - - os.Setenv(config.AirshipConfigEnv, expectedAirshipConfig) - os.Setenv(config.AirshipKubeConfigEnv, expectedKubeConfig) - defer os.Unsetenv(config.AirshipConfigEnv) - defer os.Unsetenv(config.AirshipKubeConfigEnv) - - testSettings.Create = true - testSettings.InitConfig() - - assert.Equal(t, expectedAirshipConfig, testSettings.AirshipConfigPath) - assert.Equal(t, expectedKubeConfig, testSettings.KubeConfigPath) - }) - - t.Run("PreferCmdLineArgToDefault", func(subTest *testing.T) { - // Set up a fake $HOME directory - testDir, cleanup := testutil.TempDir(t, "test-home") - defer cleanup(t) - defer setHome(testDir)() - - expectedAirshipConfig := filepath.Join(testDir, "airshipCmdLine") - expectedKubeConfig := filepath.Join(testDir, "kubeCmdLine") - - testSettings := environment.AirshipCTLSettings{ - AirshipConfigPath: expectedAirshipConfig, - KubeConfigPath: expectedKubeConfig, - } - - testSettings.InitAirshipConfigPath() - testSettings.InitKubeConfigPath() - assert.Equal(t, expectedAirshipConfig, testSettings.AirshipConfigPath) - assert.Equal(t, expectedKubeConfig, testSettings.KubeConfigPath) - }) - - t.Run("PreferCmdLineArgToEnv", func(subTest *testing.T) { - // Set up a fake $HOME directory - testDir, cleanup := testutil.TempDir(t, "test-home") - defer cleanup(t) - defer setHome(testDir)() - - expectedAirshipConfig := filepath.Join(testDir, "airshipCmdLine") - expectedKubeConfig := filepath.Join(testDir, "kubeCmdLine") - - // set up "decoy" environment variables. These should be - // ignored, since we're simulating passing in command line - // arguments - wrongAirshipConfig := filepath.Join(testDir, "wrongAirshipConfig") - wrongKubeConfig := filepath.Join(testDir, "wrongKubeConfig") - os.Setenv(config.AirshipConfigEnv, wrongAirshipConfig) - os.Setenv(config.AirshipKubeConfigEnv, wrongKubeConfig) - defer os.Unsetenv(config.AirshipConfigEnv) - defer os.Unsetenv(config.AirshipKubeConfigEnv) - - testSettings := environment.AirshipCTLSettings{ - AirshipConfigPath: expectedAirshipConfig, - KubeConfigPath: expectedKubeConfig, - } - - testSettings.InitAirshipConfigPath() - testSettings.InitKubeConfigPath() - assert.Equal(t, expectedAirshipConfig, testSettings.AirshipConfigPath) - assert.Equal(t, expectedKubeConfig, testSettings.KubeConfigPath) - }) -} - -// setHome sets the HOME environment variable to `path`, and returns a function -// that can be used to reset HOME to its original value -func setHome(path string) (resetHome func()) { - oldHome := os.Getenv("HOME") - os.Setenv("HOME", path) - return func() { - os.Setenv("HOME", oldHome) - } -} diff --git a/pkg/k8s/poller/poller_test.go b/pkg/k8s/poller/poller_test.go index 44bb797bc..54b327c09 100755 --- a/pkg/k8s/poller/poller_test.go +++ b/pkg/k8s/poller/poller_test.go @@ -22,21 +22,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "opendev.org/airship/airshipctl/pkg/cluster" - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/k8s/client/fake" "opendev.org/airship/airshipctl/pkg/k8s/poller" k8sutils "opendev.org/airship/airshipctl/pkg/k8s/utils" ) func TestNewStatusPoller(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - Config: config.NewConfig(), - KubeConfigPath: "testdata/kubeconfig.yaml", - } airClient := fake.NewClient() - f := k8sutils.FactoryFromKubeConfigPath(settings.KubeConfigPath) + f := k8sutils.FactoryFromKubeConfigPath("testdata/kubeconfig.yaml") restConfig, err := f.ToRESTConfig() require.NoError(t, err) restMapper, err := f.ToRESTMapper() diff --git a/pkg/phase/apply/apply_test.go b/pkg/phase/apply/apply_test.go index b1797326b..eeab7455e 100644 --- a/pkg/phase/apply/apply_test.go +++ b/pkg/phase/apply/apply_test.go @@ -27,7 +27,6 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/k8s/applier" "opendev.org/airship/airshipctl/pkg/phase/apply" @@ -122,13 +121,8 @@ func makeNewFakeRootSettings(t *testing.T, kp string, dir string) *config.Config adir, err := filepath.Abs(dir) require.NoError(t, err) - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: adir, - KubeConfigPath: akp, - } + settings, err := config.CreateFactory(&adir, &akp)() + require.NoError(t, err) - settings.InitConfig() - settings.Config.SetKubeConfigPath(kp) - settings.Config.SetLoadedConfigPath(dir) - return settings.Config + return settings } diff --git a/pkg/phase/phase_test.go b/pkg/phase/phase_test.go index bf77e8515..28e359808 100644 --- a/pkg/phase/phase_test.go +++ b/pkg/phase/phase_test.go @@ -30,7 +30,6 @@ import ( airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/k8s/applier" "opendev.org/airship/airshipctl/pkg/phase" "opendev.org/airship/airshipctl/pkg/phase/ifc" @@ -39,14 +38,14 @@ import ( func TestPhasePlan(t *testing.T) { testCases := []struct { name string - settings func() *config.Config + settings func(t *testing.T) *config.Config expectedPlan map[string][]string expectedErr error }{ { name: "No context", - settings: func() *config.Config { - s := makeDefaultSettings() + settings: func(t *testing.T) *config.Config { + s := makeDefaultSettings(t) s.CurrentContext = "badCtx" return s }, @@ -67,8 +66,8 @@ func TestPhasePlan(t *testing.T) { }, { name: "No Phase Plan", - settings: func() *config.Config { - s := makeDefaultSettings() + settings: func(t *testing.T) *config.Config { + s := makeDefaultSettings(t) m, err := s.CurrentContextManifest() require.NoError(t, err) m.SubPath = "no_plan_site" @@ -92,7 +91,7 @@ func TestPhasePlan(t *testing.T) { for _, test := range testCases { tt := test t.Run(tt.name, func(t *testing.T) { - cmd := phase.Cmd{Config: tt.settings()} + cmd := phase.Cmd{Config: tt.settings(t)} actualPlan, actualErr := cmd.Plan() assert.Equal(t, tt.expectedErr, actualErr) assert.Equal(t, tt.expectedPlan, actualPlan) @@ -103,15 +102,15 @@ func TestPhasePlan(t *testing.T) { func TestGetPhase(t *testing.T) { testCases := []struct { name string - settings func() *config.Config + settings func(t *testing.T) *config.Config phaseName string expectedPhase *airshipv1.Phase expectedErr error }{ { name: "No context", - settings: func() *config.Config { - s := makeDefaultSettings() + settings: func(t *testing.T) *config.Config { + s := makeDefaultSettings(t) s.CurrentContext = "badCtx" return s }, @@ -161,7 +160,7 @@ func TestGetPhase(t *testing.T) { for _, test := range testCases { tt := test t.Run(tt.name, func(t *testing.T) { - cmd := phase.Cmd{Config: tt.settings()} + cmd := phase.Cmd{Config: tt.settings(t)} actualPhase, actualErr := cmd.GetPhase(tt.phaseName) assert.Equal(t, tt.expectedErr, actualErr) assert.Equal(t, tt.expectedPhase, actualPhase) @@ -172,15 +171,15 @@ func TestGetPhase(t *testing.T) { func TestGetExecutor(t *testing.T) { testCases := []struct { name string - settings func() *config.Config + settings func(t *testing.T) *config.Config phase *airshipv1.Phase expectedExc ifc.Executor expectedErr error }{ { name: "No context", - settings: func() *config.Config { - s := makeDefaultSettings() + settings: func(t *testing.T) *config.Config { + s := makeDefaultSettings(t) s.CurrentContext = "badCtx" return s }, @@ -250,7 +249,7 @@ func TestGetExecutor(t *testing.T) { for _, test := range testCases { tt := test t.Run(tt.name, func(t *testing.T) { - cmd := phase.Cmd{Config: tt.settings()} + cmd := phase.Cmd{Config: tt.settings(t)} actualExc, actualErr := cmd.GetExecutor(tt.phase) assert.Equal(t, tt.expectedErr, actualErr) assertEqualExecutor(t, tt.expectedExc, actualExc) @@ -268,11 +267,10 @@ func assertEqualExecutor(t *testing.T, expected, actual ifc.Executor) { assert.IsType(t, expected, actual) } -func makeDefaultSettings() *config.Config { - testSettings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/airshipconfig.yaml", - KubeConfigPath: "testdata/kubeconfig.yaml", - } - testSettings.InitConfig() - return testSettings.Config +func makeDefaultSettings(t *testing.T) *config.Config { + airshipConfigPath := "testdata/airshipconfig.yaml" + kubeConfigPath := "testdata/kubeconfig.yaml" + testSettings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) + return testSettings }