From 8bed52b05746f16206b8c1fcd128c4e32d56caa0 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Wed, 10 Mar 2021 14:13:31 -0600 Subject: [PATCH] Refactor phase related errors * phase related errors were moved to separate package to prevent cycling import error; * error duplicates were removed; * non-related to package errors were moved to appropriate ones. Change-Id: If6cfb08c48d356c1517a13157a5fe740dc1635a4 Signed-off-by: Ruslan Aliev Relates-To: #485 --- pkg/errors/common.go | 9 --- pkg/phase/client.go | 8 ++- pkg/phase/client_test.go | 5 +- pkg/phase/errors.go | 84 ---------------------- pkg/phase/errors/errors.go | 62 ++++++++++++++++ pkg/phase/executors/baremetal_manager.go | 7 +- pkg/phase/executors/clusterctl.go | 17 ++--- pkg/phase/executors/clusterctl_test.go | 7 +- pkg/phase/executors/common.go | 3 +- pkg/phase/executors/container.go | 8 +-- pkg/phase/executors/{ => errors}/errors.go | 37 ++++++++-- pkg/phase/executors/k8s_applier.go | 5 +- pkg/phase/helper.go | 7 +- pkg/phase/helper_test.go | 3 +- pkg/phase/render.go | 8 ++- pkg/phase/render_test.go | 7 +- 16 files changed, 144 insertions(+), 133 deletions(-) delete mode 100644 pkg/phase/errors.go create mode 100644 pkg/phase/errors/errors.go rename pkg/phase/executors/{ => errors}/errors.go (62%) diff --git a/pkg/errors/common.go b/pkg/errors/common.go index 3165820d6..dc823461e 100644 --- a/pkg/errors/common.go +++ b/pkg/errors/common.go @@ -27,12 +27,3 @@ func (e ErrNotImplemented) Error() string { } return "not implemented" } - -// ErrInvalidPhase is returned if the phase is invalid -type ErrInvalidPhase struct { - Reason string -} - -func (e ErrInvalidPhase) Error() string { - return fmt.Sprintf("invalid phase: %s", e.Reason) -} diff --git a/pkg/phase/client.go b/pkg/phase/client.go index 9063235b2..29447e98f 100644 --- a/pkg/phase/client.go +++ b/pkg/phase/client.go @@ -28,7 +28,9 @@ import ( "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" "opendev.org/airship/airshipctl/pkg/k8s/utils" "opendev.org/airship/airshipctl/pkg/log" + "opendev.org/airship/airshipctl/pkg/phase/errors" "opendev.org/airship/airshipctl/pkg/phase/executors" + executorerrors "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -42,7 +44,7 @@ func DefaultExecutorRegistry() map[schema.GroupVersionKind]ifc.ExecutorFactory { for _, execName := range []string{executors.Clusterctl, executors.KubernetesApply, executors.GenericContainer, executors.Ephemeral, executors.BMHManager} { if err := executors.RegisterExecutor(execName, execMap); err != nil { - log.Fatal(ErrExecutorRegistration{ExecutorName: execName, Err: err}) + log.Fatal(executorerrors.ErrExecutorRegistration{ExecutorName: execName, Err: err}) } } return execMap @@ -78,7 +80,7 @@ func (p *phase) Executor() (ifc.Executor, error) { // Look for executor factory defined in registry executorFactory, found := p.registry()[refGVK] if !found { - return nil, ErrExecutorNotFound{GVK: refGVK} + return nil, executorerrors.ErrExecutorNotFound{GVK: refGVK} } cMap, err := p.helper.ClusterMap() @@ -195,7 +197,7 @@ func (p *phase) Status() (ifc.PhaseStatus, error) { func (p *phase) DocumentRoot() (string, error) { relativePath := p.apiObj.Config.DocumentEntryPoint if relativePath == "" { - return "", ErrDocumentEntrypointNotDefined{ + return "", errors.ErrDocumentEntrypointNotDefined{ PhaseName: p.apiObj.Name, PhaseNamespace: p.apiObj.Namespace, } diff --git a/pkg/phase/client_test.go b/pkg/phase/client_test.go index 394219318..b04a1237c 100644 --- a/pkg/phase/client_test.go +++ b/pkg/phase/client_test.go @@ -27,6 +27,7 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/phase" + "opendev.org/airship/airshipctl/pkg/phase/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -261,7 +262,7 @@ func TestDocumentRoot(t *testing.T) { { name: "Error entrypoint does not exists", phaseID: ifc.ID{Name: "some_phase"}, - expectedErr: phase.ErrDocumentEntrypointNotDefined{PhaseName: "some_phase"}, + expectedErr: errors.ErrDocumentEntrypointNotDefined{PhaseName: "some_phase"}, }, { name: "Success entrypoint with doc prefix path", @@ -331,7 +332,7 @@ func TestBundleFactoryExecutor(t *testing.T) { require.NoError(t, err) _, err = p.Executor() assert.Error(t, err) - assert.Equal(t, phase.ErrDocumentEntrypointNotDefined{PhaseName: "no_entry_point"}, err) + assert.Equal(t, errors.ErrDocumentEntrypointNotDefined{PhaseName: "no_entry_point"}, err) } func TestPlanRun(t *testing.T) { diff --git a/pkg/phase/errors.go b/pkg/phase/errors.go deleted file mode 100644 index 1789359f1..000000000 --- a/pkg/phase/errors.go +++ /dev/null @@ -1,84 +0,0 @@ -/* - 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 phase - -import ( - "fmt" - - "k8s.io/apimachinery/pkg/runtime/schema" -) - -// ErrExecutorNotFound is returned if phase executor was not found in executor -// registry map -type ErrExecutorNotFound struct { - GVK schema.GroupVersionKind -} - -func (e ErrExecutorNotFound) Error() string { - return fmt.Sprintf("executor identified by '%s' is not found", e.GVK) -} - -// ErrExecutorRegistration is a wrapper for executor registration errors -type ErrExecutorRegistration struct { - ExecutorName string - Err error -} - -func (e ErrExecutorRegistration) Error() string { - return fmt.Sprintf("failed to register executor %s, registration function returned %s", e.ExecutorName, e.Err.Error()) -} - -// ErrDocumentEntrypointNotDefined returned when phase has no entrypoint defined and phase needs it -type ErrDocumentEntrypointNotDefined struct { - PhaseName string - PhaseNamespace string -} - -func (e ErrDocumentEntrypointNotDefined) Error() string { - return fmt.Sprintf("documentEntryPoint is not defined for the phase '%s' in namespace '%s'", - e.PhaseName, - e.PhaseNamespace) -} - -// ErrExecutorRefNotDefined returned when phase has no entrypoint defined and phase needs it -type ErrExecutorRefNotDefined struct { - PhaseName string - PhaseNamespace string -} - -func (e ErrExecutorRefNotDefined) Error() string { - return fmt.Sprintf("Phase name '%s', namespace '%s' must have executorRef field defined in config", - e.PhaseName, - e.PhaseNamespace) -} - -// ErrUknownRenderSource returned when render command source doesn't match any known types -type ErrUknownRenderSource struct { - Source string -} - -func (e ErrUknownRenderSource) Error() string { - return fmt.Sprintf("wrong render source '%s' specified must be one of %s, %s, %s", - e.Source, RenderSourceConfig, RenderSourceExecutor, RenderSourceExecutor) -} - -// ErrRenderPhaseNameNotSpecified returned when render command is called with either phase or -// executor source and phase name is not specified -type ErrRenderPhaseNameNotSpecified struct{} - -func (e ErrRenderPhaseNameNotSpecified) Error() string { - return fmt.Sprintf("must specify phase name when using '%s' or '%s' as source", - RenderSourceExecutor, RenderSourcePhase) -} diff --git a/pkg/phase/errors/errors.go b/pkg/phase/errors/errors.go new file mode 100644 index 000000000..bd506ec6e --- /dev/null +++ b/pkg/phase/errors/errors.go @@ -0,0 +1,62 @@ +/* + 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 errors + +import ( + "fmt" +) + +// ErrDocumentEntrypointNotDefined returned when phase has no entrypoint defined and phase needs it +type ErrDocumentEntrypointNotDefined struct { + PhaseName string + PhaseNamespace string +} + +func (e ErrDocumentEntrypointNotDefined) Error() string { + return fmt.Sprintf("documentEntryPoint is not defined for the phase '%s' in namespace '%s'", + e.PhaseName, + e.PhaseNamespace) +} + +// ErrUnknownRenderSource returned when render command source doesn't match any known types +type ErrUnknownRenderSource struct { + Source string + ValidSources []string +} + +func (e ErrUnknownRenderSource) Error() string { + return fmt.Sprintf("wrong render source '%s' specified must be one of %v", + e.Source, e.ValidSources) +} + +// ErrRenderPhaseNameNotSpecified returned when render command is called with either phase or +// executor source and phase name is not specified +type ErrRenderPhaseNameNotSpecified struct { + Sources []string +} + +func (e ErrRenderPhaseNameNotSpecified) Error() string { + return fmt.Sprintf("must specify phase name when using %v as source", + e.Sources) +} + +// ErrInvalidPhase is returned if the phase is invalid +type ErrInvalidPhase struct { + Reason string +} + +func (e ErrInvalidPhase) Error() string { + return fmt.Sprintf("invalid phase: %s", e.Reason) +} diff --git a/pkg/phase/executors/baremetal_manager.go b/pkg/phase/executors/baremetal_manager.go index 5305daef3..821ceaa1f 100644 --- a/pkg/phase/executors/baremetal_manager.go +++ b/pkg/phase/executors/baremetal_manager.go @@ -21,10 +21,11 @@ import ( "opendev.org/airship/airshipctl/pkg/api/v1alpha1" airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" - "opendev.org/airship/airshipctl/pkg/errors" + commonerrors "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/inventory" inventoryifc "opendev.org/airship/airshipctl/pkg/inventory/ifc" + "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -112,7 +113,7 @@ func (e *BaremetalManagerExecutor) validate() (inventoryifc.BaremetalOperation, // TODO add remote direct validation, make sure that ISO-URL is specified result = "" default: - err = ErrUnknownExecutorAction{Action: string(e.options.Spec.Operation), ExecutorName: BMHManager} + err = errors.ErrUnknownExecutorAction{Action: string(e.options.Spec.Operation), ExecutorName: BMHManager} } return result, err } @@ -144,5 +145,5 @@ func toCommandOptions(i inventoryifc.Inventory, // Status returns the status of the given phase func (e *BaremetalManagerExecutor) Status() (ifc.ExecutorStatus, error) { - return ifc.ExecutorStatus{}, errors.ErrNotImplemented{What: BMHManager} + return ifc.ExecutorStatus{}, commonerrors.ErrNotImplemented{What: BMHManager} } diff --git a/pkg/phase/executors/clusterctl.go b/pkg/phase/executors/clusterctl.go index 7e5639afe..5029d45d9 100755 --- a/pkg/phase/executors/clusterctl.go +++ b/pkg/phase/executors/clusterctl.go @@ -24,11 +24,12 @@ import ( "opendev.org/airship/airshipctl/pkg/cluster/clustermap" "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/errors" airerrors "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" "opendev.org/airship/airshipctl/pkg/log" + phaseerrors "opendev.org/airship/airshipctl/pkg/phase/errors" + "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -72,7 +73,7 @@ func (c *ClusterctlExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { case airshipv1.Init: c.init(opts, evtCh) default: - handleError(evtCh, ErrUnknownExecutorAction{Action: string(c.options.Action), ExecutorName: "clusterctl"}) + handleError(evtCh, errors.ErrUnknownExecutorAction{Action: string(c.options.Action), ExecutorName: "clusterctl"}) } } @@ -174,17 +175,17 @@ func isAlreadyExistsError(err error) bool { func (c *ClusterctlExecutor) Validate() error { switch c.options.Action { case "": - return airerrors.ErrInvalidPhase{Reason: "ClusterctlExecutor.Action is empty"} + return phaseerrors.ErrInvalidPhase{Reason: "ClusterctlExecutor.Action is empty"} case airshipv1.Init: if c.options.InitOptions.CoreProvider == "" { - return airerrors.ErrInvalidPhase{Reason: "ClusterctlExecutor.InitOptions.CoreProvider is empty"} + return phaseerrors.ErrInvalidPhase{Reason: "ClusterctlExecutor.InitOptions.CoreProvider is empty"} } case airshipv1.Move: if c.options.MoveOptions.Namespace == "" { - return airerrors.ErrInvalidPhase{Reason: "ClusterctlExecutor.MoveOptions.Namespace is empty"} + return phaseerrors.ErrInvalidPhase{Reason: "ClusterctlExecutor.MoveOptions.Namespace is empty"} } default: - return ErrUnknownExecutorAction{Action: string(c.options.Action)} + return errors.ErrUnknownExecutorAction{Action: string(c.options.Action)} } // TODO: need to find if any other validation needs to be added return nil @@ -204,7 +205,7 @@ func (c *ClusterctlExecutor) Render(w io.Writer, ro ifc.RenderOptions) error { for _, prv := range prvList { res := strings.Split(prv, ":") if len(res) != 2 { - return ErrUnableParseProvider{ + return errors.ErrUnableParseProvider{ Provider: prv, ProviderType: prvType, } @@ -235,5 +236,5 @@ func (c *ClusterctlExecutor) Render(w io.Writer, ro ifc.RenderOptions) error { // Status returns the status of the given phase func (c *ClusterctlExecutor) Status() (ifc.ExecutorStatus, error) { - return ifc.ExecutorStatus{}, errors.ErrNotImplemented{What: Clusterctl} + return ifc.ExecutorStatus{}, airerrors.ErrNotImplemented{What: Clusterctl} } diff --git a/pkg/phase/executors/clusterctl_test.go b/pkg/phase/executors/clusterctl_test.go index 3fc6366c5..d3e278741 100755 --- a/pkg/phase/executors/clusterctl_test.go +++ b/pkg/phase/executors/clusterctl_test.go @@ -16,7 +16,7 @@ package executors_test import ( "bytes" - "errors" + goerrors "errors" "fmt" "testing" "time" @@ -31,6 +31,7 @@ import ( "opendev.org/airship/airshipctl/pkg/fs" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" "opendev.org/airship/airshipctl/pkg/phase/executors" + "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" testfs "opendev.org/airship/airshipctl/testutil/fs" ) @@ -126,7 +127,7 @@ func TestNewClusterctlExecutor(t *testing.T) { } func TestClusterctlExecutorRun(t *testing.T) { - errTmpFile := errors.New("TmpFile error") + errTmpFile := goerrors.New("TmpFile error") testCases := []struct { name string @@ -141,7 +142,7 @@ func TestClusterctlExecutorRun(t *testing.T) { cfgDoc: executorDoc(t, fmt.Sprintf(executorConfigTmplGood, "someAction")), bundlePath: "testdata/executor_init", expectedEvt: []events.Event{ - wrapError(executors.ErrUnknownExecutorAction{Action: "someAction", ExecutorName: "clusterctl"}), + wrapError(errors.ErrUnknownExecutorAction{Action: "someAction", ExecutorName: "clusterctl"}), }, clusterMap: clustermap.NewClusterMap(v1alpha1.DefaultClusterMap()), }, diff --git a/pkg/phase/executors/common.go b/pkg/phase/executors/common.go index 4c9f34914..9043f37e1 100755 --- a/pkg/phase/executors/common.go +++ b/pkg/phase/executors/common.go @@ -19,6 +19,7 @@ import ( airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/events" + "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -54,7 +55,7 @@ func RegisterExecutor(executorName string, registry map[schema.GroupVersionKind] gvks, _, err = airshipv1.Scheme.ObjectKinds(&airshipv1.BaremetalManager{}) execObj = NewBaremetalExecutor default: - return ErrUnknownExecutorName{ExecutorName: executorName} + return errors.ErrUnknownExecutorName{ExecutorName: executorName} } if err != nil { diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index 838613022..abfc0add7 100644 --- a/pkg/phase/executors/container.go +++ b/pkg/phase/executors/container.go @@ -23,7 +23,7 @@ import ( "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/container" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/errors" + commonerrors "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" "opendev.org/airship/airshipctl/pkg/log" @@ -165,12 +165,12 @@ func bundleReader(bundle document.Bundle) (io.Reader, error) { // Validate executor configuration and documents func (c *ContainerExecutor) Validate() error { - return errors.ErrNotImplemented{} + return commonerrors.ErrNotImplemented{} } // Render executor documents func (c *ContainerExecutor) Render(_ io.Writer, _ ifc.RenderOptions) error { - return errors.ErrNotImplemented{} + return commonerrors.ErrNotImplemented{} } func (c *ContainerExecutor) setConfig() error { @@ -202,5 +202,5 @@ func (c *ContainerExecutor) setConfig() error { // Status returns the status of the given phase func (c *ContainerExecutor) Status() (ifc.ExecutorStatus, error) { - return ifc.ExecutorStatus{}, errors.ErrNotImplemented{What: GenericContainer} + return ifc.ExecutorStatus{}, commonerrors.ErrNotImplemented{What: GenericContainer} } diff --git a/pkg/phase/executors/errors.go b/pkg/phase/executors/errors/errors.go similarity index 62% rename from pkg/phase/executors/errors.go rename to pkg/phase/executors/errors/errors.go index dfc960a1e..a0660651f 100755 --- a/pkg/phase/executors/errors.go +++ b/pkg/phase/executors/errors/errors.go @@ -12,10 +12,12 @@ limitations under the License. */ -package executors +package errors import ( "fmt" + + "k8s.io/apimachinery/pkg/runtime/schema" ) // ErrUnknownExecutorAction is returned when unknown action or operation is requested @@ -58,11 +60,34 @@ func (e ErrNilExecutorDoc) Error() string { return "executor document is nil" } -// ErrInvalidPhase is returned if the phase is invalid -type ErrInvalidPhase struct { - Reason string +// ErrExecutorRefNotDefined returned when phase has no entrypoint defined and phase needs it +type ErrExecutorRefNotDefined struct { + PhaseName string + PhaseNamespace string } -func (e ErrInvalidPhase) Error() string { - return fmt.Sprintf("invalid phase: %s", e.Reason) +func (e ErrExecutorRefNotDefined) Error() string { + return fmt.Sprintf("Phase name '%s', namespace '%s' must have executorRef field defined in config", + e.PhaseName, + e.PhaseNamespace) +} + +// ErrExecutorNotFound is returned if phase executor was not found in executor +// registry map +type ErrExecutorNotFound struct { + GVK schema.GroupVersionKind +} + +func (e ErrExecutorNotFound) Error() string { + return fmt.Sprintf("executor identified by '%s' is not found", e.GVK) +} + +// ErrExecutorRegistration is a wrapper for executor registration errors +type ErrExecutorRegistration struct { + ExecutorName string + Err error +} + +func (e ErrExecutorRegistration) Error() string { + return fmt.Sprintf("failed to register executor %s, registration function returned %s", e.ExecutorName, e.Err.Error()) } diff --git a/pkg/phase/executors/k8s_applier.go b/pkg/phase/executors/k8s_applier.go index 71e1317a0..a846e5bce 100644 --- a/pkg/phase/executors/k8s_applier.go +++ b/pkg/phase/executors/k8s_applier.go @@ -32,6 +32,7 @@ import ( "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" "opendev.org/airship/airshipctl/pkg/k8s/utils" "opendev.org/airship/airshipctl/pkg/log" + "opendev.org/airship/airshipctl/pkg/phase/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -129,14 +130,14 @@ func (e *KubeApplierExecutor) prepareApplier(ch chan events.Event) (*k8sapplier. // Validate document set func (e *KubeApplierExecutor) Validate() error { if e.BundleName == "" { - return ErrInvalidPhase{Reason: "k8s applier BundleName is empty"} + return errors.ErrInvalidPhase{Reason: "k8s applier BundleName is empty"} } docs, err := e.ExecutorBundle.GetAllDocuments() if err != nil { return err } if len(docs) == 0 { - return ErrInvalidPhase{Reason: "no executor documents in the bundle"} + return errors.ErrInvalidPhase{Reason: "no executor documents in the bundle"} } // TODO: need to find if any other validation needs to be added return nil diff --git a/pkg/phase/helper.go b/pkg/phase/helper.go index 4b8a83890..638e37fec 100644 --- a/pkg/phase/helper.go +++ b/pkg/phase/helper.go @@ -15,7 +15,7 @@ package phase import ( - "errors" + goerrors "errors" "path/filepath" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -27,6 +27,7 @@ import ( "opendev.org/airship/airshipctl/pkg/inventory" inventoryifc "opendev.org/airship/airshipctl/pkg/inventory/ifc" "opendev.org/airship/airshipctl/pkg/log" + "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -195,7 +196,7 @@ func (helper *Helper) getDocsByPhasePlan(planID ifc.ID, bundle document.Bundle) doc, filterErr := bundle.SelectOne(selector) if filterErr != nil { - if errors.As(filterErr, &document.ErrDocNotFound{}) { + if goerrors.As(filterErr, &document.ErrDocNotFound{}) { log.Debug(filterErr.Error()) continue } @@ -282,7 +283,7 @@ func (helper *Helper) ExecutorDoc(phaseID ifc.ID) (document.Document, error) { phaseConfig := phaseObj.Config if phaseConfig.ExecutorRef == nil { - return nil, ErrExecutorRefNotDefined{PhaseName: phaseID.Name, PhaseNamespace: phaseID.Namespace} + return nil, errors.ErrExecutorRefNotDefined{PhaseName: phaseID.Name, PhaseNamespace: phaseID.Namespace} } // Searching executor configuration document referenced in diff --git a/pkg/phase/helper_test.go b/pkg/phase/helper_test.go index 0d41a552e..ddb31a321 100644 --- a/pkg/phase/helper_test.go +++ b/pkg/phase/helper_test.go @@ -28,6 +28,7 @@ import ( airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/phase" + "opendev.org/airship/airshipctl/pkg/phase/executors/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -449,7 +450,7 @@ func TestHelperExecutorDoc(t *testing.T) { name: "Error get phase without executor", config: testConfig, phaseID: ifc.ID{Name: "no_executor_phase"}, - errContains: phase.ErrExecutorRefNotDefined{ + errContains: errors.ErrExecutorRefNotDefined{ PhaseName: "no_executor_phase", }.Error(), }, diff --git a/pkg/phase/render.go b/pkg/phase/render.go index a572afcb4..3597b5e8f 100644 --- a/pkg/phase/render.go +++ b/pkg/phase/render.go @@ -21,6 +21,7 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" + "opendev.org/airship/airshipctl/pkg/phase/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -122,10 +123,13 @@ func (fo *RenderCommand) Validate() (err error) { // do nothing, source config doesnt need any parameters case RenderSourceExecutor, RenderSourcePhase: if fo.PhaseID.Name == "" { - err = ErrRenderPhaseNameNotSpecified{} + err = errors.ErrRenderPhaseNameNotSpecified{Sources: []string{RenderSourceExecutor, RenderSourcePhase}} } default: - err = ErrUknownRenderSource{Source: fo.Source} + err = errors.ErrUnknownRenderSource{ + Source: fo.Source, + ValidSources: []string{RenderSourceConfig, RenderSourceExecutor, RenderSourcePhase}, + } } return err } diff --git a/pkg/phase/render_test.go b/pkg/phase/render_test.go index 28395f1af..821b500a0 100644 --- a/pkg/phase/render_test.go +++ b/pkg/phase/render_test.go @@ -26,6 +26,7 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/phase" + "opendev.org/airship/airshipctl/pkg/phase/errors" "opendev.org/airship/airshipctl/pkg/phase/ifc" "opendev.org/airship/airshipctl/testutil" ) @@ -114,14 +115,16 @@ func TestRender(t *testing.T) { settings: &phase.RenderCommand{ Source: "unknown", }, - expErr: phase.ErrUknownRenderSource{Source: "unknown"}, + expErr: errors.ErrUnknownRenderSource{Source: "unknown", + ValidSources: []string{phase.RenderSourceConfig, phase.RenderSourceExecutor, phase.RenderSourcePhase}}, }, { name: "phase name not specified", settings: &phase.RenderCommand{ Source: phase.RenderSourcePhase, }, - expErr: phase.ErrRenderPhaseNameNotSpecified{}, + expErr: errors.ErrRenderPhaseNameNotSpecified{ + Sources: []string{phase.RenderSourceExecutor, phase.RenderSourcePhase}}, }, }