Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structure Refactoring #55

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
24 changes: 12 additions & 12 deletions cmd/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/simontheleg/konf-go/config"
"github.com/simontheleg/konf-go/konf"
log "github.com/simontheleg/konf-go/log"
"github.com/simontheleg/konf-go/store"
"github.com/spf13/afero"
"github.com/spf13/cobra"
)
Expand All @@ -23,12 +24,14 @@ An active config is considered unused when no process points to it anymore`,
RunE: func(cmd *cobra.Command, args []string) error {

fs := afero.NewOsFs()
err := cleanLeftOvers(fs)
sm := &store.Storemanager{Activedir: config.ActiveDir(), Storedir: config.StoreDir(), Fs: fs}

err := cleanLeftOvers(sm)
if err != nil {
return err
}

err = selfClean(fs)
err = selfClean(sm)
if err != nil {
return err
}
Expand All @@ -41,11 +44,12 @@ An active config is considered unused when no process points to it anymore`,
// it is required as the idempotent clean would delete all files that
// do not belong to any process anymore, but of course the current process
// is still running at this time
func selfClean(f afero.Fs) error {
func selfClean(sm *store.Storemanager) error {
pid := os.Getppid()

fpath := konf.IDFromProcessID(pid).ActivePath()
err := f.Remove(fpath)
konfID := konf.IDFromProcessID(pid)
fpath := sm.ActivePathFromID(konfID)
err := sm.Fs.Remove(fpath)

if errors.Is(err, fs.ErrNotExist) {
log.Info("current konf '%s' was already deleted, nothing to self-cleanup\n", fpath)
Expand All @@ -64,8 +68,8 @@ func selfClean(f afero.Fs) error {
// any leftovers that can occur if a previous session was not cleaned up nicely. This is
// necessary as we cannot tell a user that a selfClean has failed if they close the shell
// session before
func cleanLeftOvers(f afero.Fs) error {
konfs, err := afero.ReadDir(f, config.ActiveDir())
func cleanLeftOvers(sm *store.Storemanager) error {
konfs, err := afero.ReadDir(sm.Fs, sm.Activedir)

if err != nil {
return err
Expand All @@ -86,7 +90,7 @@ func cleanLeftOvers(f afero.Fs) error {
}

if p == nil {
err := f.Remove(konfID.ActivePath())
err := sm.Fs.Remove(sm.ActivePathFromID(konfID))
if err != nil {
return err
}
Expand All @@ -95,7 +99,3 @@ func cleanLeftOvers(f afero.Fs) error {

return nil
}

func init() {
rootCmd.AddCommand(cleanupCmd)
}
83 changes: 44 additions & 39 deletions cmd/cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ import (
"syscall"
"testing"

"github.com/simontheleg/konf-go/config"
"github.com/simontheleg/konf-go/konf"
"github.com/simontheleg/konf-go/store"
"github.com/simontheleg/konf-go/testhelper"
"github.com/simontheleg/konf-go/utils"
"github.com/spf13/afero"
)

func TestSelfClean(t *testing.T) {
activeDir := config.ActiveDir()
activeDir := "./konf/active"
storeDir := "./konf/store"

ppid := os.Getppid()

Expand All @@ -28,26 +29,28 @@ func TestSelfClean(t *testing.T) {
NotExpFiles []string
}{
"PID FS": {
ppidFS(),
ppidFS(activeDir),
nil,
[]string{activeDir + "/abc", konf.KonfID("1234").ActivePath()},
[]string{activeDir + "/abc", activeDir + "/1234"},
[]string{activeDir + "/" + fmt.Sprint(ppid) + ".yaml"},
},
"PID file deleted by external source": {
ppidFileMissing(),
ppidFileMissing(activeDir),
nil,
[]string{activeDir + "/abc", konf.KonfID("1234").ActivePath()},
[]string{activeDir + "/abc", activeDir + "/1234"},
[]string{},
},
// Unfortunately it was not possible with afero to test what happens if
// someone changes the active dir permissions an we cannot delete it anymore
// apparently in the memFS afero can just delete these files
// Unfortunately it was not possible with afero memFS to test what happens if
// someone changes the active dir permissions and we cannot delete it anymore.
// Apparently in the memFS afero can just delete these files, regardless of
// permissions :D
}

for name, tc := range tt {
t.Run(name, func(t *testing.T) {

err := selfClean(tc.Fs)
sm := &store.Storemanager{Fs: tc.Fs, Activedir: activeDir, Storedir: storeDir}
err := selfClean(sm)

if !testhelper.EqualError(err, tc.ExpError) {
t.Errorf("Want error '%s', got '%s'", tc.ExpError, err)
Expand All @@ -74,26 +77,26 @@ func TestSelfClean(t *testing.T) {
}
}

func ppidFS() afero.Fs {
func ppidFS(activeDir string) afero.Fs {
ppid := os.Getppid()
fs := ppidFileMissing()
fs := ppidFileMissing(activeDir)
sm := testhelper.SampleKonfManager{}
afero.WriteFile(fs, konf.IDFromProcessID(ppid).ActivePath(), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
afero.WriteFile(fs, activeDir+"/"+fmt.Sprint(ppid), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
return fs
}

func ppidFileMissing() afero.Fs {
func ppidFileMissing(activeDir string) afero.Fs {
fs := afero.NewMemMapFs()
sm := testhelper.SampleKonfManager{}
afero.WriteFile(fs, config.ActiveDir()+"/abc", []byte("I am not even a kubeconfig, what am I doing here?"), utils.KonfPerm)
afero.WriteFile(fs, konf.KonfID("1234").ActivePath(), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
afero.WriteFile(fs, activeDir+"/abc", []byte("I am not even a kubeconfig, what am I doing here?"), utils.KonfPerm)
afero.WriteFile(fs, activeDir+"/1234", []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
return fs
}

func TestCleanLeftOvers(t *testing.T) {

tt := map[string]struct {
Setup func(t *testing.T) (afero.Fs, []*exec.Cmd, []*exec.Cmd)
Setup func(t *testing.T, sm *store.Storemanager) ([]*exec.Cmd, []*exec.Cmd)
ExpErr error
}{
"all procs still running": {
Expand All @@ -116,21 +119,23 @@ func TestCleanLeftOvers(t *testing.T) {

for name, tc := range tt {
t.Run(name, func(t *testing.T) {
f, cmdsRunning, cmdsStopped := tc.Setup(t)
sm := &store.Storemanager{Fs: afero.NewMemMapFs(), Activedir: "./konf/active", Storedir: "./konf/store"}
cmdsRunning, cmdsStopped := tc.Setup(t, sm)

t.Cleanup(func() {
cleanUpRunningCmds(t, cmdsRunning)
})

err := cleanLeftOvers(f)
err := cleanLeftOvers(sm)

if !errors.Is(err, tc.ExpErr) {
t.Errorf("Want error '%s', got '%s'", tc.ExpErr, err)
}

for _, cmd := range cmdsRunning {
fpath := konf.IDFromProcessID(cmd.Process.Pid).ActivePath()
_, err := f.Stat(fpath)
id := konf.IDFromProcessID(cmd.Process.Pid)
fpath := sm.ActivePathFromID(id)
_, err := sm.Fs.Stat(fpath)

if err != nil {
if errors.Is(err, fs.ErrNotExist) {
Expand All @@ -142,8 +147,9 @@ func TestCleanLeftOvers(t *testing.T) {
}

for _, cmd := range cmdsStopped {
fpath := konf.IDFromProcessID(cmd.Process.Pid).ActivePath()
_, err := f.Stat(fpath)
id := konf.IDFromProcessID(cmd.Process.Pid)
fpath := sm.ActivePathFromID(id)
_, err := sm.Fs.Stat(fpath)

if !errors.Is(err, fs.ErrNotExist) {
t.Fatalf("Unexpected error occurred: '%s'", err)
Expand All @@ -159,12 +165,11 @@ func TestCleanLeftOvers(t *testing.T) {

}

func mixedFSWithAllProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
func mixedFSWithAllProcs(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
// we are simulating other instances of konf here
numOfConfs := 3

fs = afero.NewMemMapFs()
sm := testhelper.SampleKonfManager{}
skm := testhelper.SampleKonfManager{}

for i := 1; i <= numOfConfs; i++ {
// set sleep to an extremely high number as the argument "infinity" does not exist in all versions of the util
Expand All @@ -176,15 +181,15 @@ func mixedFSWithAllProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cm
}
pid := cmd.Process.Pid
cmdsRunning = append(cmdsRunning, cmd)
afero.WriteFile(fs, konf.IDFromProcessID(pid).ActivePath(), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
afero.WriteFile(sm.Fs, sm.ActivePathFromID(konf.IDFromProcessID(pid)), []byte(skm.SingleClusterSingleContextEU()), utils.KonfPerm)
}

return fs, cmdsRunning, nil
return cmdsRunning, nil
}

// returns a state where there are more fs files than cmds
func mixedFSIncompleteProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
fs, cmdsRunning, cmdsStopped = mixedFSWithAllProcs(t)
func mixedFSIncompleteProcs(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
cmdsRunning, cmdsStopped = mixedFSWithAllProcs(t, sm)

cmdToKill := cmdsRunning[0]
origPID := cmdToKill.Process.Pid
Expand All @@ -210,22 +215,22 @@ func mixedFSIncompleteProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd,
cmdsStopped = append(cmdsStopped, cmdsRunning[0])
cmdsRunning = cmdsRunning[1:]

return fs, cmdsRunning, cmdsStopped
return cmdsRunning, cmdsStopped
}

func mixedFSDirtyDir(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
fs, cmdsRunning, cmdsStopped = mixedFSIncompleteProcs(t)
func mixedFSDirtyDir(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
cmdsRunning, cmdsStopped = mixedFSIncompleteProcs(t, sm)

afero.WriteFile(fs, konf.KonfID("/not-a-valid-process-id").ActivePath(), []byte{}, utils.KonfPerm)
id := konf.KonfID("/not-a-valid-process-id")
afero.WriteFile(sm.Fs, sm.ActivePathFromID(id), []byte{}, utils.KonfPerm)

return fs, cmdsRunning, cmdsStopped
return cmdsRunning, cmdsStopped

}

func emptyFS(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
fs = afero.NewMemMapFs()

return fs, nil, nil
func emptyFS(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
// no op
return nil, nil
}

func cleanUpRunningCmds(t *testing.T, cmds []*exec.Cmd) {
Expand Down
4 changes: 0 additions & 4 deletions cmd/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,3 @@ func (c *completionCmd) completion(cmd *cobra.Command, args []string) error {

return nil
}

func init() {
rootCmd.AddCommand(newCompletionCmd().cmd)
}
41 changes: 20 additions & 21 deletions cmd/delete.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"github.com/simontheleg/konf-go/config"
"github.com/simontheleg/konf-go/konf"
"github.com/simontheleg/konf-go/log"
"github.com/simontheleg/konf-go/prompt"
Expand All @@ -10,21 +11,22 @@ import (
)

type deleteCmd struct {
fs afero.Fs

fetchconfs func(afero.Fs) ([]*store.Metadata, error)
selectSingleKonf func(afero.Fs, prompt.RunFunc) (konf.KonfID, error)
deleteKonfWithID func(afero.Fs, konf.KonfID) error
idsForGlobs func(afero.Fs, []string) ([]konf.KonfID, error)
sm *store.Storemanager
fetchconfs func() ([]*store.Metadata, error)
selectSingleKonf func(*store.Storemanager, prompt.RunFunc) (konf.KonfID, error)
deleteKonfWithID func(*store.Storemanager, konf.KonfID) error
idsForGlobs func(*store.Storemanager, []string) ([]konf.KonfID, error)
prompt prompt.RunFunc

cmd *cobra.Command
}

func newDeleteCommand() *deleteCmd {
fs := afero.NewOsFs()
sm := &store.Storemanager{Fs: fs, Activedir: config.ActiveDir(), Storedir: config.StoreDir()}
dc := &deleteCmd{
fs: afero.NewOsFs(),
fetchconfs: store.FetchAllKonfs,
sm: sm,
fetchconfs: sm.FetchAllKonfs,
selectSingleKonf: selectSingleKonf,
deleteKonfWithID: deleteKonfWithID,
idsForGlobs: idsForGlobs,
Expand Down Expand Up @@ -54,20 +56,20 @@ func (c *deleteCmd) delete(cmd *cobra.Command, args []string) error {

if len(args) == 0 {
var id konf.KonfID
id, err = c.selectSingleKonf(c.fs, c.prompt)
id, err = c.selectSingleKonf(c.sm, c.prompt)
if err != nil {
return err
}
ids = append(ids, id)
} else {
ids, err = c.idsForGlobs(c.fs, args)
ids, err = c.idsForGlobs(c.sm, args)
if err != nil {
return err
}
}

for _, id := range ids {
if err := c.deleteKonfWithID(c.fs, id); err != nil {
if err := c.deleteKonfWithID(c.sm, id); err != nil {
return err
}
}
Expand All @@ -76,20 +78,21 @@ func (c *deleteCmd) delete(cmd *cobra.Command, args []string) error {
return nil
}

func deleteKonfWithID(fs afero.Fs, id konf.KonfID) error {
if err := fs.Remove(id.StorePath()); err != nil {
func deleteKonfWithID(sm *store.Storemanager, id konf.KonfID) error {
path := sm.StorePathFromID(id)
if err := sm.Fs.Remove(path); err != nil {
return err
}
log.Info("Successfully deleted konf %q at %q", id, id.StorePath())
log.Info("Successfully deleted konf %q at %q", id, path)
return nil
}

// idsForGlobs takes in a slice of patterns and returns corresponding IDs from
// the konfStore
func idsForGlobs(f afero.Fs, patterns []string) ([]konf.KonfID, error) {
func idsForGlobs(sm *store.Storemanager, patterns []string) ([]konf.KonfID, error) {
var ids []konf.KonfID
for _, pattern := range patterns {
metadata, err := store.FetchKonfsForGlob(f, pattern) // resolve any globs among the arguments
metadata, err := sm.FetchKonfsForGlob(pattern) // resolve any globs among the arguments
if err != nil {
return nil, err
}
Expand All @@ -102,7 +105,7 @@ func idsForGlobs(f afero.Fs, patterns []string) ([]konf.KonfID, error) {
}

func (c *deleteCmd) completeDelete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
konfs, err := store.FetchAllKonfs(c.fs)
konfs, err := c.fetchconfs()
if err != nil {
// if the store is just empty, return no suggestions, instead of throwing an error
if _, ok := err.(*store.EmptyStore); ok {
Expand All @@ -122,7 +125,3 @@ func (c *deleteCmd) completeDelete(cmd *cobra.Command, args []string, toComplete

return sug, cobra.ShellCompDirectiveNoFileComp
}

func init() {
rootCmd.AddCommand(newDeleteCommand().cmd)
}
Loading
Loading