Merge pull request #58 from jessicayuen/env-rename-fix

Renaming environment should also rename nested .jsonnet

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()

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