From 3276e63a85be9bff9fbb2d37fc95bed7486cdd1c Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Wed, 8 May 2024 17:33:29 -0300 Subject: [PATCH 1/5] enable log export --- src/_nebari/provider/terraform.py | 16 +++++++-- src/_nebari/utils.py | 55 ++++++++++++++++++------------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/_nebari/provider/terraform.py b/src/_nebari/provider/terraform.py index 6f6ad6930b..b1863ba0d2 100644 --- a/src/_nebari/provider/terraform.py +++ b/src/_nebari/provider/terraform.py @@ -148,7 +148,13 @@ def apply(directory=None, targets=None, var_files=None): + ["-var-file=" + _ for _ in var_files] ) with timer(logger, "terraform apply"): - run_terraform_subprocess(command, cwd=directory, prefix="terraform") + _stage = directory.split("/")[-1] + run_terraform_subprocess( + command, + cwd=directory, + prefix="terraform", + output_file=f"terraform_apply_{_stage}.log", + ) def output(directory=None): @@ -208,7 +214,13 @@ def destroy(directory=None, targets=None, var_files=None): ) with timer(logger, "terraform destroy"): - run_terraform_subprocess(command, cwd=directory, prefix="terraform") + _stage = directory.split("/")[-1] + run_terraform_subprocess( + command, + cwd=directory, + prefix="terraform", + output_file=f"terraform_destroy_{_stage}.log", + ) def rm_local_state(directory=None): diff --git a/src/_nebari/utils.py b/src/_nebari/utils.py index 3ae4ad4bd8..05a548f539 100644 --- a/src/_nebari/utils.py +++ b/src/_nebari/utils.py @@ -45,7 +45,9 @@ def change_directory(directory): def run_subprocess_cmd(processargs, **kwargs): - """Runs subprocess command with realtime stdout logging with optional line prefix.""" + """Runs subprocess command with realtime stdout logging with optional line prefix, and outputs to a file.""" + output_file_path = kwargs.pop("output_file", None) + if "prefix" in kwargs: line_prefix = f"[{kwargs['prefix']}]: ".encode("utf-8") kwargs.pop("prefix") @@ -60,11 +62,12 @@ def run_subprocess_cmd(processargs, **kwargs): process = subprocess.Popen( processargs, - **kwargs, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, preexec_fn=os.setsid, + **kwargs, ) + # Set timeout thread timeout_timer = None if timeout > 0: @@ -73,30 +76,38 @@ def kill_process(): try: os.killpg(process.pid, signal.SIGTERM) except ProcessLookupError: - pass # Already finished + pass # Process has already finished timeout_timer = threading.Timer(timeout, kill_process) timeout_timer.start() - for line in iter(lambda: process.stdout.readline(), b""): - full_line = line_prefix + line - if strip_errors: - full_line = full_line.decode("utf-8") - full_line = re.sub( - r"\x1b\[31m", "", full_line - ) # Remove red ANSI escape code - full_line = full_line.encode("utf-8") - - sys.stdout.buffer.write(full_line) - sys.stdout.flush() - - if timeout_timer is not None: - timeout_timer.cancel() - - process.stdout.close() - return process.wait( - timeout=10 - ) # Should already have finished because we have drained stdout + output_file = open(output_file_path, "wb") if output_file_path else None + + try: + for line in iter(lambda: process.stdout.readline(), b""): + if output_file: + output_file.write(line) + output_file.flush() + + full_line = line_prefix + line + if strip_errors: + full_line = full_line.decode("utf-8") + full_line = re.sub( + r"\x1b\[31m", "", full_line + ) # Remove red ANSI escape code + full_line = full_line.encode("utf-8") + + sys.stdout.buffer.write(full_line) + sys.stdout.flush() + + finally: + if timeout_timer: + timeout_timer.cancel() + process.stdout.close() + if output_file: + output_file.close() + + return process.wait(timeout=10) def load_yaml(config_filename: Path): From 712c36bf1ca38b7ca373894a28390afc45838a62 Mon Sep 17 00:00:00 2001 From: Marcelo Villa Date: Mon, 20 May 2024 14:17:54 -0500 Subject: [PATCH 2/5] Incorporate tf-profile plugin and save results as gha artifacts. --- .github/workflows/test_local_integration.yaml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.github/workflows/test_local_integration.yaml b/.github/workflows/test_local_integration.yaml index 81810abfe1..af21054465 100644 --- a/.github/workflows/test_local_integration.yaml +++ b/.github/workflows/test_local_integration.yaml @@ -4,6 +4,9 @@ env: TEST_USERNAME: "test-user" TEST_PASSWORD: "P@sswo3d" NEBARI_IMAGE_TAG: "main" + NEBARI_EXPORT_LOG_FILES: "true" + NEBARI_EXPORT_LOG_FILES_PATH: "terraform-logs" + NEBARI_TF_PROFILE_RESULTS_PATH: "tf-profile-results" on: pull_request: @@ -75,6 +78,15 @@ jobs: pip install .[dev] playwright install + - name: Clone nebari-tf-profile-plugin + uses: actions/checkout@v3 + with: + repository: nebari-dev/nebari-tf-profile-plugin + path: nebari-tf-profile-plugin + + - name: Install tf-profile plugin + run: pip install ./nebari-tf-profile-plugin + - uses: azure/setup-kubectl@v4.0.0 with: version: v1.19.16 @@ -196,3 +208,10 @@ jobs: working-directory: local-deployment run: | nebari destroy --config nebari-config.yaml --disable-prompt + + - name: Save tf-profile results as artifacts + if: always() + uses: actions/upload-artifact@v4.3.1 + with: + name: tf-profile-logs + path: ${{ env.NEBARI_TF_PROFILE_RESULTS_PATH }} From 1efba1f6bf2d70c00de26bb0827671cd258f5e79 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Mon, 20 May 2024 17:07:18 -0300 Subject: [PATCH 3/5] add new flags for log exporting --- src/_nebari/deploy.py | 9 ++++- src/_nebari/destroy.py | 6 +++- src/_nebari/provider/terraform.py | 55 +++++++++++++++++++++++++----- src/_nebari/stages/base.py | 11 +++++- src/_nebari/subcommands/deploy.py | 6 ++++ src/_nebari/subcommands/destroy.py | 7 +++- 6 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/_nebari/deploy.py b/src/_nebari/deploy.py index 46cc20179b..3b5e861756 100644 --- a/src/_nebari/deploy.py +++ b/src/_nebari/deploy.py @@ -15,6 +15,7 @@ def deploy_configuration( stages: List[hookspecs.NebariStage], disable_prompt: bool = False, disable_checks: bool = False, + export_logfiles: bool = False, ) -> Dict[str, Any]: if config.prevent_deploy: raise ValueError( @@ -50,7 +51,13 @@ def deploy_configuration( with contextlib.ExitStack() as stack: for stage in stages: s = stage(output_directory=pathlib.Path.cwd(), config=config) - stack.enter_context(s.deploy(stage_outputs, disable_prompt)) + stack.enter_context( + s.deploy( + stage_outputs, + disable_prompt, + export_terraform_logs=export_logfiles, + ) + ) if not disable_checks: s.check(stage_outputs, disable_prompt) diff --git a/src/_nebari/destroy.py b/src/_nebari/destroy.py index 900ad8acf8..073f7bb928 100644 --- a/src/_nebari/destroy.py +++ b/src/_nebari/destroy.py @@ -9,7 +9,11 @@ logger = logging.getLogger(__name__) -def destroy_configuration(config: schema.Main, stages: List[hookspecs.NebariStage]): +def destroy_configuration( + config: schema.Main, + stages: List[hookspecs.NebariStage], + export_logfiles: bool = False, +): logger.info( """Removing all infrastructure, your local files will still remain, you can use 'nebari deploy' to re-install infrastructure using same config file\n""" diff --git a/src/_nebari/provider/terraform.py b/src/_nebari/provider/terraform.py index b1863ba0d2..81c141987a 100644 --- a/src/_nebari/provider/terraform.py +++ b/src/_nebari/provider/terraform.py @@ -1,7 +1,9 @@ import contextlib +import datetime import io import json import logging +import os import platform import re import subprocess @@ -28,6 +30,7 @@ def deploy( terraform_import: bool = False, terraform_apply: bool = True, terraform_destroy: bool = False, + terraform_logs_export: bool = False, input_vars: Dict[str, Any] = {}, state_imports: List[Any] = [], ): @@ -68,10 +71,18 @@ def deploy( ) if terraform_apply: - apply(directory, var_files=[f.name]) + apply( + directory, + var_files=[f.name], + terraform_logs_export=terraform_logs_export, + ) if terraform_destroy: - destroy(directory, var_files=[f.name]) + destroy( + directory, + var_files=[f.name], + terraform_logs_export=terraform_logs_export, + ) return output(directory) @@ -137,10 +148,29 @@ def init(directory=None, upgrade=True): run_terraform_subprocess(command, cwd=directory, prefix="terraform") -def apply(directory=None, targets=None, var_files=None): +def _gen_terraform_logfile_dest(prefix, suffix): + _root_log_dir = os.environ.get( + "NEBARI_EXPORT_LOG_FILES_PATH", Path.cwd().as_posix() + ) + directory = Path(_root_log_dir) / datetime.now().strftime("%Y-%m-%d-%H-%M-%S") + directory.mkdir(parents=True, exist_ok=True) + return directory / f"{prefix}_{suffix}.log" + + +def apply( + directory=None, targets=None, var_files=None, terraform_logs_export: bool = False +): targets = targets or [] var_files = var_files or [] + if terraform_logs_export: + output_file = _gen_terraform_logfile_dest( + "terraform_apply", directory.split("/")[-1] + ) + logger.info(f"terraform destroy logs will be exported to {output_file}") + else: + output_file = None + logger.info(f"terraform apply directory={directory} targets={targets}") command = ( ["apply", "-auto-approve"] @@ -148,12 +178,11 @@ def apply(directory=None, targets=None, var_files=None): + ["-var-file=" + _ for _ in var_files] ) with timer(logger, "terraform apply"): - _stage = directory.split("/")[-1] run_terraform_subprocess( command, cwd=directory, prefix="terraform", - output_file=f"terraform_apply_{_stage}.log", + output_file=output_file, ) @@ -199,11 +228,22 @@ def refresh(directory=None, var_files=None): run_terraform_subprocess(command, cwd=directory, prefix="terraform") -def destroy(directory=None, targets=None, var_files=None): +def destroy( + directory=None, targets=None, var_files=None, terraform_logs_export: bool = False +): targets = targets or [] var_files = var_files or [] logger.info(f"terraform destroy directory={directory} targets={targets}") + + if terraform_logs_export: + output_file = _gen_terraform_logfile_dest( + "terraform_destroy", directory.split("/")[-1] + ) + logger.info(f"terraform destroy logs will be exported to {output_file}") + else: + output_file = None + command = ( [ "destroy", @@ -214,12 +254,11 @@ def destroy(directory=None, targets=None, var_files=None): ) with timer(logger, "terraform destroy"): - _stage = directory.split("/")[-1] run_terraform_subprocess( command, cwd=directory, prefix="terraform", - output_file=f"terraform_destroy_{_stage}.log", + output_file=output_file, ) diff --git a/src/_nebari/stages/base.py b/src/_nebari/stages/base.py index 60a4821a24..633abb5473 100644 --- a/src/_nebari/stages/base.py +++ b/src/_nebari/stages/base.py @@ -58,7 +58,10 @@ def set_outputs( @contextlib.contextmanager def deploy( - self, stage_outputs: Dict[str, Dict[str, Any]], disable_prompt: bool = False + self, + stage_outputs: Dict[str, Dict[str, Any]], + disable_prompt: bool = False, + export_terraform_logs: bool = False, ): deploy_config = dict( directory=str(self.output_directory / self.stage_prefix), @@ -69,6 +72,9 @@ def deploy( deploy_config["terraform_import"] = True deploy_config["state_imports"] = state_imports + if export_terraform_logs: + deploy_config["terraform_logs_export"] = True + self.set_outputs(stage_outputs, terraform.deploy(**deploy_config)) self.post_deploy(stage_outputs, disable_prompt) yield @@ -89,6 +95,7 @@ def destroy( stage_outputs: Dict[str, Dict[str, Any]], status: Dict[str, bool], ignore_errors: bool = True, + export_terraform_logs: bool = False, ): self.set_outputs( stage_outputs, @@ -99,6 +106,7 @@ def destroy( terraform_import=True, terraform_apply=False, terraform_destroy=False, + terraform_logs_export=export_terraform_logs, ), ) yield @@ -110,6 +118,7 @@ def destroy( terraform_import=True, terraform_apply=False, terraform_destroy=True, + terraform_logs_export=export_terraform_logs, ) status["stages/" + self.name] = True except terraform.TerraformException as e: diff --git a/src/_nebari/subcommands/deploy.py b/src/_nebari/subcommands/deploy.py index 0aa861027f..5a371ff8b2 100644 --- a/src/_nebari/subcommands/deploy.py +++ b/src/_nebari/subcommands/deploy.py @@ -59,6 +59,11 @@ def deploy( "--skip-remote-state-provision", help="Skip terraform state deployment which is often required in CI once the terraform remote state bootstrapping phase is complete", ), + export_logfiles: bool = typer.Option( + False, + "--export-logfiles", + help="Toggles the export of Terraform's stages logfiles", + ), ): """ Deploy the Nebari cluster from your [purple]nebari-config.yaml[/purple] file. @@ -89,4 +94,5 @@ def deploy( stages, disable_prompt=disable_prompt, disable_checks=disable_checks, + export_logfiles=export_logfiles, ) diff --git a/src/_nebari/subcommands/destroy.py b/src/_nebari/subcommands/destroy.py index d94f8cd056..6e460dc5bc 100644 --- a/src/_nebari/subcommands/destroy.py +++ b/src/_nebari/subcommands/destroy.py @@ -32,6 +32,11 @@ def destroy( "--disable-prompt", help="Destroy entire Nebari cluster without confirmation request. Suggested for CI use.", ), + export_logfiles: bool = typer.Option( + False, + "--export-logfiles", + help="Toggles the export of Terraform's stages logfiles", + ), ): """ Destroy the Nebari cluster from your [purple]nebari-config.yaml[/purple] file. @@ -49,7 +54,7 @@ def _run_destroy( if not disable_render: render_template(output_directory, config, stages) - destroy_configuration(config, stages) + destroy_configuration(config, stages, export_logfiles=export_logfiles) if disable_prompt: _run_destroy() From bc62d15959e2560835505411751e48e00cc65804 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Mon, 20 May 2024 17:12:54 -0300 Subject: [PATCH 4/5] add env var overriding to export logs --- src/_nebari/subcommands/deploy.py | 3 ++- src/_nebari/subcommands/destroy.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/_nebari/subcommands/deploy.py b/src/_nebari/subcommands/deploy.py index 5a371ff8b2..6068acd636 100644 --- a/src/_nebari/subcommands/deploy.py +++ b/src/_nebari/subcommands/deploy.py @@ -1,3 +1,4 @@ +import os import pathlib from typing import Optional @@ -60,7 +61,7 @@ def deploy( help="Skip terraform state deployment which is often required in CI once the terraform remote state bootstrapping phase is complete", ), export_logfiles: bool = typer.Option( - False, + os.getenv("NEBARI_EXPORT_LOG_FILES", False), "--export-logfiles", help="Toggles the export of Terraform's stages logfiles", ), diff --git a/src/_nebari/subcommands/destroy.py b/src/_nebari/subcommands/destroy.py index 6e460dc5bc..83215d99cf 100644 --- a/src/_nebari/subcommands/destroy.py +++ b/src/_nebari/subcommands/destroy.py @@ -1,3 +1,4 @@ +import os import pathlib import typer @@ -33,7 +34,7 @@ def destroy( help="Destroy entire Nebari cluster without confirmation request. Suggested for CI use.", ), export_logfiles: bool = typer.Option( - False, + os.getenv("NEBARI_EXPORT_LOG_FILES", False), "--export-logfiles", help="Toggles the export of Terraform's stages logfiles", ), From 4e1baa475c03697e2bf48efd5b25aad1f3f7ae7e Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Mon, 20 May 2024 17:13:17 -0300 Subject: [PATCH 5/5] print out available nebari stages/hooks --- .github/workflows/test_local_integration.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test_local_integration.yaml b/.github/workflows/test_local_integration.yaml index af21054465..8267dab1dc 100644 --- a/.github/workflows/test_local_integration.yaml +++ b/.github/workflows/test_local_integration.yaml @@ -87,6 +87,10 @@ jobs: - name: Install tf-profile plugin run: pip install ./nebari-tf-profile-plugin + - name: List pluging and stages + run: | + nebari info + - uses: azure/setup-kubectl@v4.0.0 with: version: v1.19.16