Move container phase executor to a separate package

Having an executor within container package creates
potential import cycling. This patch moves it to a separate
package which can be used to conveniently store all the executors
at one place.

Change-Id: Ibd07c2ba46d1971604bdbd9e5e360759bb68a659
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>
Relates-To: #432

pkg/phase/client.go

@@ -21,7 +21,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
-	"opendev.org/airship/airshipctl/pkg/container"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/events"
 	"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
@@ -47,7 +46,7 @@ func DefaultExecutorRegistry() map[schema.GroupVersionKind]ifc.ExecutorFactory {
 	if err := executors.RegisterIsogenExecutor(execMap); err != nil {
 		log.Fatal(ErrExecutorRegistration{ExecutorName: "isogen", Err: err})
 	}
-	if err := container.RegisterExecutor(execMap); err != nil {
+	if err := executors.RegisterContainerExecutor(execMap); err != nil {
 		log.Fatal(ErrExecutorRegistration{ExecutorName: "generic-container", Err: err})
 	}
 	return execMap

pkg/phase/executors/container.go

@@ -12,7 +12,7 @@
  limitations under the License.
 */
 
-package container
+package executors
 
 import (
 	"bytes"
@@ -40,10 +40,10 @@ const (
 	yamlSeparator = "---\n"
 )
 
-var _ ifc.Executor = &Executor{}
+var _ ifc.Executor = &ContainerExecutor{}
 
-// Executor contains resources for generic container executor
+// ContainerExecutor contains resources for generic container executor
-type Executor struct {
+type ContainerExecutor struct {
 	ExecutorBundle   document.Bundle
 	ExecutorDocument document.Document
 
@@ -52,19 +52,19 @@ type Executor struct {
 	targetPath string
 }
 
-// RegisterExecutor adds executor to phase executor registry
+// RegisterContainerExecutor adds executor to phase executor registry
-func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error {
+func RegisterContainerExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error {
 	obj := v1alpha1.DefaultGenericContainer()
 	gvks, _, err := v1alpha1.Scheme.ObjectKinds(obj)
 	if err != nil {
 		return err
 	}
-	registry[gvks[0]] = NewExecutor
+	registry[gvks[0]] = NewContainerExecutor
 	return nil
 }
 
-// NewExecutor creates instance of phase executor
+// NewContainerExecutor creates instance of phase executor
-func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
+func NewContainerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 	bundle, err := cfg.BundleFactory()
 	if err != nil {
 		return nil, err
@@ -78,7 +78,7 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 		return nil, err
 	}
 
-	return &Executor{
+	return &ContainerExecutor{
 		ExecutorBundle:   bundle,
 		ExecutorDocument: cfg.ExecutorDocument,
 
@@ -91,7 +91,7 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
 }
 
 // Run generic container as a phase runner
-func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
+func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 	defer close(evtCh)
 
 	evtCh <- events.NewEvent().WithGenericContainerEvent(events.GenericContainerEvent{
@@ -129,7 +129,7 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 }
 
 // SetInput sets input for function
-func (c *Executor) SetInput(evtCh chan events.Event) {
+func (c *ContainerExecutor) SetInput(evtCh chan events.Event) {
 	docs, err := c.ExecutorBundle.GetAllDocuments()
 	if err != nil {
 		handleError(evtCh, err)
@@ -150,7 +150,7 @@ func (c *Executor) SetInput(evtCh chan events.Event) {
 }
 
 // PrepareFunctions prepares data for function
-func (c *Executor) PrepareFunctions(evtCh chan events.Event) {
+func (c *ContainerExecutor) PrepareFunctions(evtCh chan events.Event) {
 	rnode, err := kyaml.Parse(c.ContConf.Config)
 	if err != nil {
 		handleError(evtCh, err)
@@ -174,7 +174,7 @@ func (c *Executor) PrepareFunctions(evtCh chan events.Event) {
 }
 
 // SetMounts allows to set relative path for storage mounts to prevent security issues
-func (c *Executor) SetMounts() {
+func (c *ContainerExecutor) SetMounts() {
 	if len(c.ContConf.Spec.Container.StorageMounts) == 0 {
 		return
 	}
@@ -186,17 +186,11 @@ func (c *Executor) SetMounts() {
 }
 
 // Validate executor configuration and documents
-func (c *Executor) Validate() error {
+func (c *ContainerExecutor) Validate() error {
 	return errors.ErrNotImplemented{}
 }
 
 // Render executor documents
-func (c *Executor) Render(_ io.Writer, _ ifc.RenderOptions) error {
+func (c *ContainerExecutor) Render(_ io.Writer, _ ifc.RenderOptions) error {
 	return errors.ErrNotImplemented{}
 }
-
-func handleError(ch chan<- events.Event, err error) {
-	ch <- events.NewEvent().WithErrorEvent(events.ErrorEvent{
-		Error: err,
-	})
-}

pkg/phase/executors/container_test.go

@@ -12,7 +12,7 @@
  limitations under the License.
 */
 
-package container_test
+package executors_test
 
 import (
 	"bytes"
@@ -27,15 +27,15 @@ import (
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
 	"opendev.org/airship/airshipctl/pkg/config"
-	"opendev.org/airship/airshipctl/pkg/container"
 	"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 (
-	executorDoc = `
+	containerExecutorDoc = `
 apiVersion: airshipit.org/v1alpha1
 kind: GenericContainer
 metadata:
@@ -86,7 +86,7 @@ metadata:
     config.kubernetes.io/function: "container:\n  image: quay.io/test/image:v0.0.1\nexec:
       {}\nstarlark: {}\n"
 `
-	singleExecutorBundlePath = "testdata/single"
+	singleExecutorBundlePath = "../../container/testdata/single"
 	firstDocInput            = `---
 apiVersion: v1
 kind: Secret
@@ -97,7 +97,7 @@ stringData:
     #!/bin/sh
     echo WORKS! $var >&2
 type: Opaque`
-	manyExecutorBundlePath = "testdata/many"
+	manyExecutorBundlePath = "../../container/testdata/many"
 	secondDocInput         = `---
 apiVersion: v1
 kind: Secret
@@ -110,27 +110,27 @@ type: Opaque
 	yamlSeparator = "---\n"
 )
 
-func TestRegisterExecutor(t *testing.T) {
+func TestRegisterContainerExecutor(t *testing.T) {
 	registry := make(map[schema.GroupVersionKind]ifc.ExecutorFactory)
 	expectedGVK := schema.GroupVersionKind{
 		Group:   "airshipit.org",
 		Version: "v1alpha1",
 		Kind:    "GenericContainer",
 	}
-	err := container.RegisterExecutor(registry)
+	err := executors.RegisterContainerExecutor(registry)
 	require.NoError(t, err)
 
 	_, found := registry[expectedGVK]
 	assert.True(t, found)
 }
 
-func TestNewExecutor(t *testing.T) {
+func TestNewContainerExecutor(t *testing.T) {
-	execDoc, err := document.NewDocumentFromBytes([]byte(executorDoc))
+	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
 	require.NoError(t, err)
-	_, err = container.NewExecutor(ifc.ExecutorConfig{
+	_, err = executors.NewContainerExecutor(ifc.ExecutorConfig{
 		ExecutorDocument: execDoc,
 		BundleFactory:    testBundleFactory(singleExecutorBundlePath),
-		Helper:           makeDefaultHelper(t),
+		Helper:           makeDefaultContainerHelper(t),
 	})
 	require.NoError(t, err)
 }
@@ -138,9 +138,9 @@ func TestNewExecutor(t *testing.T) {
 func TestSetInputSingleDocument(t *testing.T) {
 	bundle, err := document.NewBundleByPath(singleExecutorBundlePath)
 	require.NoError(t, err)
-	execDoc, err := document.NewDocumentFromBytes([]byte(executorDoc))
+	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
 	require.NoError(t, err)
-	e := &container.Executor{
+	e := &executors.ContainerExecutor{
 		ExecutorBundle:   bundle,
 		ExecutorDocument: execDoc,
 
@@ -169,9 +169,9 @@ func TestSetInputSingleDocument(t *testing.T) {
 func TestSetInputManyDocuments(t *testing.T) {
 	bundle, err := document.NewBundleByPath(manyExecutorBundlePath)
 	require.NoError(t, err)
-	execDoc, err := document.NewDocumentFromBytes([]byte(executorDoc))
+	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
 	require.NoError(t, err)
-	e := &container.Executor{
+	e := &executors.ContainerExecutor{
 		ExecutorBundle:   bundle,
 		ExecutorDocument: execDoc,
 
@@ -207,14 +207,14 @@ func TestSetInputManyDocuments(t *testing.T) {
 func TestPrepareFunctions(t *testing.T) {
 	bundle, err := document.NewBundleByPath(singleExecutorBundlePath)
 	require.NoError(t, err)
-	execDoc, err := document.NewDocumentFromBytes([]byte(executorDoc))
+	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
 	require.NoError(t, err)
 	contConf := &v1alpha1.GenericContainer{
 		Spec: runtimeutil.FunctionSpec{},
 	}
 	err = execDoc.ToAPIObject(contConf, v1alpha1.Scheme)
 	require.NoError(t, err)
-	e := &container.Executor{
+	e := &executors.ContainerExecutor{
 		ExecutorBundle:   bundle,
 		ExecutorDocument: execDoc,
 
@@ -233,16 +233,10 @@ func TestPrepareFunctions(t *testing.T) {
 	assert.Equal(t, transformedFunction, strFuncs)
 }
 
-func testBundleFactory(path string) document.BundleFactoryFunc {
+func makeDefaultContainerHelper(t *testing.T) ifc.Helper {
-	return func() (document.Bundle, error) {
-		return document.NewBundleByPath(path)
-	}
-}
-
-func makeDefaultHelper(t *testing.T) ifc.Helper {
 	t.Helper()
 	cfg := config.NewConfig()
-	cfg.Manifests[config.AirshipDefaultManifest].TargetPath = "./testdata"
+	cfg.Manifests[config.AirshipDefaultManifest].TargetPath = "../../container/testdata"
 	cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = "metadata.yaml"
 	cfg.Manifests[config.AirshipDefaultManifest].Repositories[config.DefaultTestPhaseRepo].URLString = ""
 	cfg.SetLoadedConfigPath(".")