Capture terraform errors from deploy command, use scrolling buffer (#…

…1868) * capture terraform errors and outputs * correct leading whitespace * update changelog * ignore "No state file .." message * revise exit return codes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add tests for coverage * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * check success case in tf version test * update tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * pre-commit fixes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * modify to pass without `terraform` in CI test env * retry tests fix * retry retry fix * remove useless pragma tags * use exist status 1 for error * include crm.up test with 'valid' options * correct exit code * update test * new test for up command with 'valid' options * update to work without terraform install * more changes for CI tests * complete coverage for test_tf_version_error * rename to avoid module vs. function conflict * add comment * add tests for deploy up, down, status * fix typo * fix paths since rename * use importlib for right path of -e install (awsbatch issue) --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
AgnostiqHQ · Nov 30, 2023 · 2e05b04 · 2e05b04
1 parent 6c9ce39
commit 2e05b04
Show file tree

Hide file tree

Showing 9 changed files with 617 additions and 201 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,14 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [UNRELEASED]
 
+### Changed
+
+- Terraform output to use scrolling buffer.
+- Terraform output handling to show errors.
+
 ## [0.231.0-rc.0] - 2023-11-28
 
 ### Authors
 
 - Ara Ghukasyan <[email protected]>
 - Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
 
-
 ### Added
 
 - check for `/bin/bash` AND `/bin/sh` (in that order) to execute bash leptons

