From 410f9c4fd73e63e3b949de4d1aa9c6f27d9d7fea Mon Sep 17 00:00:00 2001
From: Kostiantyn Kalynovskyi <kkalynovskyi@mirantis.com>
Date: Wed, 9 Sep 2020 00:04:08 -0500
Subject: [PATCH] Use Helper in clusterctl and kube apply executors

Relates-To: #342

Change-Id: I6f3f9b5aac3877f5738f0ec077c65bcf5acfc54d
---
 pkg/clusterctl/client/executor.go            | 11 ++-----
 pkg/clusterctl/client/executor_test.go       | 33 +++++++++-----------
 pkg/clusterctl/client/testdata/metadata.yaml |  2 ++
 pkg/k8s/applier/executor.go                  |  5 ++-
 pkg/k8s/applier/executor_test.go             | 33 +++++++++++---------
 pkg/k8s/applier/testdata/metadata.yaml       |  2 ++
 6 files changed, 42 insertions(+), 44 deletions(-)
 create mode 100644 pkg/clusterctl/client/testdata/metadata.yaml
 create mode 100644 pkg/k8s/applier/testdata/metadata.yaml

diff --git a/pkg/clusterctl/client/executor.go b/pkg/clusterctl/client/executor.go
index e5cc0c7bd..33a45e9ed 100644
--- a/pkg/clusterctl/client/executor.go
+++ b/pkg/clusterctl/client/executor.go
@@ -16,7 +16,6 @@ package client
 
 import (
 	"io"
-	"path/filepath"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
@@ -34,7 +33,6 @@ var _ ifc.Executor = &ClusterctlExecutor{}
 // ClusterctlExecutor phase executor
 type ClusterctlExecutor struct {
 	clusterName string
-	dumpRoot    string
 
 	Interface
 	bundle  document.Bundle
@@ -59,11 +57,7 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 	if err := cfg.ExecutorDocument.ToAPIObject(options, airshipv1.Scheme); err != nil {
 		return nil, err
 	}
-	tgtPath, err := cfg.AirshipConfig.CurrentContextTargetPath()
-	if err != nil {
-		return nil, err
-	}
-	client, err := NewClient(tgtPath, log.DebugEnabled(), options)
+	client, err := NewClient(cfg.Helper.TargetPath(), log.DebugEnabled(), options)
 	if err != nil {
 		return nil, err
 	}
@@ -73,7 +67,6 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 		bundle:      cfg.ExecutorBundle,
 		options:     options,
 		kubecfg:     cfg.KubeConfig,
-		dumpRoot:    filepath.Dir(cfg.AirshipConfig.LoadedConfigPath()),
 	}, nil
 }
 
@@ -100,7 +93,7 @@ func (c *ClusterctlExecutor) init(opts ifc.RunOptions, evtCh chan events.Event)
 			Operation: events.ClusterctlInitStart,
 		},
 	}
