Do not pass phase helper to executor initializers

Phase helper provides plenty of useful methods for
 a phase client. But these methods are not used by
 phase executor initializers except for getting some
 configuration values. So, it is better to provide
 initializers with necessary values instead of passing
 them the phase helper.

Change-Id: I8c596455e30444570a86efad73d792af0ca83a33
Relates-To: #464
Relates-To: #465

pkg/phase/client.go

@@ -104,13 +104,16 @@ func (p *phase) Executor() (ifc.Executor, error) {
 
 	return executorFactory(
 		ifc.ExecutorConfig{
-			ClusterMap:       cMap,
+			ClusterMap:        cMap,
-			BundleFactory:    bundleFactory,
+			BundleFactory:     bundleFactory,
-			PhaseName:        p.apiObj.Name,
+			PhaseName:         p.apiObj.Name,
-			KubeConfig:       kubeconf,
+			KubeConfig:        kubeconf,
-			ExecutorDocument: executorDoc,
+			ExecutorDocument:  executorDoc,
-			ClusterName:      p.apiObj.ClusterName,
+			ClusterName:       p.apiObj.ClusterName,
-			Helper:           p.helper,
+			PhaseConfigBundle: p.helper.PhaseConfigBundle(),
+			SinkBasePath:      p.helper.PhaseEntryPointBasePath(),
+			TargetPath:        p.helper.TargetPath(),
+			Inventory:         p.helper.Inventory(),
 		})
 }
 
@@ -196,9 +199,7 @@ func (p *phase) DocumentRoot() (string, error) {
 			PhaseNamespace: p.apiObj.Namespace,
 		}
 	}
-
+	return filepath.Join(p.helper.PhaseEntryPointBasePath(), relativePath), nil
-	phaseEntryPointBasePath := p.helper.PhaseEntryPointBasePath()
-	return filepath.Join(phaseEntryPointBasePath, relativePath), nil
 }
 
 // Details returns description of the phase

pkg/phase/executors/baremetal_manager.go

@@ -37,16 +37,12 @@ type BaremetalManagerExecutor struct {
 
 // NewBaremetalExecutor constructor for baremetal executor
 func NewBaremetalExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
-	inv, err := cfg.Helper.Inventory()
-	if err != nil {
-		return nil, err
-	}
 	options := airshipv1.DefaultBaremetalManager()
 	if err := cfg.ExecutorDocument.ToAPIObject(options, airshipv1.Scheme); err != nil {
 		return nil, err
 	}
 	return &BaremetalManagerExecutor{
-		inventory: inv,
+		inventory: cfg.Inventory,
 		options:   options,
 	}, nil
 }

pkg/phase/executors/baremetal_manager_test.go

@@ -16,18 +16,22 @@ package executors_test
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/events"
+	inventoryifc "opendev.org/airship/airshipctl/pkg/inventory/ifc"
 	"opendev.org/airship/airshipctl/pkg/k8s/utils"
 	"opendev.org/airship/airshipctl/pkg/phase/executors"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
 	testdoc "opendev.org/airship/airshipctl/testutil/document"
+	testinventory "opendev.org/airship/airshipctl/testutil/inventory"
 )
 
 var bmhExecutorTemplate = `apiVersion: airshipit.org/v1alpha1
@@ -44,13 +48,27 @@ spec:
     remoteDirect:
       isoURL: %s`
 
+func testBaremetalInventory() inventoryifc.Inventory {
+	bmhi := &testinventory.MockBMHInventory{}
+	bmhi.On("SelectOne", mock.Anything).Return()
+	bi := &testinventory.MockInventory{}
+	bi.On("BaremetalInventory").Return(bmhi, nil)
+	return bi
+}
+
+func testBaremetalInventoryNoKustomization() inventoryifc.Inventory {
+	bi := &testinventory.MockInventory{}
+	bi.On("BaremetalInventory").
+		Return(nil, errors.New("there is no kustomization.yaml"))
+	return bi
+}
+
 func TestNewBMHExecutor(t *testing.T) {
 	t.Run("success", func(t *testing.T) {
 		execDoc := executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "reboot", "/home/iso-url"))
 		executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{
 			ExecutorDocument: execDoc,
 			BundleFactory:    testBundleFactory(singleExecutorBundlePath),
-			Helper:           makeDefaultHelper(t, "../testdata", defaultMetadataPath),
 		})
 		assert.NoError(t, err)
 		assert.NotNil(t, executor)
