From 5b5142d0cfd3c53fb7597b3a9cf56d6c565174d3 Mon Sep 17 00:00:00 2001 From: Jessica Yuen <im.jessicayuen@gmail.com> Date: Tue, 7 Nov 2017 10:39:13 -0800 Subject: [PATCH] Renaming environment should also rename nested .jsonnet Currently the env directory is being renamed on `env set foo --name=bar`, however the `foo.jsonnet` file also needs to be renamed to `bar.jsonnet`. This commit fixes that. --- metadata/environment.go | 85 ++++++++++++++++++++++++++---------- metadata/environment_test.go | 26 ++++++++--- 2 files changed, 83 insertions(+), 28 deletions(-) diff --git a/metadata/environment.go b/metadata/environment.go index 29eb82af..18b1b019 100644 --- a/metadata/environment.go +++ b/metadata/environment.go @@ -180,23 +180,9 @@ func (m *manager) DeleteEnvironment(name string) error { // Need to ensure empty parent directories are also removed. log.Debug("Removing empty parent directories, if any") - parentDir := name - for parentDir != "." { - parentDir = filepath.Dir(parentDir) - parentPath := string(appendToAbsPath(m.environmentsPath, parentDir)) - - isEmpty, err := afero.IsEmpty(m.appFS, parentPath) - if err != nil { - log.Debugf("Failed to check whether parent directory at path '%s' is empty", parentPath) - return err - } - if isEmpty { - log.Debugf("Failed to remove parent directory at path '%s'", parentPath) - err := m.appFS.RemoveAll(parentPath) - if err != nil { - return err - } - } + err = m.cleanEmptyParentDirs(name) + if err != nil { + return err } log.Infof("Successfully removed environment '%s'", name) @@ -292,16 +278,45 @@ func (m *manager) SetEnvironment(name string, desired *Environment) error { return fmt.Errorf("Can not update '%s' to '%s', it already exists", name, desired.Name) } + // // Move the directory - pathOld := string(appendToAbsPath(m.environmentsPath, name)) - pathNew := string(appendToAbsPath(m.environmentsPath, desired.Name)) - log.Debugf("Moving directory at path '%s' to '%s'", pathOld, pathNew) - err = m.appFS.Rename(pathOld, pathNew) + // + pathOld := appendToAbsPath(m.environmentsPath, name) + pathNew := appendToAbsPath(m.environmentsPath, desired.Name) + jsonnetPathOld := string(appendToAbsPath(pathOld, path.Base(name)+".jsonnet")) + jsonnetPathNew := string(appendToAbsPath(pathOld, path.Base(desired.Name)+".jsonnet")) + exists, err := afero.DirExists(m.appFS, string(pathNew)) + if err != nil { + return err + } + if exists { + return fmt.Errorf("Failed to create environment, directory exists at path: %s", string(pathNew)) + } + // Need to first create subdirectories that don't exist + intermediatePath := path.Dir(string(pathNew)) + log.Debugf("Moving directory at path '%s' to '%s'", string(pathOld), string(pathNew)) + err = m.appFS.MkdirAll(intermediatePath, defaultFolderPermissions) + if err != nil { + return err + } + // note: we have to move the jsonnet file first because of a bug with afero: spf13/afero:#141 + log.Debugf("Renaming jsonnet file from '%s' to '%s'", path.Base(jsonnetPathOld), path.Base(jsonnetPathNew)) + err = m.appFS.Rename(jsonnetPathOld, jsonnetPathNew) + if err != nil { + log.Debugf("Failed to move path '%s' to '%s", jsonnetPathOld, jsonnetPathNew) + return err + } + // finally, move the directory + err = m.appFS.Rename(string(pathOld), string(pathNew)) + if err != nil { + log.Debugf("Failed to move path '%s' to '%s", string(pathOld), string(pathNew)) + return err + } + // clean up any empty parent directory paths + err = m.cleanEmptyParentDirs(name) if err != nil { - log.Debugf("Failed to move path '%s' to '%s", pathOld, pathNew) return err } - name = desired.Name } @@ -403,6 +418,30 @@ func (m *manager) SetEnvironmentParams(env, component string, params param.Param return nil } +func (m *manager) cleanEmptyParentDirs(name string) error { + // clean up any empty parent directory paths + log.Debug("Removing empty parent directories, if any") + parentDir := name + for parentDir != "." { + parentDir = filepath.Dir(parentDir) + parentPath := string(appendToAbsPath(m.environmentsPath, parentDir)) + + isEmpty, err := afero.IsEmpty(m.appFS, parentPath) + if err != nil { + log.Debugf("Failed to check whether parent directory at path '%s' is empty", parentPath) + return err + } + if isEmpty { + log.Debugf("Failed to remove parent directory at path '%s'", parentPath) + err := m.appFS.RemoveAll(parentPath) + if err != nil { + return err + } + } + } + return nil +} + func (m *manager) generateKsonnetLibData(spec ClusterSpec) ([]byte, []byte, []byte, error) { // Get cluster specification data, possibly from the network. text, err := spec.data() diff --git a/metadata/environment_test.go b/metadata/environment_test.go index c2cb99f1..9cdb5425 100644 --- a/metadata/environment_test.go +++ b/metadata/environment_test.go @@ -18,7 +18,7 @@ package metadata import ( "encoding/json" "fmt" - "os" + "path" "reflect" "strings" "testing" @@ -55,18 +55,26 @@ func mockEnvironments(t *testing.T, appName string) *manager { envNames := []string{defaultEnvName, mockEnvName, mockEnvName2, mockEnvName3} for _, env := range envNames { envPath := appendToAbsPath(m.environmentsPath, env) + testFS.Mkdir(string(envPath), defaultFolderPermissions) + testDirExists(t, string(envPath)) + + jsonnetPath := appendToAbsPath(envPath, path.Base(env)+".jsonnet") + jsonnetData := m.generateOverrideData() + err = afero.WriteFile(testFS, string(jsonnetPath), jsonnetData, defaultFilePermissions) + if err != nil { + t.Fatalf("Could not write file at path: %s", jsonnetPath) + } + testFileExists(t, string(jsonnetPath)) specPath := appendToAbsPath(envPath, mockSpecJSON) specData, err := generateSpecData(mockSpecJSONServer, mockNamespace) if err != nil { t.Fatalf("Expected to marshal:\nserver: %s\nnamespace: %s\n, but failed", mockSpecJSONServer, mockNamespace) } - err = afero.WriteFile(testFS, string(specPath), specData, os.ModePerm) + err = afero.WriteFile(testFS, string(specPath), specData, defaultFilePermissions) if err != nil { t.Fatalf("Could not write file at path: %s", specPath) } - - testDirExists(t, string(envPath)) } return m @@ -90,6 +98,15 @@ func testDirNotExists(t *testing.T, path string) { } } +func testFileExists(t *testing.T, path string) { + exists, err := afero.Exists(testFS, path) + if err != nil { + t.Fatalf("Expected file at '%s' to exist, but failed:\n%v", path, err) + } else if !exists { + t.Fatalf("Expected file at '%s' to exist, but it does not", path) + } +} + func TestDeleteEnvironment(t *testing.T) { appName := "test-delete-envs" m := mockEnvironments(t, appName) @@ -166,7 +183,6 @@ func TestSetEnvironment(t *testing.T) { envPath := appendToAbsPath(AbsPath(appName), environmentsDir) expectedPathExists := appendToAbsPath(envPath, set.Name) expectedPathNotExists := appendToAbsPath(envPath, mockEnvName) - testDirExists(t, string(expectedPathExists)) testDirNotExists(t, string(expectedPathNotExists)) -- GitLab