airship
/
airshipctl
Code Issues Proposed changes

Merge "Use BundleFactory instead of bundle in executors"

This commit is contained in:
Zuul 2020-09-30 14:24:52 +00:00 committed by Gerrit Code Review
parent d4ea864eeb e6e56ffa2e
commit e4a2c68d3e
14 changed files with 153 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/bootstrap/isogen/executor.go
View File

@ -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

14
pkg/bootstrap/isogen/executor_test.go
View File

@ -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)
}
}

3
pkg/clusterctl/client/executor.go
View File

@ -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,

18
pkg/clusterctl/client/executor_test.go
View File

@ -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 {

4
pkg/document/bundle.go
View File

@ -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) {

21
pkg/k8s/applier/executor.go
View File

@ -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
}

85
pkg/k8s/applier/executor_test.go
View File

@ -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)
}
}

25
pkg/phase/client.go
View File

@ -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

38
pkg/phase/client_test.go
View File

@ -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
}

12
pkg/phase/errors.go
View File

@ -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)
}

2
pkg/phase/ifc/executor.go
View File

@ -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
}

2
pkg/phase/ifc/phase.go
View File

@ -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

7
pkg/remote/management.go
View File

@ -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
}

7
pkg/remote/remote_direct.go
View File

@ -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
}