@@ -64,7 +82,6 @@ func TestNewBMHExecutor(t *testing.T) {
 		executor, actualErr := executors.NewBaremetalExecutor(ifc.ExecutorConfig{
 			ExecutorDocument: execDoc,
 			BundleFactory:    testBundleFactory(singleExecutorBundlePath),
-			Helper:           makeDefaultHelper(t, "../testdata", defaultMetadataPath),
 		})
 		assert.Equal(t, exepectedErr, actualErr)
 		assert.Nil(t, executor)
@@ -77,6 +94,7 @@ func TestBMHExecutorRun(t *testing.T) {
 		expectedErr string
 		runOptions  ifc.RunOptions
 		execDoc     document.Document
+		inventory   inventoryifc.Inventory
 	}{
 		{
 			name:        "error validate dry-run",
@@ -86,32 +104,37 @@ func TestBMHExecutorRun(t *testing.T) {
 				// any value but zero
 				Timeout: 40,
 			},
-			execDoc: executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "unknown", "")),
+			execDoc:   executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "unknown", "")),
+			inventory: testBaremetalInventory(),
 		},
 		{
 			name: "success validate dry-run",
 			runOptions: ifc.RunOptions{
 				DryRun: true,
 			},
-			execDoc: executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "remote-direct", "/some/url")),
+			execDoc:   executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "remote-direct", "/some/url")),
+			inventory: testBaremetalInventory(),
 		},
 		{
 			name:        "error unknown action type",
 			runOptions:  ifc.RunOptions{},
 			execDoc:     executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "unknown", "")),
 			expectedErr: "unknown action type",
+			inventory:   testBaremetalInventory(),
 		},
 		{
 			name:        "error no kustomization.yaml for inventory remote-direct",
 			runOptions:  ifc.RunOptions{},
 			execDoc:     executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "remote-direct", "")),
 			expectedErr: "kustomization.yaml",
+			inventory:   testBaremetalInventoryNoKustomization(),
 		},
 		{
-			name:        "error no kustomization.yaml for inventory remote-direct",
+			name:        "error no kustomization.yaml for inventory reboot",
 			runOptions:  ifc.RunOptions{},
 			execDoc:     executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "reboot", "")),
 			expectedErr: "kustomization.yaml",
+			inventory:   testBaremetalInventoryNoKustomization(),
 		},
 	}
 
@@ -120,8 +143,7 @@ func TestBMHExecutorRun(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{
 				ExecutorDocument: tt.execDoc,
-				BundleFactory:    testBundleFactory(singleExecutorBundlePath),
+				Inventory:        tt.inventory,
-				Helper:           makeDefaultHelper(t, "../testdata/", defaultMetadataPath),
 			})
 			require.NoError(t, err)
 			require.NotNil(t, executor)
@@ -179,8 +201,6 @@ func TestBMHValidate(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{
 				ExecutorDocument: tt.execDoc,
-				BundleFactory:    testBundleFactory(singleExecutorBundlePath),
-				Helper:           makeDefaultHelper(t, "../testdata/", defaultMetadataPath),
 			})
 			require.NoError(t, err)
 			require.NotNil(t, executor)
@@ -201,8 +221,6 @@ func TestBMHManagerRender(t *testing.T) {
 	execDoc := executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "reboot", "/home/iso-url"))
 	executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{
 		ExecutorDocument: execDoc,
-		BundleFactory:    testBundleFactory(singleExecutorBundlePath),
-		Helper:           makeDefaultHelper(t, "../testdata", defaultMetadataPath),
 	})
 	require.NoError(t, err)
 	require.NotNil(t, executor)

pkg/phase/executors/clusterctl.go

