airship/airshipctl
Code Issues Proposed changes

Move document filesystem to a separate package

Browse Source 
In order to widely use filesystem interface in airshipctl
there is need to move it to separate package to avoid
importing unnecessary dependencies from document
package and, as a consequence, possible cyclic dependencies.
 * filesystem moved from document/fs to pkg/fs

Change-Id: I3b6298462f03db43594a9fa26bf23ab7687c5589
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>
Relates-To: #415
This commit is contained in:
Ruslan Aliev
2020-11-16 19:24:06 -06:00
parent c18db07043
commit ca71de3951
11 changed files with 87 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -26,12 +26,13 @@ import (
"opendev.org/airship/airshipctl/pkg/config"
"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/ifc"
"opendev.org/airship/airshipctl/testutil/fs"
testfs "opendev.org/airship/airshipctl/testutil/fs"
)
const (
@@ -279,9 +280,9 @@ func testKubeconfig(stringData string) kubeconfig.Interface {
return kubeconfig.NewKubeConfig(
kubeconfig.FromByte([]byte(stringData)),
kubeconfig.InjectFileSystem(
fs.MockFileSystem{
MockTempFile: func(root, pattern string) (document.File, error) {
return fs.TestFile{
testfs.MockFileSystem{
MockTempFile: func(root, pattern string) (fs.File, error) {
return testfs.TestFile{
MockName: func() string { return "kubeconfig-142398" },
MockWrite: func() (int, error) { return 0, nil },
MockClose: func() error { return nil },

10
pkg/k8s/kubeconfig/builder.go
View File

@@ -19,8 +19,8 @@ import (
"opendev.org/airship/airshipctl/pkg/cluster/clustermap"
"opendev.org/airship/airshipctl/pkg/config"
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/errors"
"opendev.org/airship/airshipctl/pkg/fs"
"opendev.org/airship/airshipctl/pkg/util"
)
@@ -77,8 +77,8 @@ func (b *Builder) WithTempRoot(root string) *Builder {
func (b *Builder) Build() Interface {
switch {
case b.path != "":
fs := document.NewDocumentFs()
return NewKubeConfig(FromFile(b.path, fs), InjectFilePath(b.path, fs), InjectTempRoot(b.root))
fSys := fs.NewDocumentFs()
return NewKubeConfig(FromFile(b.path, fSys), InjectFilePath(b.path, fSys), InjectTempRoot(b.root))
case b.fromParent():
// TODO add method that would get kubeconfig from parent cluster and glue it together
// with parent kubeconfig if needed
@@ -88,10 +88,10 @@ func (b *Builder) Build() Interface {
case b.bundlePath != "":
return NewKubeConfig(FromBundle(b.bundlePath), InjectTempRoot(b.root))
default:
fs := document.NewDocumentFs()
fSys := fs.NewDocumentFs()
// return default path to kubeconfig file in airship workdir
path := filepath.Join(util.UserHomeDir(), config.AirshipConfigDir, KubeconfigDefaultFileName)
return NewKubeConfig(FromFile(path, fs), InjectFilePath(path, fs), InjectTempRoot(b.root))
return NewKubeConfig(FromFile(path, fSys), InjectFilePath(path, fSys), InjectTempRoot(b.root))
}
}

21
pkg/k8s/kubeconfig/kubeconfig.go
View File

@@ -22,6 +22,7 @@ import (
"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/fs"
)
// Interface provides a uniform way to interact with kubeconfig file
@@ -44,7 +45,7 @@ type kubeConfig struct {
path string
dumpRoot string
fileSystem document.FileSystem
fileSystem fs.FileSystem
sourceFunc KubeSourceFunc
}
@@ -60,7 +61,7 @@ func NewKubeConfig(source KubeSourceFunc, options ...Option) Interface {
}
kf.sourceFunc = source
if kf.fileSystem == nil {
kf.fileSystem = document.NewDocumentFs()
kf.fileSystem = fs.NewDocumentFs()
}
return kf
}
@@ -96,9 +97,9 @@ func FromSecret(kubeOpts *FromClusterOptions) KubeSourceFunc {
}
// FromFile returns KubeSource type, uses path to kubeconfig on FS as source to construct kubeconfig object
func FromFile(path string, fs document.FileSystem) KubeSourceFunc {
func FromFile(path string, fSys fs.FileSystem) KubeSourceFunc {
return func() ([]byte, error) {
return fs.ReadFile(path)
return fSys.ReadFile(path)
}
}
@@ -130,9 +131,9 @@ func FromBundle(root string) KubeSourceFunc {
}
// InjectFileSystem sets fileSystem to be used, mostly to be used for tests
func InjectFileSystem(fs document.FileSystem) Option {
func InjectFileSystem(fSys fs.FileSystem) Option {
return func(k *kubeConfig) {
k.fileSystem = fs
k.fileSystem = fSys
}
}
@@ -146,10 +147,10 @@ func InjectTempRoot(dumpRoot string) Option {
// InjectFilePath enables setting kubeconfig path, useful when you have kubeconfig
// from the actual filesystem, if this option is used, please also make sure that
// FromFile option is also used as a first argument in NewKubeConfig function
func InjectFilePath(path string, fs document.FileSystem) Option {
func InjectFilePath(path string, fSys fs.FileSystem) Option {
return func(k *kubeConfig) {
k.path = path
k.fileSystem = fs
k.fileSystem = fSys
}
}
@@ -202,12 +203,12 @@ func (k *kubeConfig) GetFile() (string, Cleanup, error) {
return k.WriteTempFile(k.dumpRoot)
}
func cleanup(path string, fs document.FileSystem) Cleanup {
func cleanup(path string, fSys fs.FileSystem) Cleanup {
if path == "" {
return func() {}
}
return func() {
if err := fs.RemoveAll(path); err != nil {
if err := fSys.RemoveAll(path); err != nil {
log.Fatalf("Failed to cleanup kubeconfig file %s, error: %v", path, err)
}
}

36
pkg/k8s/kubeconfig/kubeconfig_test.go
View File

@@ -29,10 +29,10 @@ import (
kustfs "sigs.k8s.io/kustomize/api/filesys"
"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/fs"
"opendev.org/airship/airshipctl/pkg/k8s/client/fake"
"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
"opendev.org/airship/airshipctl/testutil/fs"
testfs "opendev.org/airship/airshipctl/testutil/fs"
)
const (
@@ -123,15 +123,15 @@ var (
func TestKubeconfigContent(t *testing.T) {
expectedData := []byte(testValidKubeconfig)
fs := document.NewDocumentFs()
fSys := fs.NewDocumentFs()
kubeconf := kubeconfig.NewKubeConfig(
kubeconfig.FromByte(expectedData),
kubeconfig.InjectFileSystem(fs),
kubeconfig.InjectFileSystem(fSys),
kubeconfig.InjectTempRoot("."))
path, clean, err := kubeconf.GetFile()
require.NoError(t, err)
defer clean()
actualData, err := fs.ReadFile(path)
actualData, err := fSys.ReadFile(path)
require.NoError(t, err)
assert.Equal(t, expectedData, actualData)
}
@@ -357,9 +357,9 @@ func TestNewKubeConfig(t *testing.T) {
src: kubeconfig.FromByte([]byte(testValidKubeconfig)),
options: []kubeconfig.Option{
kubeconfig.InjectFileSystem(
fs.MockFileSystem{
MockTempFile: func(root, pattern string) (document.File, error) {
return fs.TestFile{
testfs.MockFileSystem{
MockTempFile: func(root, pattern string) (fs.File, error) {
return testfs.TestFile{
MockName: func() string { return "kubeconfig-142398" },
MockWrite: func() (int, error) { return 0, nil },
MockClose: func() error { return nil },
@@ -378,13 +378,13 @@ func TestNewKubeConfig(t *testing.T) {
options: []kubeconfig.Option{
kubeconfig.InjectTempRoot("/my-unique-root"),
kubeconfig.InjectFileSystem(
fs.MockFileSystem{
MockTempFile: func(root, _ string) (document.File, error) {
testfs.MockFileSystem{
MockTempFile: func(root, _ string) (fs.File, error) {
// check if root path is passed to the TempFile interface
if root != "/my-unique-root" {
return nil, errTempFile
}
return fs.TestFile{
return testfs.TestFile{
MockName: func() string { return "kubeconfig-142398" },
MockWrite: func() (int, error) { return 0, nil },
MockClose: func() error { return nil },
@@ -419,8 +419,8 @@ func TestNewKubeConfig(t *testing.T) {
expectedErrorContains: errTempFile.Error(),
options: []kubeconfig.Option{
kubeconfig.InjectFileSystem(
fs.MockFileSystem{
MockTempFile: func(string, string) (document.File, error) {
testfs.MockFileSystem{
MockTempFile: func(string, string) (fs.File, error) {
return nil, errTempFile
},
MockRemoveAll: func() error { return nil },
@@ -505,7 +505,7 @@ func TestKubeConfigWriteFile(t *testing.T) {
path string
expectedErrorContains string
fs document.FileSystem
fs fs.FileSystem
src kubeconfig.KubeSourceFunc
}{
{
@@ -537,8 +537,8 @@ func TestKubeConfigWriteFile(t *testing.T) {
}
}
func readFile(t *testing.T, path string, fs document.FileSystem) string {
b, err := fs.ReadFile(path)
func readFile(t *testing.T, path string, fSys fs.FileSystem) string {
b, err := fSys.ReadFile(path)
require.NoError(t, err)
return string(b)
}
@@ -549,8 +549,8 @@ func read(t *testing.T, r io.Reader) string {
return string(b)
}
func fsWithFile(t *testing.T, path string) document.FileSystem {
fSys := fs.MockFileSystem{
func fsWithFile(t *testing.T, path string) fs.FileSystem {
fSys := testfs.MockFileSystem{
FileSystem: kustfs.MakeFsInMemory(),
MockRemoveAll: func() error {
return nil

7
pkg/k8s/kubectl/kubectl.go
View File

@@ -22,6 +22,7 @@ import (
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/fs"
"opendev.org/airship/airshipctl/pkg/log"
utilyaml "opendev.org/airship/airshipctl/pkg/util/yaml"
)
@@ -31,7 +32,7 @@ import (
type Kubectl struct {
cmdutil.Factory
genericclioptions.IOStreams
document.FileSystem
fs.FileSystem
// Directory to buffer documents before passing them to kubectl commands
// default is empty, this means that /tmp dir will be used
bufferDir string
@@ -47,7 +48,7 @@ func NewKubectl(f cmdutil.Factory) *Kubectl {
Out: os.Stdout,
ErrOut: os.Stderr,
},
FileSystem: document.NewDocumentFs(),
FileSystem: fs.NewDocumentFs(),
}
}
@@ -77,7 +78,7 @@ func (kubectl *Kubectl) ApplyYaml(yaml []byte, ao *ApplyOptions) error {
return err
}
defer func(f document.File) {
defer func(f fs.File) {
fName := f.Name()
dErr := kubectl.RemoveAll(fName)
if dErr != nil {

21
pkg/k8s/kubectl/kubectl_test.go
View File

@@ -23,10 +23,11 @@ import (
corev1 "k8s.io/api/core/v1"
"opendev.org/airship/airshipctl/pkg/document"
"opendev.org/airship/airshipctl/pkg/fs"
"opendev.org/airship/airshipctl/pkg/k8s/kubectl"
k8sutils "opendev.org/airship/airshipctl/pkg/k8s/utils"
"opendev.org/airship/airshipctl/testutil"
"opendev.org/airship/airshipctl/testutil/fs"
testfs "opendev.org/airship/airshipctl/testutil/fs"
k8stest "opendev.org/airship/airshipctl/testutil/k8sutils"
)
@@ -73,14 +74,14 @@ func TestApply(t *testing.T) {
tests := []struct {
name string
expectedErr error
fs document.FileSystem
fs fs.FileSystem
}{
{
expectedErr: nil,
fs: fs.MockFileSystem{
fs: testfs.MockFileSystem{
MockRemoveAll: func() error { return nil },
MockTempFile: func(string, string) (document.File, error) {
return fs.TestFile{
MockTempFile: func(string, string) (fs.File, error) {
return testfs.TestFile{
MockName: func() string { return filenameRC },
MockWrite: func() (int, error) { return 0, nil },
MockClose: func() error { return nil },
@@ -90,15 +91,15 @@ func TestApply(t *testing.T) {
},
{
expectedErr: ErrWriteOutError,
fs: fs.MockFileSystem{
MockTempFile: func(string, string) (document.File, error) { return nil, ErrWriteOutError }},
fs: testfs.MockFileSystem{
MockTempFile: func(string, string) (fs.File, error) { return nil, ErrWriteOutError }},
},
{
expectedErr: ErrTempFileError,
fs: fs.MockFileSystem{
fs: testfs.MockFileSystem{
MockRemoveAll: func() error { return nil },
MockTempFile: func(string, string) (document.File, error) {
return fs.TestFile{
MockTempFile: func(string, string) (fs.File, error) {
return testfs.TestFile{
MockWrite: func() (int, error) { return 0, ErrTempFileError },
MockName: func() string { return filenameRC },
MockClose: func() error { return nil },