From 1b960c129d9a0a2fdeffdb859bde3241a69b68b8 Mon Sep 17 00:00:00 2001 From: Vladimir Kozhukalov Date: Fri, 2 Apr 2021 13:51:56 +0300 Subject: [PATCH] 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 | 21 +++++----- pkg/phase/executors/baremetal_manager.go | 6 +-- pkg/phase/executors/baremetal_manager_test.go | 40 ++++++++++++++----- pkg/phase/executors/clusterctl.go | 2 +- pkg/phase/executors/clusterctl_test.go | 9 +---- pkg/phase/executors/common_test.go | 19 --------- pkg/phase/executors/container.go | 19 ++++----- pkg/phase/executors/container_test.go | 38 +++++++++++++----- pkg/phase/executors/k8s_applier.go | 2 - pkg/phase/executors/k8s_applier_test.go | 9 ----- pkg/phase/helper.go | 4 +- pkg/phase/helper_test.go | 3 +- pkg/phase/ifc/executor.go | 18 +++++---- pkg/phase/ifc/helper.go | 2 +- testutil/phase/helper.go | 6 +-- 15 files changed, 96 insertions(+), 102 deletions(-) diff --git a/pkg/phase/client.go b/pkg/phase/client.go index ad4e7b64d..1221ccd20 100644 --- a/pkg/phase/client.go +++ b/pkg/phase/client.go @@ -104,13 +104,16 @@ func (p *phase) Executor() (ifc.Executor, error) { return executorFactory( ifc.ExecutorConfig{ - ClusterMap: cMap, - BundleFactory: bundleFactory, - PhaseName: p.apiObj.Name, - KubeConfig: kubeconf, - ExecutorDocument: executorDoc, - ClusterName: p.apiObj.ClusterName, - Helper: p.helper, + ClusterMap: cMap, + BundleFactory: bundleFactory, + PhaseName: p.apiObj.Name, + KubeConfig: kubeconf, + ExecutorDocument: executorDoc, + ClusterName: p.apiObj.ClusterName, + 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, } } - - phaseEntryPointBasePath := p.helper.PhaseEntryPointBasePath() - return filepath.Join(phaseEntryPointBasePath, relativePath), nil + return filepath.Join(p.helper.PhaseEntryPointBasePath(), relativePath), nil } // Details returns description of the phase diff --git a/pkg/phase/executors/baremetal_manager.go b/pkg/phase/executors/baremetal_manager.go index 821ceaa1f..f7c624818 100644 --- a/pkg/phase/executors/baremetal_manager.go +++ b/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 } diff --git a/pkg/phase/executors/baremetal_manager_test.go b/pkg/phase/executors/baremetal_manager_test.go index 8b59fdcb2..7a947d6db 100644 --- a/pkg/phase/executors/baremetal_manager_test.go +++ b/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), - Helper: makeDefaultHelper(t, "../testdata/", defaultMetadataPath), + Inventory: tt.inventory, }) 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) diff --git a/pkg/phase/executors/clusterctl.go b/pkg/phase/executors/clusterctl.go index 5029d45d9..88cb0c6f8 100755 --- a/pkg/phase/executors/clusterctl.go +++ b/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 } diff --git a/pkg/phase/executors/clusterctl_test.go b/pkg/phase/executors/clusterctl_test.go index d3e278741..692a55eea 100755 --- a/pkg/phase/executors/clusterctl_test.go +++ b/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", - helper: makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath), + name: "New Clusterctl Executor", }, } 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{} diff --git a/pkg/phase/executors/common_test.go b/pkg/phase/executors/common_test.go index f196377ef..d52c2172c 100755 --- a/pkg/phase/executors/common_test.go +++ b/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)) diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index 619c52e7b..3e8782121 100644 --- a/pkg/phase/executors/container.go +++ b/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() - if err != nil { - return nil, err - } - context, err := clusterMap.ClusterKubeconfigContext(c.Options.ClusterName) + context, err := c.Options.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 } diff --git a/pkg/phase/executors/container_test.go b/pkg/phase/executors/container_test.go index 754a0d9a9..6eea913bb 100644 --- a/pkg/phase/executors/container_test.go +++ b/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) diff --git a/pkg/phase/executors/k8s_applier.go b/pkg/phase/executors/k8s_applier.go index a846e5bce..25eac8161 100644 --- a/pkg/phase/executors/k8s_applier.go +++ b/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, diff --git a/pkg/phase/executors/k8s_applier_test.go b/pkg/phase/executors/k8s_applier_test.go index 155f693f1..0ef9ecd52 100644 --- a/pkg/phase/executors/k8s_applier_test.go +++ b/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, diff --git a/pkg/phase/helper.go b/pkg/phase/helper.go index d36725075..6b938bf2d 100644 --- a/pkg/phase/helper.go +++ b/pkg/phase/helper.go @@ -308,8 +308,8 @@ func (helper *Helper) WorkDir() string { } // Inventory return inventory interface -func (helper *Helper) Inventory() (inventoryifc.Inventory, error) { - return helper.inventory, nil +func (helper *Helper) Inventory() inventoryifc.Inventory { + return helper.inventory } // PhaseConfigBundle returns bundle based on phaseBundleRoot diff --git a/pkg/phase/helper_test.go b/pkg/phase/helper_test.go index d1428eaa0..fae6971c4 100644 --- a/pkg/phase/helper_test.go +++ b/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() - assert.NoError(t, err) + inv := helper.Inventory() assert.NotNil(t, inv) } diff --git a/pkg/phase/ifc/executor.go b/pkg/phase/ifc/executor.go index bfba8a6c6..0288d9edb 100644 --- a/pkg/phase/ifc/executor.go +++ b/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 - ClusterName string + PhaseName string + ClusterName string + SinkBasePath string + TargetPath string - ClusterMap clustermap.ClusterMap - ExecutorDocument document.Document - Helper Helper - KubeConfig kubeconfig.Interface - BundleFactory document.BundleFactoryFunc + ClusterMap clustermap.ClusterMap + ExecutorDocument document.Document + KubeConfig kubeconfig.Interface + BundleFactory document.BundleFactoryFunc + PhaseConfigBundle document.Bundle + Inventory inventoryifc.Inventory } diff --git a/pkg/phase/ifc/helper.go b/pkg/phase/ifc/helper.go index 4f66b7e5b..7c81dec40 100644 --- a/pkg/phase/ifc/helper.go +++ b/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 } diff --git a/testutil/phase/helper.go b/testutil/phase/helper.go index 8d35782e2..148fee2cb 100644 --- a/testutil/phase/helper.go +++ b/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