@@ -51,7 +51,7 @@ func NewClusterctlExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 	if err := cfg.ExecutorDocument.ToAPIObject(options, airshipv1.Scheme); err != nil {
 		return nil, err
 	}
-	client, err := client.NewClient(cfg.Helper.TargetPath(), log.DebugEnabled(), options)
+	client, err := client.NewClient(cfg.TargetPath, log.DebugEnabled(), options)
 	if err != nil {
 		return nil, err
 	}

pkg/phase/executors/clusterctl_test.go

@@ -106,12 +106,10 @@ func TestNewClusterctlExecutor(t *testing.T) {
 	sampleCfgDoc := executorDoc(t, fmt.Sprintf(executorConfigTmplGood, "init"))
 	testCases := []struct {
 		name        string
-		helper      ifc.Helper
 		expectedErr error
 	}{
 		{
-			name:   "New Clusterctl Executor",
+			name: "New Clusterctl Executor",
-			helper: makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath),
 		},
 	}
 	for _, test := range testCases {
@@ -119,7 +117,6 @@ func TestNewClusterctlExecutor(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			_, actualErr := executors.NewClusterctlExecutor(ifc.ExecutorConfig{
 				ExecutorDocument: sampleCfgDoc,
-				Helper:           tt.helper,
 			})
 			assert.Equal(t, tt.expectedErr, actualErr)
 		})
@@ -199,7 +196,6 @@ func TestClusterctlExecutorRun(t *testing.T) {
 			executor, err := executors.NewClusterctlExecutor(
 				ifc.ExecutorConfig{
 					ExecutorDocument: tt.cfgDoc,
-					Helper:           makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath),
 					KubeConfig:       kubeCfg,
 					ClusterMap:       tt.clusterMap,
 				})
@@ -275,7 +271,6 @@ func TestClusterctlExecutorValidate(t *testing.T) {
 			executor, err := executors.NewClusterctlExecutor(
 				ifc.ExecutorConfig{
 					ExecutorDocument: sampleCfgDoc,
-					Helper:           makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath),
 				})
 			require.NoError(t, err)
 			err = executor.Validate()
@@ -293,8 +288,8 @@ func TestClusterctlExecutorRender(t *testing.T) {
 	sampleCfgDoc := executorDoc(t, fmt.Sprintf(executorConfigTmpl, "init"))
 	executor, err := executors.NewClusterctlExecutor(
 		ifc.ExecutorConfig{
+			TargetPath:       "../../clusterctl/client/testdata",
 			ExecutorDocument: sampleCfgDoc,
-			Helper:           makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath),
 		})
 	require.NoError(t, err)
 	actualOut := &bytes.Buffer{}

pkg/phase/executors/common_test.go

@@ -21,18 +21,12 @@ import (
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
-	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/events"
-	"opendev.org/airship/airshipctl/pkg/phase"
 	"opendev.org/airship/airshipctl/pkg/phase/executors"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
 )
 
