diff --git a/pkg/bootstrap/isogen/executor.go b/pkg/bootstrap/isogen/executor.go index 7c0e147af..8d1998bd6 100644 --- a/pkg/bootstrap/isogen/executor.go +++ b/pkg/bootstrap/isogen/executor.go @@ -62,8 +62,13 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { return nil, err } + bundle, err := cfg.BundleFactory() + if err != nil { + return nil, err + } + return &Executor{ - ExecutorBundle: cfg.ExecutorBundle, + ExecutorBundle: bundle, ExecutorDocument: cfg.ExecutorDocument, imgConf: apiObj, }, nil diff --git a/pkg/bootstrap/isogen/executor_test.go b/pkg/bootstrap/isogen/executor_test.go index 38c5f1429..f1b930444 100644 --- a/pkg/bootstrap/isogen/executor_test.go +++ b/pkg/bootstrap/isogen/executor_test.go @@ -66,17 +66,16 @@ func TestRegisterExecutor(t *testing.T) { func TestNewExecutor(t *testing.T) { execDoc, err := document.NewDocumentFromBytes([]byte(executorDoc)) require.NoError(t, err) - bundle, err := document.NewBundleByPath(executorBundlePath) - require.NoError(t, err) _, err = NewExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, - ExecutorBundle: bundle}) + BundleFactory: testBundleFactory(executorBundlePath)}) require.NoError(t, err) } func TestExecutorRun(t *testing.T) { bundle, err := document.NewBundleByPath(executorBundlePath) - require.NoError(t, err, "Building Bundle Failed") + require.NoError(t, err) + require.NotNil(t, bundle) tempVol, cleanup := testutil.TempDir(t, "bootstrap-test") defer cleanup(t) @@ -159,7 +158,6 @@ func TestExecutorRun(t *testing.T) { imgConf: testCfg, builder: tt.builder, } - require.NoError(t, err) ch := make(chan events.Event) go executor.Run(ch, ifc.RunOptions{}) var actualEvt []events.Event @@ -183,3 +181,9 @@ func wrapError(err error) events.Event { }, } } + +func testBundleFactory(path string) document.BundleFactoryFunc { + return func() (document.Bundle, error) { + return document.NewBundleByPath(path) + } +} diff --git a/pkg/clusterctl/client/executor.go b/pkg/clusterctl/client/executor.go index e968e702b..0f31d9ce4 100644 --- a/pkg/clusterctl/client/executor.go +++ b/pkg/clusterctl/client/executor.go @@ -21,7 +21,6 @@ import ( airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/cluster/clustermap" - "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/events" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" @@ -36,7 +35,6 @@ type ClusterctlExecutor struct { clusterName string Interface - bundle document.Bundle clusterMap clustermap.ClusterMap options *airshipv1.Clusterctl kubecfg kubeconfig.Interface @@ -66,7 +64,6 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { return &ClusterctlExecutor{ clusterName: cfg.ClusterName, Interface: client, - bundle: cfg.ExecutorBundle, options: options, kubecfg: cfg.KubeConfig, clusterMap: cfg.ClusterMap, diff --git a/pkg/clusterctl/client/executor_test.go b/pkg/clusterctl/client/executor_test.go index 6575e4a80..bef6efe8a 100644 --- a/pkg/clusterctl/client/executor_test.go +++ b/pkg/clusterctl/client/executor_test.go @@ -72,9 +72,6 @@ func TestRegisterExecutor(t *testing.T) { func TestNewExecutor(t *testing.T) { sampleCfgDoc := executorDoc(t, "init") - bundle, err := document.NewBundleByPath("testdata/executor_init") - require.NoError(t, err) - testCases := []struct { name string helper ifc.Helper @@ -90,13 +87,11 @@ func TestNewExecutor(t *testing.T) { t.Run(tt.name, func(t *testing.T) { _, actualErr := cctlclient.NewExecutor(ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - ExecutorBundle: bundle, Helper: tt.helper, }) assert.Equal(t, tt.expectedErr, actualErr) }) } - require.NoError(t, err) } func TestExecutorRun(t *testing.T) { @@ -170,8 +165,6 @@ func TestExecutorRun(t *testing.T) { for _, test := range testCases { tt := test t.Run(tt.name, func(t *testing.T) { - bundle, err := document.NewBundleByPath(tt.bundlePath) - require.NoError(t, err) kubeCfg := kubeconfig.NewKubeConfig( kubeconfig.FromByte([]byte("someKubeConfig")), kubeconfig.InjectFileSystem(tt.fs), @@ -179,7 +172,6 @@ func TestExecutorRun(t *testing.T) { executor, err := cctlclient.NewExecutor( ifc.ExecutorConfig{ ExecutorDocument: tt.cfgDoc, - ExecutorBundle: bundle, Helper: makeDefaultHelper(t), KubeConfig: kubeCfg, }) @@ -201,12 +193,9 @@ func TestExecutorRun(t *testing.T) { func TestExecutorValidate(t *testing.T) { sampleCfgDoc := executorDoc(t, "init") - bundle, err := document.NewBundleByPath("testdata/executor_init") - require.NoError(t, err) executor, err := cctlclient.NewExecutor( ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - ExecutorBundle: bundle, Helper: makeDefaultHelper(t), }) require.NoError(t, err) @@ -216,19 +205,16 @@ func TestExecutorValidate(t *testing.T) { } func TestExecutorRender(t *testing.T) { sampleCfgDoc := executorDoc(t, "init") - bundle, err := document.NewBundleByPath("testdata/executor_init") - require.NoError(t, err) - executor, err := cctlclient.NewExecutor( ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - ExecutorBundle: bundle, Helper: makeDefaultHelper(t), }) require.NoError(t, err) actualOut := &bytes.Buffer{} actualErr := executor.Render(actualOut, ifc.RenderOptions{}) - assert.Equal(t, nil, actualErr) + assert.Len(t, actualOut.Bytes(), 0) + assert.NoError(t, actualErr) } func makeDefaultHelper(t *testing.T) ifc.Helper { diff --git a/pkg/document/bundle.go b/pkg/document/bundle.go index 876d91ee7..7cb1c8df3 100644 --- a/pkg/document/bundle.go +++ b/pkg/document/bundle.go @@ -66,6 +66,10 @@ type Bundle interface { var pluginPath string var pluginPathLock = &sync.Mutex{} +// BundleFactoryFunc is a function that returns bundle, can be used to build bundle on demand +// instead of inplace, useful, when you don't know if bundle will be needed or not, see phase for detail +type BundleFactoryFunc func() (Bundle, error) + // NewBundleByPath helper function that returns new document.Bundle interface based on clusterType and // phase, example: helpers.NewBunde(airConfig, "ephemeral", "initinfra") func NewBundleByPath(rootPath string) (Bundle, error) { diff --git a/pkg/k8s/applier/executor.go b/pkg/k8s/applier/executor.go index 3391f9a50..e85b5a7f4 100644 --- a/pkg/k8s/applier/executor.go +++ b/pkg/k8s/applier/executor.go @@ -37,7 +37,7 @@ type ExecutorOptions struct { ClusterName string ExecutorDocument document.Document - ExecutorBundle document.Bundle + BundleFactory document.BundleFactoryFunc Kubeconfig kubeconfig.Interface Helper ifc.Helper } @@ -61,15 +61,16 @@ func registerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { ClusterName: cfg.ClusterName, BundleName: cfg.PhaseName, Helper: cfg.Helper, - ExecutorBundle: cfg.ExecutorBundle, ExecutorDocument: cfg.ExecutorDocument, + BundleFactory: cfg.BundleFactory, Kubeconfig: cfg.KubeConfig, }) } // Executor applies resources to kubernetes type Executor struct { - Options ExecutorOptions + Options ExecutorOptions + ExecutorBundle document.Bundle apiObject *airshipv1.KubernetesApply cleanup kubeconfig.Cleanup @@ -82,12 +83,14 @@ func NewExecutor(opts ExecutorOptions) (*Executor, error) { if err != nil { return nil, err } - if opts.ExecutorBundle == nil { - return nil, ErrNilBundle{} + bundle, err := opts.BundleFactory() + if err != nil { + return nil, err } return &Executor{ - Options: opts, - apiObject: apiObj, + ExecutorBundle: bundle, + Options: opts, + apiObject: apiObj, }, nil } @@ -126,7 +129,7 @@ func (e *Executor) prepareApplier(ch chan events.Event) (*Applier, document.Bund return nil, nil, err } log.Debug("Filtering out documents that shouldn't be applied to kubernetes from document bundle") - bundle, err := e.Options.ExecutorBundle.SelectBundle(document.NewDeployToK8sSelector()) + bundle, err := e.ExecutorBundle.SelectBundle(document.NewDeployToK8sSelector()) if err != nil { cleanup() return nil, nil, err @@ -146,7 +149,7 @@ func (e *Executor) Validate() error { // Render document set func (e *Executor) Render(w io.Writer, o ifc.RenderOptions) error { - bundle, err := e.Options.ExecutorBundle.SelectBundle(o.FilterSelector) + bundle, err := e.ExecutorBundle.SelectBundle(o.FilterSelector) if err != nil { return err } diff --git a/pkg/k8s/applier/executor_test.go b/pkg/k8s/applier/executor_test.go index a6724edbe..875afa35f 100644 --- a/pkg/k8s/applier/executor_test.go +++ b/pkg/k8s/applier/executor_test.go @@ -82,21 +82,19 @@ users: func TestNewExecutor(t *testing.T) { tests := []struct { - name string - cfgDoc string - expectedErr string - helper ifc.Helper - kubeconf kubeconfig.Interface - bundleFunc func(t *testing.T) document.Bundle + name string + cfgDoc string + expectedErr string + helper ifc.Helper + kubeconf kubeconfig.Interface + bundleFactory document.BundleFactoryFunc }{ { - name: "valid executor", - cfgDoc: ValidExecutorDoc, - kubeconf: testKubeconfig(testValidKubeconfig), - helper: makeDefaultHelper(t), - bundleFunc: func(t *testing.T) document.Bundle { - return newBundle("testdata/source_bundle", t) - }, + name: "valid executor", + cfgDoc: ValidExecutorDoc, + kubeconf: testKubeconfig(testValidKubeconfig), + helper: makeDefaultHelper(t), + bundleFactory: testBundleFactory("testdata/source_bundle"), }, { name: "wrong config document", @@ -107,11 +105,18 @@ metadata: namespace: default labels: cli-utils.sigs.k8s.io/inventory-id: "some id"`, - expectedErr: "wrong config document", - helper: makeDefaultHelper(t), - bundleFunc: func(t *testing.T) document.Bundle { - return newBundle("testdata/source_bundle", t) - }, + expectedErr: "wrong config document", + helper: makeDefaultHelper(t), + bundleFactory: testBundleFactory("testdata/source_bundle"), + }, + + { + name: "path to bundle does not exist", + cfgDoc: ValidExecutorDoc, + expectedErr: "no such file or directory", + kubeconf: testKubeconfig(testValidKubeconfig), + helper: makeDefaultHelper(t), + bundleFactory: testBundleFactory("does not exist"), }, } @@ -125,7 +130,7 @@ metadata: exec, err := applier.NewExecutor( applier.ExecutorOptions{ ExecutorDocument: doc, - ExecutorBundle: tt.bundleFunc(t), + BundleFactory: tt.bundleFactory, Kubeconfig: tt.kubeconf, Helper: tt.helper, }) @@ -149,30 +154,18 @@ func TestExecutorRun(t *testing.T) { name string containsErr string - kubeconf kubeconfig.Interface - execDoc document.Document - bundleFunc func(t *testing.T) document.Bundle - helper ifc.Helper + kubeconf kubeconfig.Interface + execDoc document.Document + bundleFactory document.BundleFactoryFunc + helper ifc.Helper }{ { - name: "cant read kubeconfig error", - containsErr: "no such file or directory", - helper: makeDefaultHelper(t), - bundleFunc: func(t *testing.T) document.Bundle { - return newBundle("testdata/source_bundle", t) - }, - kubeconf: testKubeconfig(`invalid kubeconfig`), - execDoc: toKubernetesApply(t, ValidExecutorDocNamespaced), - }, - { - name: "Nil bundle provided", - execDoc: toKubernetesApply(t, ValidExecutorDoc), - containsErr: "nil bundle provided", - kubeconf: testKubeconfig(testValidKubeconfig), - helper: makeDefaultHelper(t), - bundleFunc: func(t *testing.T) document.Bundle { - return nil - }, + name: "cant read kubeconfig error", + containsErr: "no such file or directory", + helper: makeDefaultHelper(t), + bundleFactory: testBundleFactory("testdata/source_bundle"), + kubeconf: testKubeconfig(`invalid kubeconfig`), + execDoc: toKubernetesApply(t, ValidExecutorDocNamespaced), }, } for _, tt := range tests { @@ -182,7 +175,7 @@ func TestExecutorRun(t *testing.T) { applier.ExecutorOptions{ ExecutorDocument: tt.execDoc, Helper: tt.helper, - ExecutorBundle: tt.bundleFunc(t), + BundleFactory: tt.bundleFactory, Kubeconfig: tt.kubeconf, }) if tt.name == "Nil bundle provided" { @@ -211,7 +204,7 @@ func TestRender(t *testing.T) { require.NoError(t, err) require.NotNil(t, execDoc) exec, err := applier.NewExecutor(applier.ExecutorOptions{ - ExecutorBundle: newBundle("testdata/source_bundle", t), + BundleFactory: testBundleFactory("testdata/source_bundle"), ExecutorDocument: execDoc, }) require.NoError(t, err) @@ -271,3 +264,9 @@ func testKubeconfig(stringData string) kubeconfig.Interface { }, )) } + +func testBundleFactory(path string) document.BundleFactoryFunc { + return func() (document.Bundle, error) { + return document.NewBundleByPath(path) + } +} diff --git a/pkg/phase/client.go b/pkg/phase/client.go index 0ca041aa0..a9c674845 100644 --- a/pkg/phase/client.go +++ b/pkg/phase/client.go @@ -68,13 +68,12 @@ func (p *phase) Executor() (ifc.Executor, error) { return nil, err } - var bundle document.Bundle - // just pass nil bundle if DocumentRoot is empty, executors should be ready for that - if docRoot := p.DocumentRoot(); docRoot != "" { - bundle, err = document.NewBundleByPath(docRoot) - if err != nil { - return nil, err + var bundleFactory document.BundleFactoryFunc = func() (document.Bundle, error) { + docRoot, bundleFactoryFuncErr := p.DocumentRoot() + if bundleFactoryFuncErr != nil { + return nil, bundleFactoryFuncErr } + return document.NewBundleByPath(docRoot) } refGVK := p.apiObj.Config.ExecutorRef.GroupVersionKind() @@ -103,7 +102,7 @@ func (p *phase) Executor() (ifc.Executor, error) { return executorFactory( ifc.ExecutorConfig{ ClusterMap: cMap, - ExecutorBundle: bundle, + BundleFactory: bundleFactory, PhaseName: p.apiObj.Name, KubeConfig: kubeconf, ExecutorDocument: executorDoc, @@ -143,13 +142,17 @@ func (p *phase) Render(w io.Writer, options ifc.RenderOptions) error { } // DocumentRoot root that holds all the documents associated with the phase -func (p *phase) DocumentRoot() string { - if p.apiObj.Config.DocumentEntryPoint == "" { - return "" +func (p *phase) DocumentRoot() (string, error) { + relativePath := p.apiObj.Config.DocumentEntryPoint + if relativePath == "" { + return "", ErrDocumentEntrypointNotDefined{ + PhaseName: p.apiObj.Name, + PhaseNamespace: p.apiObj.Namespace, + } } targetPath := p.helper.TargetPath() - return filepath.Join(targetPath, p.apiObj.Config.DocumentEntryPoint) + return filepath.Join(targetPath, relativePath), nil } // Details returns description of the phase diff --git a/pkg/phase/client_test.go b/pkg/phase/client_test.go index 89d8886b2..6d0ec41b9 100644 --- a/pkg/phase/client_test.go +++ b/pkg/phase/client_test.go @@ -168,6 +168,44 @@ func fakeRegistry() map[schema.GroupVersionKind]ifc.ExecutorFactory { } } +func TestDocumentRoot(t *testing.T) { + tests := []struct { + name string + expectedRoot string + phaseID ifc.ID + expectedErr error + }{ + { + name: "Success entrypoint exists", + expectedRoot: "testdata/valid_site/phases", + phaseID: ifc.ID{Name: "capi_init"}, + }, + { + name: "Error entrypoint does not exists", + phaseID: ifc.ID{Name: "some_phase"}, + expectedErr: phase.ErrDocumentEntrypointNotDefined{PhaseName: "some_phase"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + cfg := testConfig(t) + helper, err := phase.NewHelper(cfg) + require.NoError(t, err) + require.NotNil(t, helper) + + c := phase.NewClient(helper) + p, err := c.PhaseByID(ifc.ID{Name: tt.phaseID.Name, Namespace: tt.phaseID.Namespace}) + require.NoError(t, err) + require.NotNil(t, p) + + root, err := p.DocumentRoot() + assert.Equal(t, tt.expectedErr, err) + assert.Equal(t, tt.expectedRoot, root) + }) + } +} + func fakeExecFactory(config ifc.ExecutorConfig) (ifc.Executor, error) { return fakeExecutor{}, nil } diff --git a/pkg/phase/errors.go b/pkg/phase/errors.go index 499b2fd82..ce2573dd0 100644 --- a/pkg/phase/errors.go +++ b/pkg/phase/errors.go @@ -39,3 +39,15 @@ type ErrExecutorRegistration struct { 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) +} diff --git a/pkg/phase/ifc/executor.go b/pkg/phase/ifc/executor.go index 340aa97ad..b488cdca2 100644 --- a/pkg/phase/ifc/executor.go +++ b/pkg/phase/ifc/executor.go @@ -60,8 +60,8 @@ type ExecutorConfig struct { ClusterMap clustermap.ClusterMap ExecutorDocument document.Document - ExecutorBundle document.Bundle AirshipConfig *config.Config Helper Helper KubeConfig kubeconfig.Interface + BundleFactory document.BundleFactoryFunc } diff --git a/pkg/phase/ifc/phase.go b/pkg/phase/ifc/phase.go index 223991fd5..2cdb07ea4 100644 --- a/pkg/phase/ifc/phase.go +++ b/pkg/phase/ifc/phase.go @@ -25,7 +25,7 @@ import ( type Phase interface { Validate() error Run(RunOptions) error - DocumentRoot() string + DocumentRoot() (string, error) Details() (string, error) Executor() (Executor, error) Render(io.Writer, RenderOptions) error diff --git a/pkg/remote/management.go b/pkg/remote/management.go index ce61881cb..05f383252 100644 --- a/pkg/remote/management.go +++ b/pkg/remote/management.go @@ -138,7 +138,12 @@ func NewManager(cfg *config.Config, phaseName string, hosts ...HostSelector) (*M return nil, err } - docBundle, err := document.NewBundleByPath(phase.DocumentRoot()) + docRoot, err := phase.DocumentRoot() + if err != nil { + return nil, err + } + + docBundle, err := document.NewBundleByPath(docRoot) if err != nil { return nil, err } diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index 53504c7df..59ca87a0d 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -37,7 +37,12 @@ func (b baremetalHost) DoRemoteDirect(cfg *config.Config) error { return err } - docBundle, err := document.NewBundleByPath(phase.DocumentRoot()) + docRoot, err := phase.DocumentRoot() + if err != nil { + return err + } + + docBundle, err := document.NewBundleByPath(docRoot) if err != nil { return err }