From 5a6f1096ebde0ef651175a88429522004fa77a82 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Thu, 10 Dec 2020 16:32:06 -0600 Subject: [PATCH] Groom phase/executors package All the duplicated methods were removed. Unit tests reorganized. Test coverage increased. Change-Id: I0f7bd3eea63c101195ea50c0369e62945d73f297 Signed-off-by: Ruslan Aliev Closes: #432 --- pkg/bootstrap/isogen/command.go | 1 + pkg/phase/client.go | 19 +--- pkg/phase/executors/clusterctl.go | 41 ++------ pkg/phase/executors/clusterctl_test.go | 71 +++---------- pkg/phase/executors/common.go | 71 +++++++++++++ pkg/phase/executors/common_test.go | 134 ++++++++++++++++++++++++ pkg/phase/executors/container.go | 13 --- pkg/phase/executors/container_test.go | 32 +----- pkg/phase/executors/ephemeral.go | 30 +----- pkg/phase/executors/ephemeral_test.go | 17 --- pkg/phase/executors/errors.go | 10 ++ pkg/phase/executors/isogen.go | 19 ---- pkg/phase/executors/isogen_test.go | 21 ---- pkg/phase/executors/k8s_applier.go | 85 +++++---------- pkg/phase/executors/k8s_applier_test.go | 62 +++-------- pkg/phase/ifc/executor.go | 1 + 16 files changed, 293 insertions(+), 334 deletions(-) create mode 100755 pkg/phase/executors/common.go create mode 100755 pkg/phase/executors/common_test.go diff --git a/pkg/bootstrap/isogen/command.go b/pkg/bootstrap/isogen/command.go index 537b578e2..7de20c2b3 100644 --- a/pkg/bootstrap/isogen/command.go +++ b/pkg/bootstrap/isogen/command.go @@ -49,6 +49,7 @@ type BootstrapIsoOptions struct { Writer io.Writer } +// VerifyInputs verifies image configuration func VerifyInputs(cfg *v1alpha1.IsoConfiguration) error { if cfg.IsoContainer.Volume == "" { return config.ErrMissingConfig{ diff --git a/pkg/phase/client.go b/pkg/phase/client.go index 5a5866705..3c24668d2 100644 --- a/pkg/phase/client.go +++ b/pkg/phase/client.go @@ -37,20 +37,11 @@ type ExecutorRegistry func() map[schema.GroupVersionKind]ifc.ExecutorFactory func DefaultExecutorRegistry() map[schema.GroupVersionKind]ifc.ExecutorFactory { execMap := make(map[schema.GroupVersionKind]ifc.ExecutorFactory) - if err := executors.RegisterExecutor(execMap); err != nil { - log.Fatal(ErrExecutorRegistration{ExecutorName: "clusterctl", Err: err}) - } - if err := executors.RegisterKubeApplierExecutor(execMap); err != nil { - log.Fatal(ErrExecutorRegistration{ExecutorName: "kubernetes-apply", Err: err}) - } - if err := executors.RegisterIsogenExecutor(execMap); err != nil { - log.Fatal(ErrExecutorRegistration{ExecutorName: "isogen", Err: err}) - } - if err := executors.RegisterContainerExecutor(execMap); err != nil { - log.Fatal(ErrExecutorRegistration{ExecutorName: "generic-container", Err: err}) - } - if err := executors.RegisterEphemeralExecutor(execMap); err != nil { - log.Fatal(ErrExecutorRegistration{ExecutorName: "ephemeral", Err: err}) + for _, execName := range []string{executors.Clusterctl, executors.KubernetesApply, + executors.Isogen, executors.GenericContainer, executors.Ephemeral} { + if err := executors.RegisterExecutor(execName, execMap); err != nil { + log.Fatal(ErrExecutorRegistration{ExecutorName: execName, Err: err}) + } } return execMap } diff --git a/pkg/phase/executors/clusterctl.go b/pkg/phase/executors/clusterctl.go index d0d520d53..9a6120155 100755 --- a/pkg/phase/executors/clusterctl.go +++ b/pkg/phase/executors/clusterctl.go @@ -17,8 +17,6 @@ package executors import ( "io" - "k8s.io/apimachinery/pkg/runtime/schema" - airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/cluster/clustermap" "opendev.org/airship/airshipctl/pkg/clusterctl/client" @@ -41,19 +39,8 @@ type ClusterctlExecutor struct { kubecfg kubeconfig.Interface } -// RegisterExecutor adds executor to phase executor registry -func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error { - obj := &airshipv1.Clusterctl{} - gvks, _, err := airshipv1.Scheme.ObjectKinds(obj) - if err != nil { - return err - } - registry[gvks[0]] = NewExecutor - return nil -} - -// NewExecutor creates instance of 'clusterctl init' phase executor -func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { +// NewClusterctlExecutor creates instance of 'clusterctl init' phase executor +func NewClusterctlExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { options := airshipv1.DefaultClusterctl() if err := cfg.ExecutorDocument.ToAPIObject(options, airshipv1.Scheme); err != nil { return nil, err @@ -80,7 +67,7 @@ func (c *ClusterctlExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { case airshipv1.Init: c.init(opts, evtCh) default: - c.handleErr(ErrUnknownExecutorAction{Action: string(c.options.Action)}, evtCh) + handleError(evtCh, ErrUnknownExecutorAction{Action: string(c.options.Action)}) } } @@ -92,23 +79,23 @@ func (c *ClusterctlExecutor) move(opts ifc.RunOptions, evtCh chan events.Event) ns := c.options.MoveOptions.Namespace kubeConfigFile, cleanup, err := c.kubecfg.GetFile() if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } defer cleanup() fromCluster, err := c.clusterMap.ParentCluster(c.clusterName) if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } fromContext, err := c.clusterMap.ClusterKubeconfigContext(fromCluster) if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } toContext, err := c.clusterMap.ClusterKubeconfigContext(c.clusterName) if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } @@ -117,7 +104,7 @@ func (c *ClusterctlExecutor) move(opts ifc.RunOptions, evtCh chan events.Event) if !opts.DryRun { err = c.Move(kubeConfigFile, fromContext, kubeConfigFile, toContext, ns) if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } } @@ -135,7 +122,7 @@ func (c *ClusterctlExecutor) init(opts ifc.RunOptions, evtCh chan events.Event) }) kubeConfigFile, cleanup, err := c.kubecfg.GetFile() if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } @@ -153,14 +140,14 @@ func (c *ClusterctlExecutor) init(opts ifc.RunOptions, evtCh chan events.Event) context, err := c.clusterMap.ClusterKubeconfigContext(c.clusterName) if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } // Use cluster name as context in kubeconfig file err = c.Init(kubeConfigFile, context) if err != nil { - c.handleErr(err, evtCh) + handleError(evtCh, err) return } evtCh <- events.NewEvent().WithClusterctlEvent(events.ClusterctlEvent{ @@ -169,12 +156,6 @@ func (c *ClusterctlExecutor) init(opts ifc.RunOptions, evtCh chan events.Event) }) } -func (c *ClusterctlExecutor) handleErr(err error, evtCh chan events.Event) { - evtCh <- events.NewEvent().WithErrorEvent(events.ErrorEvent{ - Error: err, - }) -} - // Validate executor configuration and documents func (c *ClusterctlExecutor) Validate() error { return errors.ErrNotImplemented{} diff --git a/pkg/phase/executors/clusterctl_test.go b/pkg/phase/executors/clusterctl_test.go index 84a2d0037..a75bdd942 100755 --- a/pkg/phase/executors/clusterctl_test.go +++ b/pkg/phase/executors/clusterctl_test.go @@ -23,17 +23,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime/schema" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/cluster/clustermap" - "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" airerrors "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/fs" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" - "opendev.org/airship/airshipctl/pkg/phase" "opendev.org/airship/airshipctl/pkg/phase/executors" "opendev.org/airship/airshipctl/pkg/phase/ifc" testfs "opendev.org/airship/airshipctl/testutil/fs" @@ -59,22 +56,8 @@ providers: v0.3.3: manifests/function/capi/v0.3.3` ) -func TestRegisterExecutor(t *testing.T) { - registry := make(map[schema.GroupVersionKind]ifc.ExecutorFactory) - expectedGVK := schema.GroupVersionKind{ - Group: "airshipit.org", - Version: "v1alpha1", - Kind: "Clusterctl", - } - err := executors.RegisterExecutor(registry) - require.NoError(t, err) - - _, found := registry[expectedGVK] - assert.True(t, found) -} - func TestNewExecutor(t *testing.T) { - sampleCfgDoc := executorDoc(t, "init") + sampleCfgDoc := executorDoc(t, fmt.Sprintf(executorConfigTmpl, "init")) testCases := []struct { name string helper ifc.Helper @@ -82,13 +65,13 @@ func TestNewExecutor(t *testing.T) { }{ { name: "New Clusterctl Executor", - helper: makeDefaultHelper(t), + helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), }, } for _, test := range testCases { tt := test t.Run(tt.name, func(t *testing.T) { - _, actualErr := executors.NewExecutor(ifc.ExecutorConfig{ + _, actualErr := executors.NewClusterctlExecutor(ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, Helper: tt.helper, }) @@ -110,7 +93,7 @@ func TestExecutorRun(t *testing.T) { }{ { name: "Error unknown action", - cfgDoc: executorDoc(t, "someAction"), + cfgDoc: executorDoc(t, fmt.Sprintf(executorConfigTmpl, "someAction")), bundlePath: "testdata/executor_init", expectedEvt: []events.Event{ wrapError(executors.ErrUnknownExecutorAction{Action: "someAction"}), @@ -119,7 +102,7 @@ func TestExecutorRun(t *testing.T) { }, { name: "Error temporary file", - cfgDoc: executorDoc(t, "init"), + cfgDoc: executorDoc(t, fmt.Sprintf(executorConfigTmpl, "init")), fs: testfs.MockFileSystem{ MockTempFile: func(string, string) (fs.File, error) { return nil, errTmpFile @@ -136,7 +119,7 @@ func TestExecutorRun(t *testing.T) { }, { name: "Regular Run init", - cfgDoc: executorDoc(t, "init"), + cfgDoc: executorDoc(t, fmt.Sprintf(executorConfigTmpl, "init")), fs: testfs.MockFileSystem{ MockTempFile: func(string, string) (fs.File, error) { return testfs.TestFile{ @@ -167,10 +150,10 @@ func TestExecutorRun(t *testing.T) { kubeconfig.FromByte([]byte("someKubeConfig")), kubeconfig.InjectFileSystem(tt.fs), ) - executor, err := executors.NewExecutor( + executor, err := executors.NewClusterctlExecutor( ifc.ExecutorConfig{ ExecutorDocument: tt.cfgDoc, - Helper: makeDefaultHelper(t), + Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), KubeConfig: kubeCfg, ClusterMap: tt.clusterMap, }) @@ -197,11 +180,11 @@ func TestExecutorRun(t *testing.T) { } func TestExecutorValidate(t *testing.T) { - sampleCfgDoc := executorDoc(t, "init") - executor, err := executors.NewExecutor( + sampleCfgDoc := executorDoc(t, fmt.Sprintf(executorConfigTmpl, "init")) + executor, err := executors.NewClusterctlExecutor( ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - Helper: makeDefaultHelper(t), + Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), }) require.NoError(t, err) expectedErr := airerrors.ErrNotImplemented{} @@ -210,11 +193,11 @@ func TestExecutorValidate(t *testing.T) { } func TestExecutorRender(t *testing.T) { - sampleCfgDoc := executorDoc(t, "init") - executor, err := executors.NewExecutor( + sampleCfgDoc := executorDoc(t, fmt.Sprintf(executorConfigTmpl, "init")) + executor, err := executors.NewClusterctlExecutor( ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - Helper: makeDefaultHelper(t), + Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), }) require.NoError(t, err) actualOut := &bytes.Buffer{} @@ -222,29 +205,3 @@ func TestExecutorRender(t *testing.T) { assert.Len(t, actualOut.Bytes(), 0) assert.NoError(t, actualErr) } - -func makeDefaultHelper(t *testing.T) ifc.Helper { - t.Helper() - cfg := config.NewConfig() - cfg.Manifests[config.AirshipDefaultManifest].TargetPath = "../../clusterctl/client/testdata" - cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = "metadata.yaml" - 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 -} - -func executorDoc(t *testing.T, action string) document.Document { - cfg := []byte(fmt.Sprintf(executorConfigTmpl, action)) - cfgDoc, err := document.NewDocumentFromBytes(cfg) - require.NoError(t, err) - return cfgDoc -} - -func wrapError(err error) events.Event { - return events.NewEvent().WithErrorEvent(events.ErrorEvent{ - Error: err, - }) -} diff --git a/pkg/phase/executors/common.go b/pkg/phase/executors/common.go new file mode 100755 index 000000000..08d874165 --- /dev/null +++ b/pkg/phase/executors/common.go @@ -0,0 +1,71 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package executors + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + + airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" + "opendev.org/airship/airshipctl/pkg/events" + "opendev.org/airship/airshipctl/pkg/phase/ifc" +) + +// Constants related to phase executor names +const ( + Clusterctl = "clusterctl" + KubernetesApply = "kubernetes-apply" + Isogen = "isogen" + GenericContainer = "generic-container" + Ephemeral = "ephemeral" +) + +// RegisterExecutor adds executor to phase executor registry +func RegisterExecutor(executorName string, registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error { + var gvks []schema.GroupVersionKind + var execObj ifc.ExecutorFactory + var err error + + switch executorName { + case Clusterctl: + gvks, _, err = airshipv1.Scheme.ObjectKinds(&airshipv1.Clusterctl{}) + execObj = NewClusterctlExecutor + case KubernetesApply: + gvks, _, err = airshipv1.Scheme.ObjectKinds(&airshipv1.KubernetesApply{}) + execObj = NewKubeApplierExecutor + case Isogen: + gvks, _, err = airshipv1.Scheme.ObjectKinds(airshipv1.DefaultIsoConfiguration()) + execObj = NewIsogenExecutor + case GenericContainer: + gvks, _, err = airshipv1.Scheme.ObjectKinds(airshipv1.DefaultGenericContainer()) + execObj = NewContainerExecutor + case Ephemeral: + gvks, _, err = airshipv1.Scheme.ObjectKinds(airshipv1.DefaultBootConfiguration()) + execObj = NewEphemeralExecutor + default: + return ErrUnknownExecutorName{ExecutorName: executorName} + } + + if err != nil { + return err + } + registry[gvks[0]] = execObj + return nil +} + +func handleError(ch chan<- events.Event, err error) { + ch <- events.NewEvent().WithErrorEvent(events.ErrorEvent{ + Error: err, + }) +} diff --git a/pkg/phase/executors/common_test.go b/pkg/phase/executors/common_test.go new file mode 100755 index 000000000..cf7b1f1f2 --- /dev/null +++ b/pkg/phase/executors/common_test.go @@ -0,0 +1,134 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package executors_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "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" +) + +func TestRegisterExecutor(t *testing.T) { + testCases := []struct { + name string + executorName string + registry map[schema.GroupVersionKind]ifc.ExecutorFactory + expectedGVK schema.GroupVersionKind + expectedErr error + }{ + { + name: "register clusterctl executor", + executorName: executors.Clusterctl, + registry: make(map[schema.GroupVersionKind]ifc.ExecutorFactory), + expectedGVK: schema.GroupVersionKind{ + Group: "airshipit.org", + Version: "v1alpha1", + Kind: "Clusterctl", + }, + }, + { + name: "register container executor", + executorName: executors.GenericContainer, + registry: make(map[schema.GroupVersionKind]ifc.ExecutorFactory), + expectedGVK: schema.GroupVersionKind{ + Group: "airshipit.org", + Version: "v1alpha1", + Kind: "GenericContainer", + }, + }, + { + name: "register isogen executor", + executorName: executors.Isogen, + registry: make(map[schema.GroupVersionKind]ifc.ExecutorFactory), + expectedGVK: schema.GroupVersionKind{ + Group: "airshipit.org", + Version: "v1alpha1", + Kind: "IsoConfiguration", + }, + }, + { + name: "register k8s applier executor", + executorName: executors.KubernetesApply, + registry: make(map[schema.GroupVersionKind]ifc.ExecutorFactory), + expectedGVK: schema.GroupVersionKind{ + Group: "airshipit.org", + Version: "v1alpha1", + Kind: "KubernetesApply", + }, + }, + { + name: "register ephemeral executor", + executorName: executors.Ephemeral, + registry: make(map[schema.GroupVersionKind]ifc.ExecutorFactory), + expectedGVK: schema.GroupVersionKind{ + Group: "airshipit.org", + Version: "v1alpha1", + Kind: "BootConfiguration", + }, + }, + } + for _, test := range testCases { + tt := test + t.Run(tt.name, func(t *testing.T) { + err := executors.RegisterExecutor(tt.executorName, tt.registry) + require.NoError(t, err) + + _, found := tt.registry[tt.expectedGVK] + assert.True(t, found) + }) + } +} + +func makeDefaultHelper(t *testing.T, targetPath string) ifc.Helper { + t.Helper() + cfg := config.NewConfig() + cfg.Manifests[config.AirshipDefaultManifest].TargetPath = targetPath + cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = "metadata.yaml" + 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)) + require.NoError(t, err) + require.NotNil(t, doc) + return doc +} + +func testBundleFactory(path string) document.BundleFactoryFunc { + return func() (document.Bundle, error) { + return document.NewBundleByPath(path) + } +} + +func wrapError(err error) events.Event { + return events.NewEvent().WithErrorEvent(events.ErrorEvent{ + Error: err, + }) +} diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index 2f3dc49d8..d24d45822 100644 --- a/pkg/phase/executors/container.go +++ b/pkg/phase/executors/container.go @@ -26,8 +26,6 @@ import ( kyaml "sigs.k8s.io/kustomize/kyaml/yaml" "sigs.k8s.io/yaml" - "k8s.io/apimachinery/pkg/runtime/schema" - "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/errors" @@ -47,17 +45,6 @@ type ContainerExecutor struct { targetPath string } -// RegisterContainerExecutor adds executor to phase executor registry -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]] = NewContainerExecutor - return nil -} - // NewContainerExecutor creates instance of phase executor func NewContainerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { bundle, err := cfg.BundleFactory() diff --git a/pkg/phase/executors/container_test.go b/pkg/phase/executors/container_test.go index fe1dcc0b1..cc8e69d03 100644 --- a/pkg/phase/executors/container_test.go +++ b/pkg/phase/executors/container_test.go @@ -20,15 +20,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil" "sigs.k8s.io/kustomize/kyaml/runfn" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" - "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/phase" "opendev.org/airship/airshipctl/pkg/phase/executors" "opendev.org/airship/airshipctl/pkg/phase/ifc" yaml_util "opendev.org/airship/airshipctl/pkg/util/yaml" @@ -109,27 +106,13 @@ type: Opaque ` ) -func TestRegisterContainerExecutor(t *testing.T) { - registry := make(map[schema.GroupVersionKind]ifc.ExecutorFactory) - expectedGVK := schema.GroupVersionKind{ - Group: "airshipit.org", - Version: "v1alpha1", - Kind: "GenericContainer", - } - err := executors.RegisterContainerExecutor(registry) - require.NoError(t, err) - - _, found := registry[expectedGVK] - assert.True(t, found) -} - func TestNewContainerExecutor(t *testing.T) { execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc)) require.NoError(t, err) _, err = executors.NewContainerExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultContainerHelper(t), + Helper: makeDefaultHelper(t, "../../container/testdata"), }) require.NoError(t, err) } @@ -236,16 +219,3 @@ func TestPrepareFunctions(t *testing.T) { assert.Equal(t, transformedFunction, strFuncs) } - -func makeDefaultContainerHelper(t *testing.T) ifc.Helper { - t.Helper() - cfg := config.NewConfig() - cfg.Manifests[config.AirshipDefaultManifest].TargetPath = "../../container/testdata" - cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = "metadata.yaml" - 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 -} diff --git a/pkg/phase/executors/ephemeral.go b/pkg/phase/executors/ephemeral.go index 89a35edfc..17c4c9ffe 100644 --- a/pkg/phase/executors/ephemeral.go +++ b/pkg/phase/executors/ephemeral.go @@ -19,8 +19,6 @@ import ( "io" "time" - "k8s.io/apimachinery/pkg/runtime/schema" - "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/bootstrap/ephemeral" "opendev.org/airship/airshipctl/pkg/container" @@ -42,17 +40,6 @@ type EphemeralExecutor struct { Container container.Container } -// RegisterEphemeralExecutor adds executor to phase executor registry -func RegisterEphemeralExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error { - obj := v1alpha1.DefaultBootConfiguration() - gvks, _, err := v1alpha1.Scheme.ObjectKinds(obj) - if err != nil { - return err - } - registry[gvks[0]] = NewEphemeralExecutor - return nil -} - // NewEphemeralExecutor creates instance of phase executor func NewEphemeralExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { apiObj := &v1alpha1.BootConfiguration{} @@ -92,7 +79,7 @@ func (c *EphemeralExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { c.BootConf.BootstrapContainer.ContainerRuntime, c.BootConf.BootstrapContainer.Image) if err != nil { - handleEphemeralError(evtCh, err) + handleError(evtCh, err) return } c.Container = builder @@ -111,7 +98,7 @@ func (c *EphemeralExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { err := bootstrapOpts.VerifyInputs() if err != nil { - handleEphemeralError(evtCh, err) + handleError(evtCh, err) return } @@ -122,7 +109,7 @@ func (c *EphemeralExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { err = bootstrapOpts.CreateBootstrapContainer() if err != nil { - handleEphemeralError(evtCh, err) + handleError(evtCh, err) return } @@ -133,7 +120,7 @@ func (c *EphemeralExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { err = bootstrapOpts.VerifyArtifacts() if err != nil { - handleEphemeralError(evtCh, err) + handleError(evtCh, err) return } @@ -153,12 +140,3 @@ func (c *EphemeralExecutor) Render(w io.Writer, _ ifc.RenderOptions) error { log.Print("Ephemeral Executor Render() will be implemented later.") return nil } - -func handleEphemeralError(ch chan<- events.Event, err error) { - ch <- events.Event{ - Type: events.ErrorType, - ErrorEvent: events.ErrorEvent{ - Error: err, - }, - } -} diff --git a/pkg/phase/executors/ephemeral_test.go b/pkg/phase/executors/ephemeral_test.go index 6676347f1..ec6f3a1fb 100644 --- a/pkg/phase/executors/ephemeral_test.go +++ b/pkg/phase/executors/ephemeral_test.go @@ -24,8 +24,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime/schema" - "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/bootstrap/ephemeral" "opendev.org/airship/airshipctl/pkg/container" @@ -57,21 +55,6 @@ bootstrapContainer: ` ) -// TestRegisterEphemeralExecutor - Unit testing function RegisterEphemeralExecutor() -func TestRegisterEphemeralExecutor(t *testing.T) { - registry := make(map[schema.GroupVersionKind]ifc.ExecutorFactory) - expectedGVK := schema.GroupVersionKind{ - Group: "airshipit.org", - Version: "v1alpha1", - Kind: "BootConfiguration", - } - err := executors.RegisterEphemeralExecutor(registry) - require.NoError(t, err) - - _, found := registry[expectedGVK] - assert.True(t, found) -} - // TestNewEphemeralExecutor - Unit testing function NewExecutor() func TestNewEphemeralExecutor(t *testing.T) { execDoc, err := document.NewDocumentFromBytes([]byte(executorEphemeralDoc)) diff --git a/pkg/phase/executors/errors.go b/pkg/phase/executors/errors.go index 5c886d6f0..c3a5c7e4f 100755 --- a/pkg/phase/executors/errors.go +++ b/pkg/phase/executors/errors.go @@ -35,3 +35,13 @@ type ErrIsogenNilBundle struct { func (e ErrIsogenNilBundle) Error() string { return "Cannot build iso with empty bundle, no data source is available" } + +// ErrUnknownExecutorName is returned for unknown executor name parameter +// received by RegisterExecutor function +type ErrUnknownExecutorName struct { + ExecutorName string +} + +func (e ErrUnknownExecutorName) Error() string { + return fmt.Sprintf("unknown executor name '%s'", e.ExecutorName) +} diff --git a/pkg/phase/executors/isogen.go b/pkg/phase/executors/isogen.go index 925ae7a92..a50eabc9a 100644 --- a/pkg/phase/executors/isogen.go +++ b/pkg/phase/executors/isogen.go @@ -21,8 +21,6 @@ import ( "path/filepath" "strings" - "k8s.io/apimachinery/pkg/runtime/schema" - "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/bootstrap/isogen" "opendev.org/airship/airshipctl/pkg/container" @@ -44,17 +42,6 @@ type IsogenExecutor struct { Builder container.Container } -// RegisterIsogenExecutor adds executor to phase executor registry -func RegisterIsogenExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error { - obj := v1alpha1.DefaultIsoConfiguration() - gvks, _, err := v1alpha1.Scheme.ObjectKinds(obj) - if err != nil { - return err - } - registry[gvks[0]] = NewIsogenExecutor - return nil -} - // NewIsogenExecutor creates instance of phase executor func NewIsogenExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { apiObj := &v1alpha1.IsoConfiguration{ @@ -161,9 +148,3 @@ func (c *IsogenExecutor) Render(w io.Writer, _ ifc.RenderOptions) error { _, err := w.Write([]byte{}) return err } - -func handleError(ch chan<- events.Event, err error) { - ch <- events.NewEvent().WithErrorEvent(events.ErrorEvent{ - Error: err, - }) -} diff --git a/pkg/phase/executors/isogen_test.go b/pkg/phase/executors/isogen_test.go index 9837aba40..b1d7244ba 100644 --- a/pkg/phase/executors/isogen_test.go +++ b/pkg/phase/executors/isogen_test.go @@ -23,7 +23,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime/schema" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/container" @@ -53,20 +52,6 @@ container: executorBundlePath = "../../bootstrap/isogen/testdata/primary/site/test-site/ephemeral/bootstrap" ) -func TestRegisterIsogenExecutor(t *testing.T) { - registry := make(map[schema.GroupVersionKind]ifc.ExecutorFactory) - expectedGVK := schema.GroupVersionKind{ - Group: "airshipit.org", - Version: "v1alpha1", - Kind: "IsoConfiguration", - } - err := executors.RegisterIsogenExecutor(registry) - require.NoError(t, err) - - _, found := registry[expectedGVK] - assert.True(t, found) -} - func TestNewIsogenExecutor(t *testing.T) { execDoc, err := document.NewDocumentFromBytes([]byte(isogenExecutorDoc)) require.NoError(t, err) @@ -176,9 +161,3 @@ func TestIsogenExecutorRun(t *testing.T) { }) } } - -func testBundleFactory(path string) document.BundleFactoryFunc { - return func() (document.Bundle, error) { - return document.NewBundleByPath(path) - } -} diff --git a/pkg/phase/executors/k8s_applier.go b/pkg/phase/executors/k8s_applier.go index 602d66e5e..720bbef45 100644 --- a/pkg/phase/executors/k8s_applier.go +++ b/pkg/phase/executors/k8s_applier.go @@ -18,7 +18,6 @@ import ( "io" "time" - "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/cli-utils/pkg/common" airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" @@ -33,73 +32,47 @@ import ( "opendev.org/airship/airshipctl/pkg/phase/ifc" ) -// ExecutorOptions provide a way to configure executor -type ExecutorOptions struct { - BundleName string - ClusterName string +var _ ifc.Executor = &KubeApplierExecutor{} +// KubeApplierExecutor applies resources to kubernetes +type KubeApplierExecutor struct { + ExecutorBundle document.Bundle ExecutorDocument document.Document - BundleFactory document.BundleFactoryFunc - Kubeconfig kubeconfig.Interface + BundleName string Helper ifc.Helper - ClusterMap clustermap.ClusterMap -} -var _ ifc.Executor = &Executor{} - -// RegisterKubeApplierExecutor adds executor to phase executor registry -func RegisterKubeApplierExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error { - obj := &airshipv1.KubernetesApply{} - gvks, _, err := airshipv1.Scheme.ObjectKinds(obj) - if err != nil { - return err - } - registry[gvks[0]] = registerExecutor - return nil -} - -// registerExecutor is here so that executor in theory can be used outside phases -func registerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { - return NewKubeApplierExecutor(ExecutorOptions{ - ClusterName: cfg.ClusterName, - BundleName: cfg.PhaseName, - Helper: cfg.Helper, - ExecutorDocument: cfg.ExecutorDocument, - BundleFactory: cfg.BundleFactory, - Kubeconfig: cfg.KubeConfig, - ClusterMap: cfg.ClusterMap, - }) -} - -// Executor applies resources to kubernetes -type Executor struct { - Options ExecutorOptions - ExecutorBundle document.Bundle - - apiObject *airshipv1.KubernetesApply - cleanup kubeconfig.Cleanup + apiObject *airshipv1.KubernetesApply + cleanup kubeconfig.Cleanup + clusterMap clustermap.ClusterMap + clusterName string + kubeconfig kubeconfig.Interface } // NewKubeApplierExecutor returns instance of executor -func NewKubeApplierExecutor(opts ExecutorOptions) (*Executor, error) { +func NewKubeApplierExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { apiObj := &airshipv1.KubernetesApply{} - err := opts.ExecutorDocument.ToAPIObject(apiObj, airshipv1.Scheme) + err := cfg.ExecutorDocument.ToAPIObject(apiObj, airshipv1.Scheme) if err != nil { return nil, err } - bundle, err := opts.BundleFactory() + bundle, err := cfg.BundleFactory() if err != nil { return nil, err } - return &Executor{ - ExecutorBundle: bundle, - Options: opts, - apiObject: apiObj, + return &KubeApplierExecutor{ + ExecutorBundle: bundle, + BundleName: cfg.PhaseName, + Helper: cfg.Helper, + ExecutorDocument: cfg.ExecutorDocument, + apiObject: apiObj, + clusterMap: cfg.ClusterMap, + clusterName: cfg.ClusterName, + kubeconfig: cfg.KubeConfig, }, nil } // Run executor, should be performed in separate go routine -func (e *Executor) Run(ch chan events.Event, runOpts ifc.RunOptions) { +func (e *KubeApplierExecutor) Run(ch chan events.Event, runOpts ifc.RunOptions) { applier, filteredBundle, err := e.prepareApplier(ch) if err != nil { handleError(ch, err) @@ -120,20 +93,20 @@ func (e *Executor) Run(ch chan events.Event, runOpts ifc.RunOptions) { applyOptions := k8sapplier.ApplyOptions{ DryRunStrategy: dryRunStrategy, Prune: e.apiObject.Config.PruneOptions.Prune, - BundleName: e.Options.BundleName, + BundleName: e.BundleName, WaitTimeout: timeout, } applier.ApplyBundle(filteredBundle, applyOptions) } -func (e *Executor) prepareApplier(ch chan events.Event) (*k8sapplier.Applier, document.Bundle, error) { +func (e *KubeApplierExecutor) prepareApplier(ch chan events.Event) (*k8sapplier.Applier, document.Bundle, error) { log.Debug("Getting kubeconfig context name from cluster map") - context, err := e.Options.ClusterMap.ClusterKubeconfigContext(e.Options.ClusterName) + context, err := e.clusterMap.ClusterKubeconfigContext(e.clusterName) if err != nil { return nil, nil, err } log.Debug("Getting kubeconfig file information from kubeconfig provider") - path, cleanup, err := e.Options.Kubeconfig.GetFile() + path, cleanup, err := e.kubeconfig.GetFile() if err != nil { return nil, nil, err } @@ -151,12 +124,12 @@ func (e *Executor) prepareApplier(ch chan events.Event) (*k8sapplier.Applier, do } // Validate document set -func (e *Executor) Validate() error { +func (e *KubeApplierExecutor) Validate() error { return errors.ErrNotImplemented{} } // Render document set -func (e *Executor) Render(w io.Writer, o ifc.RenderOptions) error { +func (e *KubeApplierExecutor) Render(w io.Writer, o ifc.RenderOptions) error { bundle, err := e.ExecutorBundle.SelectBundle(o.FilterSelector) if err != nil { return err diff --git a/pkg/phase/executors/k8s_applier_test.go b/pkg/phase/executors/k8s_applier_test.go index 0ef0025ac..2f7a25a3a 100644 --- a/pkg/phase/executors/k8s_applier_test.go +++ b/pkg/phase/executors/k8s_applier_test.go @@ -23,13 +23,11 @@ import ( "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/cluster/clustermap" - "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/fs" "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/executors" "opendev.org/airship/airshipctl/pkg/phase/ifc" testfs "opendev.org/airship/airshipctl/testutil/fs" @@ -96,7 +94,7 @@ func TestNewKubeApplierExecutor(t *testing.T) { name: "valid executor", cfgDoc: ValidExecutorDoc, kubeconf: testKubeconfig(testValidKubeconfig), - helper: makeKubeApplierDefaultHelper(t), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), }, { @@ -109,7 +107,7 @@ metadata: labels: cli-utils.sigs.k8s.io/inventory-id: "some id"`, expectedErr: "wrong config document", - helper: makeKubeApplierDefaultHelper(t), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), }, @@ -118,7 +116,7 @@ metadata: cfgDoc: ValidExecutorDoc, expectedErr: "no such file or directory", kubeconf: testKubeconfig(testValidKubeconfig), - helper: makeKubeApplierDefaultHelper(t), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), bundleFactory: testBundleFactory("does not exist"), }, } @@ -131,10 +129,10 @@ metadata: require.NotNil(t, doc) exec, err := executors.NewKubeApplierExecutor( - executors.ExecutorOptions{ + ifc.ExecutorConfig{ ExecutorDocument: doc, BundleFactory: tt.bundleFactory, - Kubeconfig: tt.kubeconf, + KubeConfig: tt.kubeconf, Helper: tt.helper, }) if tt.expectedErr != "" { @@ -167,10 +165,10 @@ func TestKubeApplierExecutorRun(t *testing.T) { { name: "cant read kubeconfig error", containsErr: "no such file or directory", - helper: makeKubeApplierDefaultHelper(t), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), kubeconf: testKubeconfig(`invalid kubeconfig`), - execDoc: toKubernetesApply(t, ValidExecutorDocNamespaced), + execDoc: executorDoc(t, ValidExecutorDocNamespaced), clusterName: "ephemeral-cluster", clusterMap: clustermap.NewClusterMap(&v1alpha1.ClusterMap{ Map: map[string]*v1alpha1.Cluster{ @@ -181,10 +179,10 @@ func TestKubeApplierExecutorRun(t *testing.T) { { name: "error cluster not defined", containsErr: "cluster is not defined in in cluster map", - helper: makeKubeApplierDefaultHelper(t), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), kubeconf: testKubeconfig(testValidKubeconfig), - execDoc: toKubernetesApply(t, ValidExecutorDocNamespaced), + execDoc: executorDoc(t, ValidExecutorDocNamespaced), clusterMap: clustermap.NewClusterMap(v1alpha1.DefaultClusterMap()), }, } @@ -192,11 +190,11 @@ func TestKubeApplierExecutorRun(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { exec, err := executors.NewKubeApplierExecutor( - executors.ExecutorOptions{ + ifc.ExecutorConfig{ ExecutorDocument: tt.execDoc, Helper: tt.helper, BundleFactory: tt.bundleFactory, - Kubeconfig: tt.kubeconf, + KubeConfig: tt.kubeconf, ClusterMap: tt.clusterMap, ClusterName: tt.clusterName, }) @@ -225,7 +223,7 @@ func TestRender(t *testing.T) { execDoc, err := document.NewDocumentFromBytes([]byte(ValidExecutorDoc)) require.NoError(t, err) require.NotNil(t, execDoc) - exec, err := executors.NewKubeApplierExecutor(executors.ExecutorOptions{ + exec, err := executors.NewKubeApplierExecutor(ifc.ExecutorConfig{ BundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), ExecutorDocument: execDoc, }) @@ -240,42 +238,6 @@ func TestRender(t *testing.T) { assert.Contains(t, result, "ReplicationController") } -func makeKubeApplierDefaultHelper(t *testing.T) ifc.Helper { - t.Helper() - conf := &config.Config{ - CurrentContext: "default", - Contexts: map[string]*config.Context{ - "default": { - Manifest: "default-manifest", - }, - }, - Manifests: map[string]*config.Manifest{ - "default-manifest": { - MetadataPath: "metadata.yaml", - TargetPath: "../../k8s/applier/testdata", - PhaseRepositoryName: config.DefaultTestPhaseRepo, - Repositories: map[string]*config.Repository{ - config.DefaultTestPhaseRepo: { - URLString: "", - }, - }, - }, - }, - } - helper, err := phase.NewHelper(conf) - require.NoError(t, err) - require.NotNil(t, helper) - return helper -} - -// toKubernetesApply converts string to document object -func toKubernetesApply(t *testing.T, s string) document.Document { - doc, err := document.NewDocumentFromBytes([]byte(s)) - require.NoError(t, err) - require.NotNil(t, doc) - return doc -} - func testKubeconfig(stringData string) kubeconfig.Interface { return kubeconfig.NewKubeConfig( kubeconfig.FromByte([]byte(stringData)), diff --git a/pkg/phase/ifc/executor.go b/pkg/phase/ifc/executor.go index c7da6a448..6308e1d6b 100644 --- a/pkg/phase/ifc/executor.go +++ b/pkg/phase/ifc/executor.go @@ -57,6 +57,7 @@ type ExecutorFactory func(config ExecutorConfig) (Executor, error) type ExecutorConfig struct { PhaseName string ClusterName string + BundleName string ClusterMap clustermap.ClusterMap ExecutorDocument document.Document