diff --git a/covalent/cloud_resource_manager/core.py b/covalent/cloud_resource_manager/core.py
@@ -121,9 +121,9 @@ def get_plugin_settings(
         for key, value in executor_options.items():
             try:
                 settings_dict[key]["value"] = value
-            except:
+            except Exception:
                 logger.error(f"No such option '{key}'. Use --help for available options")
-                sys.exit()
+                sys.exit(1)
 
     return settings_dict
 
@@ -164,10 +164,9 @@ def __init__(
         self._terraform_log_env_vars = {
             "TF_LOG": "ERROR",
             "TF_LOG_PATH": os.path.join(self.executor_tf_path, "terraform-error.log"),
-            "PATH": "$PATH:/usr/bin",
         }
 
-    def _print_stdout(self, process: subprocess.Popen, print_callback: Callable) -> int:
+    def _poll_process(self, process: subprocess.Popen, print_callback: Callable) -> int:
         """
         Print the stdout from the subprocess to console
 
@@ -179,12 +178,10 @@ def _print_stdout(self, process: subprocess.Popen, print_callback: Callable) ->
             Return code of the process.
 
         """
-        while (retcode := process.poll()) is None:
-            if (proc_stdout := process.stdout.readline()) and print_callback:
-                print_callback(proc_stdout.strip().decode("utf-8"))
-        return retcode
-
-        # TODO: Return the command output along with return code
+        while (returncode := process.poll()) is None:
+            if print_callback:
+                print_callback(process.stdout.readline())
+        return returncode
 
     def _parse_terraform_error_log(self) -> List[str]:
         """Parse the terraform error logs.
@@ -235,16 +232,21 @@ def _get_resource_status(
         Returns:
             status: str - status of plugin
         """
-        _, stderr = proc.communicate()
+
         cmds = cmd.split(" ")
         tfstate_path = cmds[-1].split("=")[-1]
-        if stderr is None:
-            return self._terraform_error_validator(tfstate_path=tfstate_path)
-        else:
-            raise subprocess.CalledProcessError(
-                returncode=1, cmd=cmd, stderr=self._parse_terraform_error_log()
+
+        returncode = self._poll_process(proc, print_callback=None)
+        stderr = proc.stderr.read()
+        if returncode != 0 and "No state file was found!" not in stderr:
+            print(
+                "Unable to get resource status due to the following error:\n\n",
+                stderr,
+                file=sys.stderr,
             )
 
+        return self._terraform_error_validator(tfstate_path=tfstate_path)
+
     def _log_error_msg(self, cmd) -> None:
         """
         Log error msg with valid command to terraform-erro.log
@@ -261,7 +263,6 @@ def _log_error_msg(self, cmd) -> None:
     def _run_in_subprocess(
         self,
         cmd: str,
-        workdir: str,
         env_vars: Optional[Dict[str, str]] = None,
         print_callback: Optional[Callable] = None,
     ) -> Union[None, str]:
@@ -270,39 +271,50 @@ def _run_in_subprocess(
 
         Args:
             cmd: Command to execute in the subprocess
-            workdir: Working directory of the subprocess
             env_vars: Dictionary of environment variables to set in the processes execution environment
 
         Returns:
             Union[None, str]
                 - For 'covalent deploy status'
-                    returns status of the deplyment
+                    returns status of the deployment
                 - Others
                     return None
         """
-        if git := shutil.which("git"):
-            proc = subprocess.Popen(
-                args=cmd,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT,
-                cwd=workdir,
-                shell=True,
-                env=env_vars,
-            )
-            TERRAFORM_STATE = "state list -state"
-            if TERRAFORM_STATE in cmd:
-                return self._get_resource_status(proc=proc, cmd=cmd)
-            retcode = self._print_stdout(proc, print_callback)
-
-            if retcode != 0:
-                self._log_error_msg(cmd=cmd)
-                raise subprocess.CalledProcessError(
-                    returncode=retcode, cmd=cmd, stderr=self._parse_terraform_error_log()
-                )
-        else:
+        if not shutil.which("git"):
             self._log_error_msg(cmd=cmd)
             logger.error("Git not found on the system.")
-            sys.exit()
+            sys.exit(1)
+
+        env_vars = env_vars or {}
+        env_vars.update({"PATH": os.environ["PATH"]})
+
+        proc = subprocess.Popen(
+            args=cmd,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            cwd=self.executor_tf_path,
+            universal_newlines=True,
+            shell=True,
+            env=env_vars,
+        )
+
+        if "state list -state" in cmd:
+            return self._get_resource_status(proc=proc, cmd=cmd)
+
+        returncode = self._poll_process(proc, print_callback)
+
+        if returncode != 0:
+            self._log_error_msg(cmd=cmd)
+
+            _, stderr = proc.communicate()
+            raise subprocess.CalledProcessError(
+                returncode=returncode,
+                cmd=cmd,
+                stderr=self._parse_terraform_error_log(),
+                output=stderr,
+            )
+
+        return None
 
     def _update_config(self, tf_executor_config_file: str) -> None:
         """
@@ -348,18 +360,22 @@ def _get_tf_path(self) -> str:
         """
         if terraform := shutil.which("terraform"):
             result = subprocess.run(
-                ["terraform --version"], shell=True, capture_output=True, text=True
+                ["terraform --version"],
+                shell=True,
+                capture_output=True,
+                text=True,
+                check=True,
             )
             version = result.stdout.split("v", 1)[1][:3]
             if float(version) < 1.4:
                 logger.error(
                     "Old version of terraform found. Please update it to version greater than 1.3"
                 )
-                sys.exit()
+                sys.exit(1)
             return terraform
-        else:
-            logger.error("Terraform not found on system")
-            exit()
+
+        logger.error("Terraform not found on system")
+        sys.exit(1)
 
     def _get_tf_statefile_path(self) -> str:
         """
@@ -401,14 +417,16 @@ def up(self, print_callback: Callable, dry_run: bool = True) -> None:
 
         # Run `terraform init`
         self._run_in_subprocess(
-            cmd=tf_init, workdir=self.executor_tf_path, env_vars=self._terraform_log_env_vars
+            cmd=tf_init,
+            env_vars=self._terraform_log_env_vars,
+            print_callback=print_callback,
         )
 
         # Setup terraform infra variables as passed by the user
         tf_vars_env_dict = os.environ.copy()
 
         if self.executor_options:
-            with open(tfvars_file, "w") as f:
+            with open(tfvars_file, "w", encoding="utf-8") as f:
                 for key, value in self.executor_options.items():
                     tf_vars_env_dict[f"TF_VAR_{key}"] = value
 
@@ -418,17 +436,15 @@ def up(self, print_callback: Callable, dry_run: bool = True) -> None:
         # Run `terraform plan`
         self._run_in_subprocess(
             cmd=tf_plan,
-            workdir=self.executor_tf_path,
             env_vars=self._terraform_log_env_vars,
             print_callback=print_callback,
         )
 
         # Create infrastructure as per the plan
         # Run `terraform apply`
         if not dry_run:
-            cmd_output = self._run_in_subprocess(
+            self._run_in_subprocess(
                 cmd=tf_apply,
-                workdir=self.executor_tf_path,
                 env_vars=tf_vars_env_dict.update(self._terraform_log_env_vars),
                 print_callback=print_callback,
             )
@@ -472,7 +488,6 @@ def down(self, print_callback: Callable) -> None:
         # Run `terraform destroy`
         self._run_in_subprocess(
             cmd=tf_destroy,
-            workdir=self.executor_tf_path,
             print_callback=print_callback,
             env_vars=self._terraform_log_env_vars,
         )
@@ -510,6 +525,4 @@ def status(self) -> None:
         tf_state = " ".join([terraform, "state", "list", f"-state={tf_state_file}"])
 
         # Run `terraform state list`
-        return self._run_in_subprocess(
-            cmd=tf_state, workdir=self.executor_tf_path, env_vars=self._terraform_log_env_vars
-        )
+        return self._run_in_subprocess(cmd=tf_state, env_vars=self._terraform_log_env_vars)
diff --git a/covalent_dispatcher/_cli/groups/__init__.py b/covalent_dispatcher/_cli/groups/__init__.py
@@ -13,5 +13,5 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-from .db import db
-from .deploy import deploy
+from .db_group import db
+from .deploy_group import deploy
diff --git a/covalent_dispatcher/_cli/groups/db.py → covalent_dispatcher/_cli/groups/db_group.py b/covalent_dispatcher/_cli/groups/db.py → covalent_dispatcher/_cli/groups/db_group.py