From bd287ce369db090ffd1ccd2db533fdd2dec3fd7b Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Mon, 5 Apr 2021 00:19:19 -0500 Subject: [PATCH] Add new selector by ObjectReference This selector will allow us to improve internal logic in places where we create selectors by ObjectReference objects and reuse the code. Change-Id: I4e4808bfdffc4446e9df255e2ed0b7b8f47d135c Signed-off-by: Ruslan Aliev --- pkg/document/selectors.go | 11 +++++- pkg/document/selectors_test.go | 38 +++++++++++++++++++ pkg/phase/client.go | 20 ++-------- pkg/phase/client_test.go | 7 ---- pkg/phase/executors/container.go | 7 +--- pkg/phase/helper.go | 19 ++++------ pkg/phase/helper_test.go | 7 ++++ .../phases/sample.yaml | 4 ++ 8 files changed, 71 insertions(+), 42 deletions(-) diff --git a/pkg/document/selectors.go b/pkg/document/selectors.go index 71c4ea82d..2d3384382 100644 --- a/pkg/document/selectors.go +++ b/pkg/document/selectors.go @@ -18,9 +18,9 @@ import ( "fmt" "strings" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/resid" ) @@ -104,6 +104,15 @@ func (s Selector) ByObject(obj runtime.Object, scheme *runtime.Scheme) (Selector return result, nil } +// ByObjectReference select by ObjectReference +func (s Selector) ByObjectReference(objRef *corev1.ObjectReference) Selector { + refGVK := objRef.GroupVersionKind() + return NewSelector(). + ByGvk(refGVK.Group, refGVK.Version, refGVK.Kind). + ByName(objRef.Name). + ByNamespace(objRef.Namespace) +} + // String is a convenience function which dumps all relevant information about a Selector in the following format: // [Key1=Value1, Key2=Value2, ...] func (s Selector) String() string { diff --git a/pkg/document/selectors_test.go b/pkg/document/selectors_test.go index 39583c9e0..5b14d4f0f 100644 --- a/pkg/document/selectors_test.go +++ b/pkg/document/selectors_test.go @@ -208,3 +208,41 @@ func TestSelectorToObject(t *testing.T) { }) } } + +func TestSelectorByObjRef(t *testing.T) { + tests := []struct { + name string + objRef *k8sv1.ObjectReference + expectedSel document.Selector + }{ + { + name: "Selector with GVK, name and namespace", + objRef: &k8sv1.ObjectReference{ + Kind: "TestKind", + Name: "TestName", + APIVersion: "api.version/v1", + Namespace: "TestNamespace", + }, + expectedSel: document.Selector{ + Selector: types.Selector{ + ResId: resid.ResId{ + Gvk: resid.Gvk{ + Group: "api.version", + Version: "v1", + Kind: "TestKind", + }, + Name: "TestName", + Namespace: "TestNamespace", + }, + }, + }, + }, + } + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + actualSel := document.NewSelector().ByObjectReference(tt.objRef) + assert.Equal(t, tt.expectedSel, actualSel) + }) + } +} diff --git a/pkg/phase/client.go b/pkg/phase/client.go index 579670077..f8c2decf2 100644 --- a/pkg/phase/client.go +++ b/pkg/phase/client.go @@ -63,24 +63,10 @@ type phase struct { processor events.EventProcessor } -func (p *phase) defaultBundleFactory() document.BundleFactoryFunc { - return document.BundleFactoryFromDocRoot(p.DocumentRoot) -} - -func (p *phase) defaultDocFactory() document.DocFactoryFunc { - return func() (document.Document, error) { - return p.helper.ExecutorDoc(ifc.ID{Name: p.apiObj.Name, Namespace: p.apiObj.Namespace}) - } -} - // Executor returns executor interface associated with the phase func (p *phase) Executor() (ifc.Executor, error) { - return p.executor(p.defaultDocFactory(), p.defaultBundleFactory()) -} - -func (p *phase) executor(docFactory document.DocFactoryFunc, - bundleFactory document.BundleFactoryFunc) (ifc.Executor, error) { - executorDoc, err := docFactory() + executorDoc, err := p.helper.PhaseConfigBundle().SelectOne( + document.NewSelector().ByObjectReference(p.apiObj.Config.ExecutorRef)) if err != nil { return nil, err } @@ -112,7 +98,7 @@ func (p *phase) executor(docFactory document.DocFactoryFunc, return executorFactory( ifc.ExecutorConfig{ ClusterMap: cMap, - BundleFactory: bundleFactory, + BundleFactory: document.BundleFactoryFromDocRoot(p.DocumentRoot), PhaseName: p.apiObj.Name, KubeConfig: kubeconf, ExecutorDocument: executorDoc, diff --git a/pkg/phase/client_test.go b/pkg/phase/client_test.go index 8c9576cb5..bc953f6aa 100644 --- a/pkg/phase/client_test.go +++ b/pkg/phase/client_test.go @@ -233,13 +233,6 @@ func TestPhaseValidate(t *testing.T) { registryFunc: fakeRegistry, errContains: "executor identified by 'airshipit.org/v1alpha1, Kind=SomeExecutor' is not found", }, - { - name: "Error no executor", - configFunc: testConfig, - phaseID: ifc.ID{Name: "no_executor_phase"}, - registryFunc: fakeRegistry, - errContains: "Phase name 'no_executor_phase', namespace '' must have executorRef field defined in config", - }, { name: "Error executor validate", configFunc: testConfig, diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index b47e9abed..a3489d401 100644 --- a/pkg/phase/executors/container.go +++ b/pkg/phase/executors/container.go @@ -182,12 +182,7 @@ func (c *ContainerExecutor) Render(w io.Writer, o 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) - 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.Options.PhaseConfigBundle.SelectOne(selector) + doc, err := c.Options.PhaseConfigBundle.SelectOne(document.NewSelector().ByObjectReference(c.Container.ConfigRef)) if err != nil { return err } diff --git a/pkg/phase/helper.go b/pkg/phase/helper.go index 03751d57b..b93c5989f 100644 --- a/pkg/phase/helper.go +++ b/pkg/phase/helper.go @@ -110,6 +110,13 @@ func (helper *Helper) Phase(phaseID ifc.ID) (*v1alpha1.Phase, error) { if err = doc.ToAPIObject(phase, v1alpha1.Scheme); err != nil { return nil, err } + // Phase must contain an executor + if phase.Config.ExecutorRef == nil { + return nil, errors.ErrExecutorRefNotDefined{ + PhaseName: phase.Name, + PhaseNamespace: phase.Namespace, + } + } return phase, nil } @@ -271,19 +278,9 @@ func (helper *Helper) ExecutorDoc(phaseID ifc.ID) (document.Document, error) { return nil, err } - phaseConfig := phaseObj.Config - if phaseConfig.ExecutorRef == nil { - return nil, errors.ErrExecutorRefNotDefined{PhaseName: phaseID.Name, PhaseNamespace: phaseID.Namespace} - } - // Searching executor configuration document referenced in // phase configuration - refGVK := phaseConfig.ExecutorRef.GroupVersionKind() - selector := document.NewSelector(). - ByGvk(refGVK.Group, refGVK.Version, refGVK.Kind). - ByName(phaseConfig.ExecutorRef.Name). - ByNamespace(phaseConfig.ExecutorRef.Namespace) - return helper.phaseConfigBundle.SelectOne(selector) + return helper.phaseConfigBundle.SelectOne(document.NewSelector().ByObjectReference(phaseObj.Config.ExecutorRef)) } // TargetPath returns manifest root diff --git a/pkg/phase/helper_test.go b/pkg/phase/helper_test.go index 3088b05ac..493a4f3d2 100644 --- a/pkg/phase/helper_test.go +++ b/pkg/phase/helper_test.go @@ -75,6 +75,12 @@ func TestHelperPhase(t *testing.T) { phaseID: ifc.ID{Name: "some_name"}, errContains: "found no documents", }, + { + name: "Error no executor", + config: testConfig, + phaseID: ifc.ID{Name: "no_executor_phase"}, + errContains: "Phase name 'no_executor_phase', namespace '' must have executorRef field defined in config", + }, { name: "Error bundle path doesn't exist", config: func(t *testing.T) *config.Config { @@ -95,6 +101,7 @@ func TestHelperPhase(t *testing.T) { require.Error(t, err) return } + require.NoError(t, err) require.NotNil(t, helper) actualPhase, actualErr := helper.Phase(tt.phaseID) if tt.errContains != "" { diff --git a/pkg/phase/testdata/valid_site_with_doc_prefix/phases/sample.yaml b/pkg/phase/testdata/valid_site_with_doc_prefix/phases/sample.yaml index abe9b0007..949a6c59b 100644 --- a/pkg/phase/testdata/valid_site_with_doc_prefix/phases/sample.yaml +++ b/pkg/phase/testdata/valid_site_with_doc_prefix/phases/sample.yaml @@ -3,4 +3,8 @@ kind: Phase metadata: name: sample config: + executorRef: + apiVersion: airshipit.org/v1alpha1 + kind: SomeExecutor + name: executor-name documentEntryPoint: entrypoint \ No newline at end of file