-	kubeConfigFile, cleanup, err := c.kubecfg.WriteTempFile(c.dumpRoot)
+	kubeConfigFile, cleanup, err := c.kubecfg.GetFile()
 	if err != nil {
 		c.handleErr(err, evtCh)
 		return
diff --git a/pkg/clusterctl/client/executor_test.go b/pkg/clusterctl/client/executor_test.go
index be6f7e91d..ebc178e34 100644
--- a/pkg/clusterctl/client/executor_test.go
+++ b/pkg/clusterctl/client/executor_test.go
@@ -31,6 +31,7 @@ import (
 	airerrors "opendev.org/airship/airshipctl/pkg/errors"
 	"opendev.org/airship/airshipctl/pkg/events"
 	"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
+	"opendev.org/airship/airshipctl/pkg/phase"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
 	"opendev.org/airship/airshipctl/testutil/fs"
 )
@@ -76,21 +77,12 @@ func TestNewExecutor(t *testing.T) {
 
 	testCases := []struct {
 		name        string
-		settings    *config.Config
+		helper      ifc.Helper
 		expectedErr error
 	}{
 		{
-			name:     "New Clusterctl Executor",
-			settings: sampleSettings(),
-		},
-		{
-			name: "New Clusterctl Executor",
-			settings: func() *config.Config {
-				s := sampleSettings()
-				s.CurrentContext = "non-existent-ctx"
-				return s
-			}(),
-			expectedErr: config.ErrMissingConfig{What: "Context with name 'non-existent-ctx'"},
+			name:   "New Clusterctl Executor",
+			helper: makeDefaultHelper(t),
 		},
 	}
 	for _, test := range testCases {
@@ -99,7 +91,7 @@ func TestNewExecutor(t *testing.T) {
 			_, actualErr := cctlclient.NewExecutor(ifc.ExecutorConfig{
 				ExecutorDocument: sampleCfgDoc,
 				ExecutorBundle:   bundle,
-				AirshipConfig:    tt.settings,
+				Helper:           tt.helper,
 			})
 			assert.Equal(t, tt.expectedErr, actualErr)
 		})
@@ -185,7 +177,7 @@ func TestExecutorRun(t *testing.T) {
 				ifc.ExecutorConfig{
 					ExecutorDocument: tt.cfgDoc,
 					ExecutorBundle:   bundle,
-					AirshipConfig:    sampleSettings(),
+					Helper:           makeDefaultHelper(t),
 					KubeConfig:       kubeCfg,
 				})
 			require.NoError(t, err)
@@ -208,7 +200,7 @@ func TestExecutorValidate(t *testing.T) {
 		ifc.ExecutorConfig{
 			ExecutorDocument: sampleCfgDoc,
 			ExecutorBundle:   bundle,
-			AirshipConfig:    sampleSettings(),
+			Helper:           makeDefaultHelper(t),
 		})
 	require.NoError(t, err)
 	expectedErr := airerrors.ErrNotImplemented{}
@@ -225,7 +217,7 @@ func TestExecutorRender(t *testing.T) {
 		ifc.ExecutorConfig{
 			ExecutorDocument: sampleCfgDoc,
 			ExecutorBundle:   bundle,
-			AirshipConfig:    sampleSettings(),
+			Helper:           makeDefaultHelper(t),
 		})
 	require.NoError(t, err)
 	actualOut := &bytes.Buffer{}
@@ -233,11 +225,16 @@ func TestExecutorRender(t *testing.T) {
 	assert.Equal(t, expectedErr, actualErr)
 }
 
-func sampleSettings() *config.Config {
+func makeDefaultHelper(t *testing.T) ifc.Helper {
+	t.Helper()
 	cfg := config.NewConfig()
 	cfg.Manifests[config.AirshipDefaultManifest].TargetPath = "./testdata"
+	cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = "metadata.yaml"
 	cfg.SetLoadedConfigPath(".")
-	return cfg
+	helper, err := phase.NewHelper(cfg)
+	require.NoError(t, err)
+	require.NotNil(t, helper)
+	return helper
 }
 
 func executorDoc(t *testing.T, action string) document.Document {
diff --git a/pkg/clusterctl/client/testdata/metadata.yaml b/pkg/clusterctl/client/testdata/metadata.yaml
new file mode 100644
index 000000000..d88db8f8c
--- /dev/null
+++ b/pkg/clusterctl/client/testdata/metadata.yaml
@@ -0,0 +1,2 @@
+phase:
+  path: executor_move
\ No newline at end of file
diff --git a/pkg/k8s/applier/executor.go b/pkg/k8s/applier/executor.go
index 5eee38058..c248826d6 100644
--- a/pkg/k8s/applier/executor.go
+++ b/pkg/k8s/applier/executor.go
@@ -22,7 +22,6 @@ import (
 	"sigs.k8s.io/cli-utils/pkg/common"
 
 	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/errors"
 	"opendev.org/airship/airshipctl/pkg/events"
@@ -39,7 +38,7 @@ type ExecutorOptions struct {
 	ExecutorDocument document.Document
 	ExecutorBundle   document.Bundle
 	Kubeconfig       kubeconfig.Interface
-	AirshipConfig    *config.Config
+	Helper           ifc.Helper
 }
 
 var _ ifc.Executor = &Executor{}
@@ -59,7 +58,7 @@ func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory)
 func registerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 	return NewExecutor(ExecutorOptions{
 		BundleName:       cfg.PhaseName,
-		AirshipConfig:    cfg.AirshipConfig,
+		Helper:           cfg.Helper,
 		ExecutorBundle:   cfg.ExecutorBundle,
 		ExecutorDocument: cfg.ExecutorDocument,
 		Kubeconfig:       cfg.KubeConfig,
diff --git a/pkg/k8s/applier/executor_test.go b/pkg/k8s/applier/executor_test.go
index 3b71f1fce..e1872a35e 100644
--- a/pkg/k8s/applier/executor_test.go
+++ b/pkg/k8s/applier/executor_test.go
@@ -27,6 +27,7 @@ import (
 	"opendev.org/airship/airshipctl/pkg/k8s/applier"
 	"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
 	"opendev.org/airship/airshipctl/pkg/k8s/utils"
+	"opendev.org/airship/airshipctl/pkg/phase"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
 	"opendev.org/airship/airshipctl/testutil/fs"
 )
@@ -84,15 +85,15 @@ func TestNewExecutor(t *testing.T) {
 		name        string
 		cfgDoc      string
 		expectedErr string
-		airConfig   *config.Config
+		helper      ifc.Helper
 		kubeconf    kubeconfig.Interface
 		bundleFunc  func(t *testing.T) document.Bundle
 	}{
 		{
-			name:      "valid executor",
-			cfgDoc:    ValidExecutorDoc,
-			kubeconf:  testKubeconfig(testValidKubeconfig),
-			airConfig: makeDefaultConfig(),
+			name:     "valid executor",
+			cfgDoc:   ValidExecutorDoc,
+			kubeconf: testKubeconfig(testValidKubeconfig),
+			helper:   makeDefaultHelper(t),
 			bundleFunc: func(t *testing.T) document.Bundle {
 				return newBundle("testdata/source_bundle", t)
 			},
@@ -107,7 +108,7 @@ metadata:
   labels:
     cli-utils.sigs.k8s.io/inventory-id: "some id"`,
 			expectedErr: "wrong config document",
-			airConfig:   makeDefaultConfig(),
+			helper:      makeDefaultHelper(t),
 			bundleFunc: func(t *testing.T) document.Bundle {
 				return newBundle("testdata/source_bundle", t)
 			},
@@ -126,7 +127,7 @@ metadata:
 					ExecutorDocument: doc,
 					ExecutorBundle:   tt.bundleFunc(t),
 					Kubeconfig:       tt.kubeconf,
-					AirshipConfig:    tt.airConfig,
+					Helper:           tt.helper,
 				})
 			if tt.expectedErr != "" {
 				require.Error(t, err)
@@ -151,12 +152,12 @@ func TestExecutorRun(t *testing.T) {
 		kubeconf   kubeconfig.Interface
 		execDoc    document.Document
 		bundleFunc func(t *testing.T) document.Bundle
-		airConfig  *config.Config
+		helper     ifc.Helper
 	}{
 		{
 			name:        "cant read kubeconfig error",
 			containsErr: "no such file or directory",
-			airConfig:   makeDefaultConfig(),
+			helper:      makeDefaultHelper(t),
 			bundleFunc: func(t *testing.T) document.Bundle {
 				return newBundle("testdata/source_bundle", t)
 			},
@@ -168,7 +169,7 @@ func TestExecutorRun(t *testing.T) {
 			execDoc:     toKubernetesApply(t, ValidExecutorDoc),
 			containsErr: "Cannot apply nil bundle",
 			kubeconf:    testKubeconfig(testValidKubeconfig),
-			airConfig:   makeDefaultConfig(),
+			helper:      makeDefaultHelper(t),
 			bundleFunc: func(t *testing.T) document.Bundle {
 				return nil
 			},
@@ -180,7 +181,7 @@ func TestExecutorRun(t *testing.T) {
 			exec, err := applier.NewExecutor(
 				applier.ExecutorOptions{
 					ExecutorDocument: tt.execDoc,
-					AirshipConfig:    tt.airConfig,
+					Helper:           tt.helper,
 					ExecutorBundle:   tt.bundleFunc(t),
 					Kubeconfig:       tt.kubeconf,
 				})
@@ -219,7 +220,8 @@ func TestRender(t *testing.T) {
 	assert.Contains(t, result, "ReplicationController")
 }
 
-func makeDefaultConfig() *config.Config {
+func makeDefaultHelper(t *testing.T) ifc.Helper {
+	t.Helper()
 	conf := &config.Config{
 		CurrentContext: "default",
 		Contexts: map[string]*config.Context{
@@ -229,12 +231,15 @@ func makeDefaultConfig() *config.Config {
 		},
 		Manifests: map[string]*config.Manifest{
 			"default-manifest": {
-				MetadataPath: "metadata-path",
+				MetadataPath: "metadata.yaml",
 				TargetPath:   "testdata",
 			},
 		},
 	}
-	return conf
+	helper, err := phase.NewHelper(conf)
+	require.NoError(t, err)
+	require.NotNil(t, helper)
+	return helper
 }
 
 // toKubernetesApply converts string to document object
diff --git a/pkg/k8s/applier/testdata/metadata.yaml b/pkg/k8s/applier/testdata/metadata.yaml
new file mode 100644
index 000000000..61322cc94
--- /dev/null
+++ b/pkg/k8s/applier/testdata/metadata.yaml
@@ -0,0 +1,2 @@
+phase:
+  path: "source_bundle"
\ No newline at end of file