ssvm: wrong SSVM behavior causes redownloading for all the templates (#3844)

As per discussion at #3838 and proposal by @weizhouapache this PR implements the fix.

Fixes #3838

server/src/main/java/org/apache/cloudstack/storage/NfsMountManagerImpl.java

@@ -163,16 +163,16 @@ public class NfsMountManagerImpl implements NfsMountManager {
 
     private boolean mountExists(String localRootPath) {
         Script script = new Script(true, "mount", timeout, s_logger);
-        ZfsPathParser parser = new ZfsPathParser(localRootPath);
+        PathParser parser = new PathParser(localRootPath);
         script.execute(parser);
         return parser.getPaths().stream().filter(s -> s.contains(localRootPath)).findAny().map(s -> true).orElse(false);
     }
 
-    public static class ZfsPathParser extends OutputInterpreter {
+    public static class PathParser extends OutputInterpreter {
         String _parent;
         List<String> paths = new ArrayList<>();
 
-        public ZfsPathParser(String parent) {
+        public PathParser(String parent) {
             _parent = parent;
         }
 

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -67,7 +67,7 @@ import org.apache.cloudstack.storage.configdrive.ConfigDrive;
 import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder;
 import org.apache.cloudstack.storage.template.DownloadManager;
 import org.apache.cloudstack.storage.template.DownloadManagerImpl;
-import org.apache.cloudstack.storage.template.DownloadManagerImpl.ZfsPathParser;
+import org.apache.cloudstack.storage.NfsMountManagerImpl.PathParser;
 import org.apache.cloudstack.storage.template.UploadEntity;
 import org.apache.cloudstack.storage.template.UploadManager;
 import org.apache.cloudstack.storage.template.UploadManagerImpl;
@@ -2905,7 +2905,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         script = new Script(!_inSystemVM, "mount", _timeout, s_logger);
 
         List<String> res = new ArrayList<String>();
-        ZfsPathParser parser = new ZfsPathParser(localRootPath);
+        PathParser parser = new PathParser(localRootPath);
         script.execute(parser);
         res.addAll(parser.getPaths());
         for (String s : res) {

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java

@@ -16,7 +16,6 @@
 // under the License.
 package org.apache.cloudstack.storage.template;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -60,6 +59,7 @@ import org.apache.cloudstack.storage.command.DownloadCommand;
 import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
 import org.apache.cloudstack.storage.command.DownloadProgressCommand;
 import org.apache.cloudstack.storage.command.DownloadProgressCommand.RequestType;
+import org.apache.cloudstack.storage.NfsMountManagerImpl.PathParser;
 import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource;
 import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
 import org.apache.commons.collections.CollectionUtils;
@@ -82,7 +82,6 @@ import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.StringUtils;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.OutputInterpreter;
 import com.cloud.utils.script.Script;
 import com.cloud.utils.storage.QCOW2Utils;
 import org.apache.cloudstack.utils.security.ChecksumValue;
@@ -850,8 +849,12 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
 
         Script script = new Script(listVolScr, LOGGER);
         script.add("-r", rootdir);
-        ZfsPathParser zpp = new ZfsPathParser(rootdir);
+        PathParser zpp = new PathParser(rootdir);
         script.execute(zpp);
+        if (script.getExitValue() != 0) {
+            LOGGER.error("Error while executing script " + script.toString());
+            throw new CloudRuntimeException("Error while executing script " + script.toString());
+        }
         result.addAll(zpp.getPaths());
         LOGGER.info("found " + zpp.getPaths().size() + " volumes" + zpp.getPaths());
         return result;
@@ -862,8 +865,12 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
 
         Script script = new Script(listTmpltScr, LOGGER);
         script.add("-r", rootdir);
-        ZfsPathParser zpp = new ZfsPathParser(rootdir);
+        PathParser zpp = new PathParser(rootdir);
         script.execute(zpp);
+        if (script.getExitValue() != 0) {
+            LOGGER.error("Error while executing script " + script.toString());
+            throw new CloudRuntimeException("Error while executing script " + script.toString());
+        }
         result.addAll(zpp.getPaths());
         LOGGER.info("found " + zpp.getPaths().size() + " templates" + zpp.getPaths());
         return result;
@@ -961,33 +968,6 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
         return result;
     }
 
-    public static class ZfsPathParser extends OutputInterpreter {
-        String _parent;
-        List<String> paths = new ArrayList<String>();
-
-        public ZfsPathParser(String parent) {
-            _parent = parent;
-        }
-
-        @Override
-        public String interpret(BufferedReader reader) throws IOException {
-            String line = null;
-            while ((line = reader.readLine()) != null) {
-                paths.add(line);
-            }
-            return null;
-        }
-
-        public List<String> getPaths() {
-            return paths;
-        }
-
-        @Override
-        public boolean drain() {
-            return true;
-        }
-    }
-
     public DownloadManagerImpl() {
     }