-const (
-	defaultMetadataPath = "metadata.yaml"
-)
-
 func TestRegisterExecutor(t *testing.T) {
 	testCases := []struct {
 		name         string
@@ -94,19 +88,6 @@ func TestRegisterExecutor(t *testing.T) {
 	}
 }
 
-func makeDefaultHelper(t *testing.T, targetPath, metaPath string) ifc.Helper {
-	t.Helper()
-	cfg := config.NewConfig()
-	cfg.Manifests[config.AirshipDefaultManifest].TargetPath = targetPath
-	cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = metaPath
-	cfg.Manifests[config.AirshipDefaultManifest].Repositories[config.DefaultTestPhaseRepo].URLString = ""
-	cfg.SetLoadedConfigPath(".")
-	helper, err := phase.NewHelper(cfg)
-	require.NoError(t, err)
-	require.NotNil(t, helper)
-	return helper
-}
-
 // executorDoc converts string to document object
 func executorDoc(t *testing.T, s string) document.Document {
 	doc, err := document.NewDocumentFromBytes([]byte(s))

pkg/phase/executors/container.go

@@ -36,13 +36,13 @@ var _ ifc.Executor = &ContainerExecutor{}
 
 // ContainerExecutor contains resources for generic container executor
 type ContainerExecutor struct {
-	ResultsDir string
+	ResultsDir    string
+	MountBasePath string
 
 	Container        *v1alpha1.GenericContainer
 	ClientFunc       container.ClientV1Alpha1FactoryFunc
 	ExecutorBundle   document.Bundle
 	ExecutorDocument document.Document
-	Helper           ifc.Helper
 	Options          ifc.ExecutorConfig
 }
 
@@ -66,16 +66,16 @@ func NewContainerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 
 	var resultsDir string
 	if apiObj.Spec.SinkOutputDir != "" {
-		resultsDir = filepath.Join(cfg.Helper.PhaseEntryPointBasePath(), apiObj.Spec.SinkOutputDir)
+		resultsDir = filepath.Join(cfg.SinkBasePath, apiObj.Spec.SinkOutputDir)
 	}
 
 	return &ContainerExecutor{
 		ResultsDir:       resultsDir,
+		MountBasePath:    cfg.TargetPath,
 		ExecutorBundle:   bundle,
 		ExecutorDocument: cfg.ExecutorDocument,
 		// TODO extend tests with proper client, make it interface
 		ClientFunc: container.NewClientV1Alpha1,
-		Helper:     cfg.Helper,
 		Container:  apiObj,
 		Options:    cfg,
 	}, nil
@@ -124,7 +124,7 @@ func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 		return
 	}
 
-	err = c.ClientFunc(c.ResultsDir, input, output, c.Container, c.Helper.TargetPath()).Run()
+	err = c.ClientFunc(c.ResultsDir, input, output, c.Container, c.MountBasePath).Run()
 	if err != nil {
 		handleError(evtCh, err)
 		return
@@ -138,11 +138,7 @@ func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 
 // SetKubeConfig adds env variable and mounts kubeconfig to container
 func (c *ContainerExecutor) SetKubeConfig() (kubeconfig.Cleanup, error) {
-	clusterMap, err := c.Helper.ClusterMap()
+	context, err := c.Options.ClusterMap.ClusterKubeconfigContext(c.Options.ClusterName)
-	if err != nil {
-		return nil, err
-	}
-	context, err := clusterMap.ClusterKubeconfigContext(c.Options.ClusterName)
 	if err != nil {
 		return nil, err
 	}
@@ -180,13 +176,12 @@ func (c *ContainerExecutor) Render(_ io.Writer, _ ifc.RenderOptions) error {
 func (c *ContainerExecutor) setConfig() error {
 	if c.Container.ConfigRef != nil {
 		log.Debugf("Config reference is specified, looking for the object in config ref: '%v'", c.Container.ConfigRef)
-		log.Debugf("using bundle root %s", c.Helper.PhaseBundleRoot())
 		gvk := c.Container.ConfigRef.GroupVersionKind()
 		selector := document.NewSelector().
 			ByName(c.Container.ConfigRef.Name).
 			ByNamespace(c.Container.ConfigRef.Namespace).
 			ByGvk(gvk.Group, gvk.Version, gvk.Kind)
-		doc, err := c.Helper.PhaseConfigBundle().SelectOne(selector)
+		doc, err := c.Options.PhaseConfigBundle.SelectOne(selector)
 		if err != nil {
 			return err
 		}

pkg/phase/executors/container_test.go

@@ -22,6 +22,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
+	"opendev.org/airship/airshipctl/pkg/cluster/clustermap"
 	"opendev.org/airship/airshipctl/pkg/container"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/events"
@@ -61,9 +62,28 @@ const (
       cmd: encrypt
       unencrypted-regex: '^(kind|apiVersion|group|metadata)$'`
 
+	singleExecutorClusterMap = `apiVersion: airshipit.org/v1alpha1
+kind: ClusterMap
+metadata:
+  labels:
+    airshipit.org/deploy-k8s: "false"
+  name: main-map
+map:
+  testCluster: {}
+`
 	singleExecutorBundlePath = "../../container/testdata/single"
 )
 
+func testClusterMap(t *testing.T) clustermap.ClusterMap {
+	doc, err := document.NewDocumentFromBytes([]byte(singleExecutorClusterMap))
+	require.NoError(t, err)
+	require.NotNil(t, doc)
+	apiObj := v1alpha1.DefaultClusterMap()
+	err = doc.ToAPIObject(apiObj, v1alpha1.Scheme)
+	require.NoError(t, err)
+	return clustermap.NewClusterMap(apiObj)
+}
+
 func TestNewContainerExecutor(t *testing.T) {
 	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
 	require.NoError(t, err)
@@ -72,7 +92,6 @@ func TestNewContainerExecutor(t *testing.T) {
 		e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{
 			ExecutorDocument: execDoc,
 			BundleFactory:    testBundleFactory(singleExecutorBundlePath),
-			Helper:           makeDefaultHelper(t, "../../container/testdata", "metadata.yaml"),
 		})
 		assert.NoError(t, err)
 		assert.NotNil(t, e)
@@ -84,7 +103,6 @@ func TestNewContainerExecutor(t *testing.T) {
 			BundleFactory: func() (document.Bundle, error) {
 				return nil, fmt.Errorf("bundle error")
 			},
-			Helper: makeDefaultHelper(t, "../../container/testdata", "metadata.yaml"),
 		})
 		assert.Error(t, err)
 		assert.Nil(t, e)
@@ -96,7 +114,6 @@ func TestNewContainerExecutor(t *testing.T) {
 			BundleFactory: func() (document.Bundle, error) {
 				return nil, errors.ErrDocumentEntrypointNotDefined{}
 			},
-			Helper: makeDefaultHelper(t, "../../container/testdata", "metadata.yaml"),
 		})
 		assert.NoError(t, err)
 		assert.NotNil(t, e)
@@ -108,7 +125,6 @@ func TestGenericContainer(t *testing.T) {
 		name         string
 		outputPath   string
 		expectedErr  string
-		targetPath   string
 		resultConfig string
 
 		containerAPI   *v1alpha1.GenericContainer
@@ -125,7 +141,6 @@ func TestGenericContainer(t *testing.T) {
 				},
 			},
 			clientFunc: container.NewClientV1Alpha1,
-			targetPath: singleExecutorBundlePath,
 		},
 		{
 			name: "error kyaml cant parse config",
@@ -138,7 +153,6 @@ func TestGenericContainer(t *testing.T) {
 			runOptions:  ifc.RunOptions{},
 			expectedErr: "wrong Node Kind",
 			clientFunc:  container.NewClientV1Alpha1,
-			targetPath:  singleExecutorBundlePath,
 		},
 		{
 			name: "error no object referenced in config",
@@ -150,13 +164,11 @@ func TestGenericContainer(t *testing.T) {
 			},
 			runOptions:  ifc.RunOptions{DryRun: true},
 			expectedErr: "found no documents",
-			targetPath:  singleExecutorBundlePath,
 		},
 		{
 			name:         "success dry run",
 			containerAPI: &v1alpha1.GenericContainer{},
 			runOptions:   ifc.RunOptions{DryRun: true},
-			targetPath:   singleExecutorBundlePath,
 		},
 		{
 			name: "success referenced config present",
@@ -168,7 +180,6 @@ func TestGenericContainer(t *testing.T) {
 				},
 			},
 			runOptions: ifc.RunOptions{DryRun: true},
-			targetPath: singleExecutorBundlePath,
 			resultConfig: `apiVersion: v1
 kind: Secret
 metadata:
@@ -187,12 +198,14 @@ type: Opaque
 		t.Run(tt.name, func(t *testing.T) {
 			b, err := document.NewBundleByPath(singleExecutorBundlePath)
 			require.NoError(t, err)
+			phaseConfigBundle, err := document.NewBundleByPath(singleExecutorBundlePath)
+			require.NoError(t, err)
+
 			container := executors.ContainerExecutor{
 				ResultsDir:     tt.outputPath,
 				ExecutorBundle: b,
 				Container:      tt.containerAPI,
 				ClientFunc:     tt.clientFunc,
-				Helper:         makeDefaultHelper(t, tt.targetPath, "../metadata.yaml"),
 				Options: ifc.ExecutorConfig{
 					ClusterName: "testCluster",
 					KubeConfig: fakeKubeConfig{
@@ -200,6 +213,8 @@ type: Opaque
 							return "testPath", func() {}, nil
 						},
 					},
+					ClusterMap:        testClusterMap(t),
+					PhaseConfigBundle: phaseConfigBundle,
 				},
 			}
 
@@ -243,6 +258,7 @@ func TestSetKubeConfig(t *testing.T) {
 						return "testPath", func() {}, nil
 					},
 				},
+				ClusterMap: testClusterMap(t),
 			},
 		},
 		{
@@ -254,6 +270,7 @@ func TestSetKubeConfig(t *testing.T) {
 						return "", func() {}, getFileErr
 					},
 				},
+				ClusterMap: testClusterMap(t),
 			},
 			expectedErr: getFileErr,
 		},
@@ -265,7 +282,6 @@ func TestSetKubeConfig(t *testing.T) {
 			e := executors.ContainerExecutor{
 				Options:   tt.opts,
 				Container: &v1alpha1.GenericContainer{},
-				Helper:    makeDefaultHelper(t, singleExecutorBundlePath, "../metadata.yaml"),
 			}
 			_, err := e.SetKubeConfig()
 			assert.Equal(t, tt.expectedErr, err)

pkg/phase/executors/k8s_applier.go

@@ -43,7 +43,6 @@ type KubeApplierExecutor struct {
 	ExecutorBundle   document.Bundle
 	ExecutorDocument document.Document
 	BundleName       string
-	Helper           ifc.Helper
 
 	apiObject   *airshipv1.KubernetesApply
 	cleanup     kubeconfig.Cleanup
@@ -66,7 +65,6 @@ func NewKubeApplierExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 	return &KubeApplierExecutor{
 		ExecutorBundle:   bundle,
 		BundleName:       cfg.PhaseName,
-		Helper:           cfg.Helper,
 		ExecutorDocument: cfg.ExecutorDocument,
 		apiObject:        apiObj,
 		clusterMap:       cfg.ClusterMap,

pkg/phase/executors/k8s_applier_test.go

@@ -86,7 +86,6 @@ func TestNewKubeApplierExecutor(t *testing.T) {
 		name          string
 		cfgDoc        string
 		expectedErr   string
-		helper        ifc.Helper
 		kubeconf      kubeconfig.Interface
 		bundleFactory document.BundleFactoryFunc
 	}{
@@ -94,7 +93,6 @@ func TestNewKubeApplierExecutor(t *testing.T) {
 			name:          "valid executor",
 			cfgDoc:        ValidExecutorDoc,
 			kubeconf:      testKubeconfig(testValidKubeconfig),
-			helper:        makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath),
 			bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
 		},
 		{
@@ -107,7 +105,6 @@ metadata:
   labels:
     cli-utils.sigs.k8s.io/inventory-id: "some id"`,
 			expectedErr:   "wrong config document",
-			helper:        makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath),
 			bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
 		},
 
@@ -116,7 +113,6 @@ metadata:
 			cfgDoc:        ValidExecutorDoc,
 			expectedErr:   "no such file or directory",
 			kubeconf:      testKubeconfig(testValidKubeconfig),
-			helper:        makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath),
 			bundleFactory: testBundleFactory("does not exist"),
 		},
 	}
@@ -133,7 +129,6 @@ metadata:
 					ExecutorDocument: doc,
 					BundleFactory:    tt.bundleFactory,
 					KubeConfig:       tt.kubeconf,
-					Helper:           tt.helper,
 				})
 			if tt.expectedErr != "" {
 				require.Error(t, err)
@@ -159,13 +154,11 @@ func TestKubeApplierExecutorRun(t *testing.T) {
 		kubeconf      kubeconfig.Interface
 		execDoc       document.Document
 		bundleFactory document.BundleFactoryFunc
-		helper        ifc.Helper
 		clusterMap    clustermap.ClusterMap
 	}{
 		{
 			name:          "cant read kubeconfig error",
 			containsErr:   "no such file or directory",
-			helper:        makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath),
 			bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
 			kubeconf:      testKubeconfig(`invalid kubeconfig`),
 			execDoc:       executorDoc(t, ValidExecutorDocNamespaced),
@@ -179,7 +172,6 @@ func TestKubeApplierExecutorRun(t *testing.T) {
 		{
 			name:          "error cluster not defined",
 			containsErr:   "is not defined in cluster map",
-			helper:        makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath),
 			bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
 			kubeconf:      testKubeconfig(testValidKubeconfig),
 			execDoc:       executorDoc(t, ValidExecutorDocNamespaced),
@@ -192,7 +184,6 @@ func TestKubeApplierExecutorRun(t *testing.T) {
 			exec, err := executors.NewKubeApplierExecutor(
 				ifc.ExecutorConfig{
 					ExecutorDocument: tt.execDoc,
-					Helper:           tt.helper,
 					BundleFactory:    tt.bundleFactory,
 					KubeConfig:       tt.kubeconf,
 					ClusterMap:       tt.clusterMap,

pkg/phase/helper.go

@@ -308,8 +308,8 @@ func (helper *Helper) WorkDir() string {
 }
 
 // Inventory return inventory interface
-func (helper *Helper) Inventory() (inventoryifc.Inventory, error) {
+func (helper *Helper) Inventory() inventoryifc.Inventory {
-	return helper.inventory, nil
+	return helper.inventory
 }
 
 // PhaseConfigBundle returns bundle based on phaseBundleRoot

pkg/phase/helper_test.go

@@ -577,8 +577,7 @@ func TestHelperInventory(t *testing.T) {
 	helper, err := phase.NewHelper(testConfig(t))
 	require.NoError(t, err)
 	require.NotNil(t, helper)
-	inv, err := helper.Inventory()
+	inv := helper.Inventory()
-	assert.NoError(t, err)
 	assert.NotNil(t, inv)
 }
 

pkg/phase/ifc/executor.go

@@ -21,6 +21,7 @@ import (
 	"opendev.org/airship/airshipctl/pkg/cluster/clustermap"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/events"
+	inventoryifc "opendev.org/airship/airshipctl/pkg/inventory/ifc"
 	"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
 )
 
@@ -58,12 +59,15 @@ type ExecutorFactory func(config ExecutorConfig) (Executor, error)
 
 // ExecutorConfig container to store all executor options
 type ExecutorConfig struct {
-	PhaseName   string
+	PhaseName    string
-	ClusterName string
+	ClusterName  string
+	SinkBasePath string
+	TargetPath   string
 
-	ClusterMap       clustermap.ClusterMap
+	ClusterMap        clustermap.ClusterMap
-	ExecutorDocument document.Document
+	ExecutorDocument  document.Document
-	Helper           Helper
+	KubeConfig        kubeconfig.Interface
-	KubeConfig       kubeconfig.Interface
+	BundleFactory     document.BundleFactoryFunc
-	BundleFactory    document.BundleFactoryFunc
+	PhaseConfigBundle document.Bundle
+	Inventory         inventoryifc.Inventory
 }

pkg/phase/ifc/helper.go

@@ -35,7 +35,7 @@ type Helper interface {
 	ClusterMap() (clustermap.ClusterMap, error)
 	ExecutorDoc(phaseID ID) (document.Document, error)
 	PhaseBundleRoot() string
-	Inventory() (ifc.Inventory, error)
+	Inventory() ifc.Inventory
 	PhaseEntryPointBasePath() string
 	PhaseConfigBundle() document.Bundle
 }

testutil/phase/helper.go

@@ -132,13 +132,13 @@ func (mh *MockHelper) PhaseBundleRoot() string {
 }
 
 // Inventory mock
-func (mh *MockHelper) Inventory() (inventoryifc.Inventory, error) {
+func (mh *MockHelper) Inventory() inventoryifc.Inventory {
 	args := mh.Called()
 	val, ok := args.Get(0).(inventoryifc.Inventory)
 	if !ok {
-		return nil, args.Error(1)
+		return nil
 	}
-	return val, args.Error(1)
+	return val
 }
 
 // PhaseEntryPointBasePath mock