airship/airshipctl
Code Issues Proposed changes

Move kubernetes-apply phase executor to a separate package

Browse Source 
Having an executor within applier package creates
potential import cycling. This patch moves it to a separate
package which can be used to conveniently store all the executors
at one place.

Change-Id: I68d909489b691e4fb7129446ef9a3fb085f8683c
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>
Relates-To: #432
This commit is contained in:
Ruslan Aliev 2020-12-10 09:27:55 -06:00
parent 36303006af
commit b85f8fa8f9
3 changed files with 32 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pkg/phase/client.go
View File

@ -24,7 +24,6 @@ import (
"opendev.org/airship/airshipctl/pkg/container"
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/events"
"opendev.org/airship/airshipctl/pkg/k8s/applier"
"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
"opendev.org/airship/airshipctl/pkg/k8s/utils"
"opendev.org/airship/airshipctl/pkg/log"
@ -42,7 +41,7 @@ func DefaultExecutorRegistry() map[schema.GroupVersionKind]ifc.ExecutorFactory {
if err := executors.RegisterExecutor(execMap); err != nil {
log.Fatal(ErrExecutorRegistration{ExecutorName: "clusterctl", Err: err})
}
if err := applier.RegisterExecutor(execMap); err != nil {
if err := executors.RegisterKubeApplierExecutor(execMap); err != nil {
log.Fatal(ErrExecutorRegistration{ExecutorName: "kubernetes-apply", Err: err})
}
if err := executors.RegisterIsogenExecutor(execMap); err != nil {

19
pkg/k8s/applier/executor.go → pkg/phase/executors/k8s_applier.go
View File

@ -12,7 +12,7 @@
limitations under the License.
*/
package applier
package executors
import (
"io"
@ -26,6 +26,7 @@ import (
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/errors"
"opendev.org/airship/airshipctl/pkg/events"
k8sapplier "opendev.org/airship/airshipctl/pkg/k8s/applier"
"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
"opendev.org/airship/airshipctl/pkg/k8s/utils"
"opendev.org/airship/airshipctl/pkg/log"
@ -46,8 +47,8 @@ type ExecutorOptions struct {
var _ ifc.Executor = &Executor{}
// RegisterExecutor adds executor to phase executor registry
func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error {
// 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 {
@ -59,7 +60,7 @@ func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory)
// registerExecutor is here so that executor in theory can be used outside phases
func registerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
return NewExecutor(ExecutorOptions{
return NewKubeApplierExecutor(ExecutorOptions{
ClusterName: cfg.ClusterName,
BundleName: cfg.PhaseName,
Helper: cfg.Helper,
@ -79,8 +80,8 @@ type Executor struct {
cleanup kubeconfig.Cleanup
}
// NewExecutor returns instance of executor
func NewExecutor(opts ExecutorOptions) (*Executor, error) {
// NewKubeApplierExecutor returns instance of executor
func NewKubeApplierExecutor(opts ExecutorOptions) (*Executor, error) {
apiObj := &airshipv1.KubernetesApply{}
err := opts.ExecutorDocument.ToAPIObject(apiObj, airshipv1.Scheme)
if err != nil {
@ -116,7 +117,7 @@ func (e *Executor) Run(ch chan events.Event, runOpts ifc.RunOptions) {
}
log.Debugf("WaitTimeout: %v", timeout)
applyOptions := ApplyOptions{
applyOptions := k8sapplier.ApplyOptions{
DryRunStrategy: dryRunStrategy,
Prune: e.apiObject.Config.PruneOptions.Prune,
BundleName: e.Options.BundleName,
@ -125,7 +126,7 @@ func (e *Executor) Run(ch chan events.Event, runOpts ifc.RunOptions) {
applier.ApplyBundle(filteredBundle, applyOptions)
}
func (e *Executor) prepareApplier(ch chan events.Event) (*Applier, document.Bundle, error) {
func (e *Executor) 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)
if err != nil {
@ -146,7 +147,7 @@ func (e *Executor) prepareApplier(ch chan events.Event) (*Applier, document.Bund
e.cleanup = cleanup
log.Debugf("Using kubeconfig at '%s' and context '%s'", path, context)
factory := utils.FactoryFromKubeConfig(path, context)
return NewApplier(ch, factory), bundle, nil
return k8sapplier.NewApplier(ch, factory), bundle, nil
}
// Validate document set

48
pkg/k8s/applier/executor_test.go → pkg/phase/executors/k8s_applier_test.go
View File

@ -12,7 +12,7 @@
limitations under the License.
*/
package applier_test
package executors_test
import (
"bytes"
@ -27,10 +27,10 @@ import (
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/events"
"opendev.org/airship/airshipctl/pkg/fs"
"opendev.org/airship/airshipctl/pkg/k8s/applier"
"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"
)
@ -83,7 +83,7 @@ users:
`
)
func TestNewExecutor(t *testing.T) {
func TestNewKubeApplierExecutor(t *testing.T) {
tests := []struct {
name string
cfgDoc string
@ -96,8 +96,8 @@ func TestNewExecutor(t *testing.T) {
name: "valid executor",
cfgDoc: ValidExecutorDoc,
kubeconf: testKubeconfig(testValidKubeconfig),
helper: makeDefaultHelper(t),
bundleFactory: testBundleFactory("testdata/source_bundle"),
helper: makeKubeApplierDefaultHelper(t),
bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
},
{
name: "wrong config document",
@ -109,8 +109,8 @@ metadata:
labels:
cli-utils.sigs.k8s.io/inventory-id: "some id"`,
expectedErr: "wrong config document",
helper: makeDefaultHelper(t),
bundleFactory: testBundleFactory("testdata/source_bundle"),
helper: makeKubeApplierDefaultHelper(t),
bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
},
{
@ -118,7 +118,7 @@ metadata:
cfgDoc: ValidExecutorDoc,
expectedErr: "no such file or directory",
kubeconf: testKubeconfig(testValidKubeconfig),
helper: makeDefaultHelper(t),
helper: makeKubeApplierDefaultHelper(t),
bundleFactory: testBundleFactory("does not exist"),
},
}
@ -130,8 +130,8 @@ metadata:
require.NoError(t, err)
require.NotNil(t, doc)
exec, err := applier.NewExecutor(
applier.ExecutorOptions{
exec, err := executors.NewKubeApplierExecutor(
executors.ExecutorOptions{
ExecutorDocument: doc,
BundleFactory: tt.bundleFactory,
Kubeconfig: tt.kubeconf,
@ -152,7 +152,7 @@ metadata:
// TODO We need valid test that checks that actual bundle has arrived to applier
// for that we need a way to inject fake applier, which is not doable with `black box` test currently
// since we tests are in different package from executor
func TestExecutorRun(t *testing.T) {
func TestKubeApplierExecutorRun(t *testing.T) {
tests := []struct {
name string
containsErr string
@ -167,8 +167,8 @@ func TestExecutorRun(t *testing.T) {
{
name: "cant read kubeconfig error",
containsErr: "no such file or directory",
helper: makeDefaultHelper(t),
bundleFactory: testBundleFactory("testdata/source_bundle"),
helper: makeKubeApplierDefaultHelper(t),
bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
kubeconf: testKubeconfig(`invalid kubeconfig`),
execDoc: toKubernetesApply(t, ValidExecutorDocNamespaced),
clusterName: "ephemeral-cluster",
@ -181,8 +181,8 @@ func TestExecutorRun(t *testing.T) {
{
name: "error cluster not defined",
containsErr: "cluster is not defined in in cluster map",
helper: makeDefaultHelper(t),
bundleFactory: testBundleFactory("testdata/source_bundle"),
helper: makeKubeApplierDefaultHelper(t),
bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
kubeconf: testKubeconfig(testValidKubeconfig),
execDoc: toKubernetesApply(t, ValidExecutorDocNamespaced),
clusterMap: clustermap.NewClusterMap(v1alpha1.DefaultClusterMap()),
@ -191,8 +191,8 @@ func TestExecutorRun(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
exec, err := applier.NewExecutor(
applier.ExecutorOptions{
exec, err := executors.NewKubeApplierExecutor(
executors.ExecutorOptions{
ExecutorDocument: tt.execDoc,
Helper: tt.helper,
BundleFactory: tt.bundleFactory,
@ -225,8 +225,8 @@ func TestRender(t *testing.T) {
execDoc, err := document.NewDocumentFromBytes([]byte(ValidExecutorDoc))
require.NoError(t, err)
require.NotNil(t, execDoc)
exec, err := applier.NewExecutor(applier.ExecutorOptions{
BundleFactory: testBundleFactory("testdata/source_bundle"),
exec, err := executors.NewKubeApplierExecutor(executors.ExecutorOptions{
BundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"),
ExecutorDocument: execDoc,
})
require.NoError(t, err)
@ -240,7 +240,7 @@ func TestRender(t *testing.T) {
assert.Contains(t, result, "ReplicationController")
}
func makeDefaultHelper(t *testing.T) ifc.Helper {
func makeKubeApplierDefaultHelper(t *testing.T) ifc.Helper {
t.Helper()
conf := &config.Config{
CurrentContext: "default",
@ -252,7 +252,7 @@ func makeDefaultHelper(t *testing.T) ifc.Helper {
Manifests: map[string]*config.Manifest{
"default-manifest": {
MetadataPath: "metadata.yaml",
TargetPath: "testdata",
TargetPath: "../../k8s/applier/testdata",
PhaseRepositoryName: config.DefaultTestPhaseRepo,
Repositories: map[string]*config.Repository{
config.DefaultTestPhaseRepo: {
@ -292,9 +292,3 @@ func testKubeconfig(stringData string) kubeconfig.Interface {
},
))
}
func testBundleFactory(path string) document.BundleFactoryFunc {
return func() (document.Bundle, error) {
return document.NewBundleByPath(path)
}
}