From a6b89c0c94cb51fafefa917e7d3dbb2dd27893dc Mon Sep 17 00:00:00 2001 From: Vladimir Kozhukalov Date: Wed, 31 Mar 2021 18:17:04 +0300 Subject: [PATCH] Use bundle mock for container executor tests Also: - Remove unnecessary bundle from baremetal executor tests. - Use inline output for container api tests and remove unnecessary test bundle files. - Fixed merge conflict. Change-Id: I400fb18fb1595c34598b451be5ad3f12012bae63 Relates-To: #464 Relates-To: #465 --- pkg/container/api_test.go | 34 ++++- pkg/container/testdata/kustomization.yaml | 0 .../testdata/many/kustomization.yaml | 2 - pkg/container/testdata/many/secret.yaml | 18 --- pkg/container/testdata/metadata.yaml | 0 .../testdata/single/cluster-map.yaml | 9 -- .../testdata/single/kustomization.yaml | 3 - pkg/container/testdata/single/secret.yaml | 9 -- pkg/phase/executors/baremetal_manager_test.go | 2 - pkg/phase/executors/common_test.go | 20 ++- pkg/phase/executors/container_test.go | 121 +++++++++++------- 11 files changed, 118 insertions(+), 100 deletions(-) delete mode 100755 pkg/container/testdata/kustomization.yaml delete mode 100644 pkg/container/testdata/many/kustomization.yaml delete mode 100644 pkg/container/testdata/many/secret.yaml delete mode 100644 pkg/container/testdata/metadata.yaml delete mode 100644 pkg/container/testdata/single/cluster-map.yaml delete mode 100644 pkg/container/testdata/single/kustomization.yaml delete mode 100644 pkg/container/testdata/single/secret.yaml diff --git a/pkg/container/api_test.go b/pkg/container/api_test.go index 616bdc9ef..7d904a5d6 100644 --- a/pkg/container/api_test.go +++ b/pkg/container/api_test.go @@ -30,17 +30,39 @@ import ( "opendev.org/airship/airshipctl/pkg/api/v1alpha1" aircontainer "opendev.org/airship/airshipctl/pkg/container" - "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/phase/ifc" "opendev.org/airship/airshipctl/pkg/util" ) -func bundlePathToInput(t *testing.T, bundlePath string) io.Reader { +const ( + singleSecretBundleOutput = `--- +apiVersion: airshipit.org/v1alpha1 +kind: ClusterMap +metadata: + labels: + airshipit.org/deploy-k8s: "false" + name: main-map +map: + testCluster: {} +... +--- +apiVersion: v1 +kind: Secret +metadata: + name: test-script +type: Opaque +stringData: + script.sh: | + #!/bin/sh + echo WORKS! $var >&2 +... +` +) + +func testInput(t *testing.T) io.Reader { t.Helper() - bundle, err := document.NewBundleByPath(bundlePath) - require.NoError(t, err) buf := bytes.NewBuffer([]byte{}) - err = bundle.Write(buf) + _, err := buf.Write([]byte(singleSecretBundleOutput)) require.NoError(t, err) return buf } @@ -266,7 +288,7 @@ func TestGenericContainer(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - input := bundlePathToInput(t, "testdata/single") + input := testInput(t) client := aircontainer.NewV1Alpha1(tt.outputPath, input, tt.output, tt.containerAPI, "", tt.execFunc) err := client.Run() diff --git a/pkg/container/testdata/kustomization.yaml b/pkg/container/testdata/kustomization.yaml deleted file mode 100755 index e69de29bb..000000000 diff --git a/pkg/container/testdata/many/kustomization.yaml b/pkg/container/testdata/many/kustomization.yaml deleted file mode 100644 index 97a9721bd..000000000 --- a/pkg/container/testdata/many/kustomization.yaml +++ /dev/null @@ -1,2 +0,0 @@ -resources: - - secret.yaml diff --git a/pkg/container/testdata/many/secret.yaml b/pkg/container/testdata/many/secret.yaml deleted file mode 100644 index 2003d3afe..000000000 --- a/pkg/container/testdata/many/secret.yaml +++ /dev/null @@ -1,18 +0,0 @@ ---- -apiVersion: v1 -kind: Secret -metadata: - name: test-script -stringData: - script.sh: | - #!/bin/sh - echo WORKS! $var >&2 -type: Opaque ---- -apiVersion: v1 -kind: Secret -metadata: - labels: - airshipit.org/ephemeral-node: "true" - name: master-0-bmc-secret -type: Opaque diff --git a/pkg/container/testdata/metadata.yaml b/pkg/container/testdata/metadata.yaml deleted file mode 100644 index e69de29bb..000000000 diff --git a/pkg/container/testdata/single/cluster-map.yaml b/pkg/container/testdata/single/cluster-map.yaml deleted file mode 100644 index c146ecf9d..000000000 --- a/pkg/container/testdata/single/cluster-map.yaml +++ /dev/null @@ -1,9 +0,0 @@ ---- -apiVersion: airshipit.org/v1alpha1 -kind: ClusterMap -metadata: - labels: - airshipit.org/deploy-k8s: "false" - name: main-map -map: - testCluster: {} diff --git a/pkg/container/testdata/single/kustomization.yaml b/pkg/container/testdata/single/kustomization.yaml deleted file mode 100644 index dcd793cc7..000000000 --- a/pkg/container/testdata/single/kustomization.yaml +++ /dev/null @@ -1,3 +0,0 @@ -resources: - - secret.yaml - - cluster-map.yaml diff --git a/pkg/container/testdata/single/secret.yaml b/pkg/container/testdata/single/secret.yaml deleted file mode 100644 index d43f4332d..000000000 --- a/pkg/container/testdata/single/secret.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: v1 -kind: Secret -metadata: - name: test-script -type: Opaque -stringData: - script.sh: | - #!/bin/sh - echo WORKS! $var >&2 diff --git a/pkg/phase/executors/baremetal_manager_test.go b/pkg/phase/executors/baremetal_manager_test.go index aff7783fa..6c67318fc 100644 --- a/pkg/phase/executors/baremetal_manager_test.go +++ b/pkg/phase/executors/baremetal_manager_test.go @@ -69,7 +69,6 @@ func TestNewBMHExecutor(t *testing.T) { execDoc := executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "reboot", "/home/iso-url")) executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, - BundleFactory: testBundleFactory(), }) assert.NoError(t, err) assert.NotNil(t, executor) @@ -82,7 +81,6 @@ func TestNewBMHExecutor(t *testing.T) { } executor, actualErr := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, - BundleFactory: testBundleFactory(), }) assert.Equal(t, exepectedErr, actualErr) assert.Nil(t, executor) diff --git a/pkg/phase/executors/common_test.go b/pkg/phase/executors/common_test.go index 876672a14..ca7bfbc3d 100644 --- a/pkg/phase/executors/common_test.go +++ b/pkg/phase/executors/common_test.go @@ -17,6 +17,9 @@ package executors_test import ( "testing" + "opendev.org/airship/airshipctl/pkg/api/v1alpha1" + "opendev.org/airship/airshipctl/pkg/cluster/clustermap" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/runtime/schema" @@ -104,15 +107,18 @@ func executorBundle(t *testing.T, s string) document.Bundle { return b } -// TODO replace this test bundle factory with one that uses bundle mock -func testBundleFactory() document.BundleFactoryFunc { - return func() (document.Bundle, error) { - return document.NewBundleByPath(singleExecutorBundlePath) - } -} - func wrapError(err error) events.Event { return events.NewEvent().WithErrorEvent(events.ErrorEvent{ Error: err, }) } + +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) +} diff --git a/pkg/phase/executors/container_test.go b/pkg/phase/executors/container_test.go index 978f08490..ae2a39e3d 100644 --- a/pkg/phase/executors/container_test.go +++ b/pkg/phase/executors/container_test.go @@ -14,16 +14,17 @@ package executors_test import ( "bytes" + goerrors "errors" "fmt" "io" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" 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" @@ -31,6 +32,7 @@ import ( "opendev.org/airship/airshipctl/pkg/phase/errors" "opendev.org/airship/airshipctl/pkg/phase/executors" "opendev.org/airship/airshipctl/pkg/phase/ifc" + testdoc "opendev.org/airship/airshipctl/testutil/document" ) const ( @@ -72,27 +74,71 @@ metadata: map: testCluster: {} ` - singleExecutorBundlePath = "../../container/testdata/single" + + refConfig = `apiVersion: v1 +kind: Secret +metadata: + name: test-script +stringData: + script.sh: | + #!/bin/sh + echo WORKS! $var >&2 +type: Opaque +` ) -func testClusterMap(t *testing.T) clustermap.ClusterMap { - doc, err := document.NewDocumentFromBytes([]byte(singleExecutorClusterMap)) +func testContainerBundleFactory() document.BundleFactoryFunc { + return func() (document.Bundle, error) { + bundle := &testdoc.MockBundle{} + return bundle, nil + } +} + +func testContainerBundleFactoryNoDocumentEntryPoint() document.BundleFactoryFunc { + return func() (document.Bundle, error) { + return nil, errors.ErrDocumentEntrypointNotDefined{} + } +} + +func testContainerBundle(t *testing.T, content string) document.Bundle { + bundle := &testdoc.MockBundle{} + writer := &bytes.Buffer{} + bundle.On("Write", writer). + Return(nil). + Run(func(args mock.Arguments) { + arg, ok := args.Get(0).(*bytes.Buffer) + if ok { + _, err := arg.Write([]byte(content)) + require.NoError(t, err) + } + }) + return bundle +} + +func testContainerPhaseConfigBundleNoDocs() document.Bundle { + bundle := &testdoc.MockBundle{} + bundle.On("SelectOne", mock.Anything). + Return(nil, goerrors.New("found no documents")) + return bundle +} + +func testContainerPhaseConfigBundleRefConfig(t *testing.T) document.Bundle { + refConfigDoc, err := document.NewDocumentFromBytes([]byte(refConfig)) 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) + bundle := &testdoc.MockBundle{} + bundle.On("SelectOne", mock.Anything). + Return(refConfigDoc, nil) + return bundle } func TestNewContainerExecutor(t *testing.T) { - execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc)) - require.NoError(t, err) + execDoc, errDoc := document.NewDocumentFromBytes([]byte(containerExecutorDoc)) + require.NoError(t, errDoc) t.Run("success new container executor", func(t *testing.T) { e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, - BundleFactory: testBundleFactory(), + BundleFactory: testContainerBundleFactory(), }) assert.NoError(t, err) assert.NotNil(t, e) @@ -101,9 +147,7 @@ func TestNewContainerExecutor(t *testing.T) { t.Run("error bundle factory", func(t *testing.T) { e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, - BundleFactory: func() (document.Bundle, error) { - return nil, fmt.Errorf("bundle error") - }, + BundleFactory: testdoc.ErrorBundleFactory, }) assert.Error(t, err) assert.Nil(t, e) @@ -112,9 +156,7 @@ func TestNewContainerExecutor(t *testing.T) { t.Run("bundle factory - empty documentEntryPoint", func(t *testing.T) { e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, - BundleFactory: func() (document.Bundle, error) { - return nil, errors.ErrDocumentEntrypointNotDefined{} - }, + BundleFactory: testContainerBundleFactoryNoDocumentEntryPoint(), }) assert.NoError(t, err) assert.NotNil(t, e) @@ -128,10 +170,11 @@ func TestGenericContainer(t *testing.T) { expectedErr string resultConfig string - containerAPI *v1alpha1.GenericContainer - executorConfig ifc.ExecutorConfig - runOptions ifc.RunOptions - clientFunc container.ClientV1Alpha1FactoryFunc + containerAPI *v1alpha1.GenericContainer + executorConfig ifc.ExecutorConfig + runOptions ifc.RunOptions + clientFunc container.ClientV1Alpha1FactoryFunc + phaseConfigBundle document.Bundle }{ { name: "error unknown container type", @@ -163,8 +206,9 @@ func TestGenericContainer(t *testing.T) { Name: "no such name", }, }, - runOptions: ifc.RunOptions{DryRun: true}, - expectedErr: "found no documents", + runOptions: ifc.RunOptions{DryRun: true}, + expectedErr: "found no documents", + phaseConfigBundle: testContainerPhaseConfigBundleNoDocs(), }, { name: "success dry run", @@ -180,31 +224,20 @@ func TestGenericContainer(t *testing.T) { APIVersion: "v1", }, }, - runOptions: ifc.RunOptions{DryRun: true}, - resultConfig: `apiVersion: v1 -kind: Secret -metadata: - name: test-script -stringData: - script.sh: | - #!/bin/sh - echo WORKS! $var >&2 -type: Opaque -`, + runOptions: ifc.RunOptions{DryRun: true}, + resultConfig: refConfig, + phaseConfigBundle: testContainerPhaseConfigBundleRefConfig(t), }, } for _, tt := range tests { tt := tt 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) + executorBundle := testContainerBundle(t, "") - container := executors.ContainerExecutor{ + containerExecutor := executors.ContainerExecutor{ ResultsDir: tt.outputPath, - ExecutorBundle: b, + ExecutorBundle: executorBundle, Container: tt.containerAPI, ClientFunc: tt.clientFunc, Options: ifc.ExecutorConfig{ @@ -215,12 +248,12 @@ type: Opaque }, }, ClusterMap: testClusterMap(t), - PhaseConfigBundle: phaseConfigBundle, + PhaseConfigBundle: tt.phaseConfigBundle, }, } ch := make(chan events.Event) - go container.Run(ch, tt.runOptions) + go containerExecutor.Run(ch, tt.runOptions) actualEvt := make([]events.Event, 0) for evt := range ch { @@ -237,7 +270,7 @@ type: Opaque assert.NoError(t, e.ErrorEvent.Error) assert.Equal(t, e.Type, events.GenericContainerType) assert.Equal(t, e.GenericContainerEvent.Operation, events.GenericContainerStop) - assert.Equal(t, tt.resultConfig, container.Container.Config) + assert.Equal(t, tt.resultConfig, containerExecutor.Container.Config) } }) }