Fix GitClient.commitAndPush() fails if no changes.

E.g. when playground is applied a 2nd time, #idempotence Also extends command executor to also return stderr and exit code. Allow for suppressing failure. This was done because a different implementation for areChangesStagedForCommit() would be: "git update-index --refresh && git diff-index --exit-code HEAD --" which return exit code 1. A requirement such as ignoring failing commands might come back in the future so let's keep it.
cloudogu · Nov 8, 2022 · b86fc65 · b86fc65
1 parent 49fe9bd
commit b86fc65
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 44 deletions.
diff --git a/src/main/groovy/com/cloudogu/gitops/cli/GitopsPlaygroundCli.groovy b/src/main/groovy/com/cloudogu/gitops/cli/GitopsPlaygroundCli.groovy
@@ -161,6 +161,5 @@ class GitopsPlaygroundCli implements Runnable {
                 ]
         ]
     }
-
 }
 
diff --git a/src/main/groovy/com/cloudogu/gitops/cli/GitopsPlaygroundCliMain.groovy b/src/main/groovy/com/cloudogu/gitops/cli/GitopsPlaygroundCliMain.groovy
@@ -16,9 +16,9 @@ class GitopsPlaygroundCliMain {
     @SuppressWarnings('GrMethodMayBeStatic') // Non-static for easier testing
     void exec(String[] args) {
         // log levels can be set via picocli.trace sys env - defaults to 'WARN'
-        if (args.contains("--trace"))
+        if (args.contains('--trace') || args.contains('-x'))
             System.setProperty("picocli.trace", "DEBUG")
-        else if (args.contains("--debug"))
+        else if (args.contains('--debug') || args.contains('-d'))
             System.setProperty("picocli.trace", "INFO")
 
         int exitCode = PicocliRunner.execute(commandClass, args)

diff --git a/src/main/groovy/com/cloudogu/gitops/utils/CommandExecutor.groovy b/src/main/groovy/com/cloudogu/gitops/utils/CommandExecutor.groovy
@@ -5,50 +5,63 @@ import groovy.util.logging.Slf4j
 @Slf4j
 class CommandExecutor {
 
-    String execute(String[] command) {
-        String commandString = command.join(" ")
-        Process proc = command.execute()
-        return getOutput(proc, commandString)
-    }
-
-    String executeAsList(String command) {
-        String[] commandList = command.split(" ")
-        Process proc = commandList.execute()
-        return getOutput(proc, command)
+    Output execute(String[] command, boolean failOnError = true) {
+        Process proc = doExecute(command)
+        return getOutput(proc, command.join(" "), failOnError)
     }
 
-    String execute(String command) {
+    Output execute(String command, boolean failOnError = true) {
         Process proc = doExecute(command)
-        return getOutput(proc, command)
+        return getOutput(proc, command, failOnError)
     }
 
+    Output execute(String command1, String command2, boolean failOnError = true) {
+        Process proc = doExecute(command1) | doExecute(command2)
+        String command = command1 + " | " + command2
+        return getOutput(proc, command, failOnError)
+    }
+
     protected Process doExecute(String command) {
+        log.trace("Executing command: '${command}'")
         command.execute()
     }
-
-    String execute(String command1, String command2) {
-        Process proc = command1.execute() | command2.execute()
-        String command = command1 + " | " + command2
-        return getOutput(proc, command)
+
+    protected Process doExecute(String[] command) {
+        log.trace("Executing command: '${command}'")
+        command.execute()
     }
 
-    protected String getOutput(Process proc, String command) {
+    protected Output getOutput(Process proc, String command, boolean failOnError = true) {
         proc.waitForOrKill(60000)
         // err must be defined first because calling proc.text closes the output stream
-        String err = proc.err.text
-        String out = proc.text
-        if (proc.exitValue() > 0) {
+        String err = proc.err.text.trim()
+        String out = proc.text.trim()
+        if (failOnError && proc.exitValue() > 0) {
             log.error("Executing command failed: ${command}")
             log.error("Stderr: ${err.toString().trim()}")
             log.error("StdOut: ${out.toString().trim()}")
             System.exit(1)
         }
-        if (!out.toString().empty) {
-            log.debug(command + "\n Success: -> " + out.toString().trim() + "\n")
+        if (out) {
+            log.debug("${command}\n Success: ${out}")
         }
-        if (!err.toString().empty) {
-            log.debug(command + "\n Warning / Error -> " + err.toString().trim() + "\n")
+        if (err) {
+            log.debug("${command}\n Warning / Error: ${err}")
+        }
+        return new Output(err, out, proc.exitValue())
+    }
+
+    static class Output {
+        String stdErr
+        String stdOut
+        int exitCode
+
+        Output() {}
+
+        Output(String stdErr, String stdOut, int exitCode) {
+            this.stdErr = stdErr
+            this.stdOut = stdOut
+            this.exitCode = exitCode
         }
-        return out.toString().trim()
     }
 }
diff --git a/src/main/groovy/com/cloudogu/gitops/utils/GitClient.groovy b/src/main/groovy/com/cloudogu/gitops/utils/GitClient.groovy
@@ -51,30 +51,40 @@ class GitClient {
     }
 
     void commitAndPush(String scmmRepoTarget, String absoluteLocalRepoTmpDir) {
-        log.debug("Pushing configured $scmmRepoTarget repo")
+        log.debug("Checking out main, adding files for repo: ${scmmRepoTarget}")
         checkoutOrCreateBranch('main')
         git("add .")
-        String[] commitCommand = ["commit", "-m", "\"Initial commit\"", "--quiet"]
-        git(commitCommand)
-        git("push -u $scmmUrlWithCredentials/repo/$scmmRepoTarget HEAD:main --force")
-
+        // "git commit" fails if no changes
+        if (areChangesStagedForCommit()) {
+            log.debug("Pushing repo: ${scmmRepoTarget}")
+            // Passing this as a single string leads to failing command
+            git(["commit", "-m", "\"Initial commit\""] as String[])
+            git("push -u $scmmUrlWithCredentials/repo/$scmmRepoTarget HEAD:main --force")
+        }
         cleanup(absoluteLocalRepoTmpDir)
 
         setDefaultBranchForRepo(scmmRepoTarget)
     }
 
+    boolean areChangesStagedForCommit() {
+        // See https://stackoverflow.com/a/5139346/
+        boolean changesStageForCommit = !git('status --porcelain').isEmpty()
+        log.debug("Stages changed for commit: ${changesStageForCommit}")
+        return changesStageForCommit
+    }
+
     private void gitRepoCommandInit(String absoluteLocalRepoTmpDir) {
         gitRepoCommand = "git --git-dir=$absoluteLocalRepoTmpDir/.git/ --work-tree=$absoluteLocalRepoTmpDir"
     }
 
     String git(String command) {
         String gitCommand = gitRepoCommand + " " + command
-        commandExecutor.executeAsList(gitCommand)
+        commandExecutor.execute(gitCommand).stdOut
     }
 
     String git(String[] command) {
         String[] gitCommand = gitRepoCommand.split(" ") + command
-        commandExecutor.execute(gitCommand)
+        commandExecutor.execute(gitCommand).stdOut
     }
 
     private void cleanup(String dir) {

diff --git a/src/main/groovy/com/cloudogu/gitops/utils/HelmClient.groovy b/src/main/groovy/com/cloudogu/gitops/utils/HelmClient.groovy
@@ -13,12 +13,12 @@ class HelmClient {
     }
 
     String addRepo(String repoName, String url) {
-        commandExecutor.execute("helm repo add ${repoName} ${url}")
+        commandExecutor.execute("helm repo add ${repoName} ${url}").stdOut
     }
     String upgrade(String release, String chart, String version, Map args) {
         String command =  "helm upgrade -i ${release} ${chart} --version=${version} " +
                 "${args.values? "--values ${args.values} " : ''}" +
                 "${args.namespace? "--namespace ${args.namespace} " : ''}"
-        commandExecutor.execute(command)
+        commandExecutor.execute(command).stdOut
     }
 }
diff --git a/src/main/groovy/com/cloudogu/gitops/utils/K8sClient.groovy b/src/main/groovy/com/cloudogu/gitops/utils/K8sClient.groovy
@@ -16,15 +16,15 @@ class K8sClient {
         String foundNodeIp = "0.0.0.0"
         String node = waitForNode()
         String[] command = ["kubectl", "get", "$node", "--template='{{range .status.addresses}}{{ if eq .type \"InternalIP\" }}{{.address}}{{end}}{{end}}'"]
-        foundNodeIp = commandExecutor.execute(command)
+        foundNodeIp = commandExecutor.execute(command).stdOut
         return foundNodeIp
     }
 
     private String waitForNode() {
-        return commandExecutor.execute("kubectl get node -oname", "head -n1")
+        return commandExecutor.execute("kubectl get node -oname", "head -n1").stdOut
     }
 
-    void applyYaml(String yamlLocation) {
-        commandExecutor.execute("kubectl apply -f $yamlLocation")
+    String applyYaml(String yamlLocation) {
+        commandExecutor.execute("kubectl apply -f $yamlLocation").stdOut
     }
 }
diff --git a/src/main/groovy/com/cloudogu/gitops/utils/NetworkingUtils.groovy b/src/main/groovy/com/cloudogu/gitops/utils/NetworkingUtils.groovy
@@ -29,7 +29,7 @@ class NetworkingUtils {
 
         String potentialClusterBindAddress = k8sClient.getInternalNodeIp()
         potentialClusterBindAddress = potentialClusterBindAddress.replaceAll("'", "")
-        String ipConfig = commandExecutor.execute("ip route get 1")
+        String ipConfig = commandExecutor.execute("ip route get 1").stdOut
         String substringWithSrcIp = ipConfig.substring(ipConfig.indexOf("src"))
         String localAddress = getIpFromString(substringWithSrcIp)
 

diff --git a/src/test/groovy/com/cloudogu/gitops/utils/CommandExecutorForTest.groovy b/src/test/groovy/com/cloudogu/gitops/utils/CommandExecutorForTest.groovy
@@ -5,8 +5,9 @@ class CommandExecutorForTest extends CommandExecutor {
     List<String> actualCommands = []
 
     @Override
-    protected String getOutput(Process proc, String command) {
+    protected Output getOutput(Process proc, String command, boolean failOnError) {
         actualCommands += command
+        return new Output()
     }
 
     @Override