From 16298f00ba104b9bdbac0590568ef6dae9c20b50 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 15:30:52 -0500 Subject: [PATCH 1/8] control: remove `default` namespace from `logs` A blank namespace is better than using "default". This is because we can figure out the proper namespace to query against within the logic of the `_logs` function or the user can specify the namespace as they desire. Also, Service Account users have no reason to query the `default` namespace. --- src/warnet/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/warnet/control.py b/src/warnet/control.py index 83d358a4e..b46914c76 100644 --- a/src/warnet/control.py +++ b/src/warnet/control.py @@ -360,7 +360,7 @@ def filter(path): @click.command() @click.argument("pod_name", type=str, default="") @click.option("--follow", "-f", is_flag=True, default=False, help="Follow logs") -@click.option("--namespace", type=str, default="default", show_default=True) +@click.option("--namespace", type=str, default="", show_default=True) def logs(pod_name: str, follow: bool, namespace: str): """Show the logs of a pod""" return _logs(pod_name, follow, namespace) From 1ac3281788add761ddcd761fd1b80af3eaeff027 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 15:33:11 -0500 Subject: [PATCH 2/8] control: remove the immediate namespace assignment There is also no reason to eagerly assign a namespace at the beginning of the `_logs` function because we can figure out the right namespace later on based on context. Also, we should remove the namespace from the Exception handling because we may not have a namespace at that point. --- src/warnet/control.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/warnet/control.py b/src/warnet/control.py index b46914c76..500228f39 100644 --- a/src/warnet/control.py +++ b/src/warnet/control.py @@ -367,8 +367,6 @@ def logs(pod_name: str, follow: bool, namespace: str): def _logs(pod_name: str, follow: bool, namespace: Optional[str] = None): - namespace = get_default_namespace_or(namespace) - def format_pods(pods: list[V1Pod]) -> list[str]: sorted_pods = sorted(pods, key=lambda pod: pod.metadata.creation_timestamp, reverse=True) return [f"{pod.metadata.name}: {pod.metadata.namespace}" for pod in sorted_pods] @@ -382,11 +380,11 @@ def format_pods(pods: list[V1Pod]) -> list[str]: pod_list.extend(formatted_tanks) except Exception as e: - print(f"Could not fetch any pods in namespace ({namespace}): {e}") + click.secho(f"Could not fetch any pods: {e}") return if not pod_list: - print(f"Could not fetch any pods in namespace ({namespace})") + click.secho("Could not fetch any pods.") return q = [ From 2cda487587140ab3e028e737d480baece09091b4 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 15:35:52 -0500 Subject: [PATCH 3/8] control: update format_pods to include namespace We need a way to filter out pods when the user specifies a namespace using: `warnet logs --namespace SOME_NAMESPACE` This is because an admin user may specify a namespace, and we want to show them only those pods that belong to the namespace. Performing this action in `format_pods` seems natural. --- src/warnet/control.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/warnet/control.py b/src/warnet/control.py index 500228f39..7b7bb9e75 100644 --- a/src/warnet/control.py +++ b/src/warnet/control.py @@ -367,15 +367,17 @@ def logs(pod_name: str, follow: bool, namespace: str): def _logs(pod_name: str, follow: bool, namespace: Optional[str] = None): - def format_pods(pods: list[V1Pod]) -> list[str]: + def format_pods(pods: list[V1Pod], namespace: Optional[str]) -> list[str]: + if namespace: + pods = [pod for pod in pods if pod.metadata.namespace == namespace] sorted_pods = sorted(pods, key=lambda pod: pod.metadata.creation_timestamp, reverse=True) return [f"{pod.metadata.name}: {pod.metadata.namespace}" for pod in sorted_pods] if pod_name == "": try: pod_list = [] - formatted_commanders = format_pods(get_mission(COMMANDER_MISSION)) - formatted_tanks = format_pods(get_mission(TANK_MISSION)) + formatted_commanders = format_pods(get_mission(COMMANDER_MISSION), namespace) + formatted_tanks = format_pods(get_mission(TANK_MISSION), namespace) pod_list.extend(formatted_commanders) pod_list.extend(formatted_tanks) From 19c5b3cfbce90beff8ee7f32defe61d75739fe44 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 15:37:44 -0500 Subject: [PATCH 4/8] control: get the pod in `warnet logs` When the user simply runs `warnet logs` or `warnet logs --namespace NS`, we should handle that logic separately from when they run `warnet logs POD_NAME`. This is because doing so separates the logic of wrangling multiple namespaces. When the user runs `warnet logs` or `warnet logs --namespace NS`, the logic of wrangling the namespaces is very simple: either grab all the namespaces we can, or grab one specific namespace. When the user runs `warnet logs SOME_POD`, we find ourselves in a situation where we actually need to query all the namespaces and the whittle down the results to one namespace which contains the pod OR let the user know that SOME_POD exists in multiple namespaces -- in which case we ask them to narrow down their query to a specific namespace. --- src/warnet/control.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/warnet/control.py b/src/warnet/control.py index 7b7bb9e75..82c880b24 100644 --- a/src/warnet/control.py +++ b/src/warnet/control.py @@ -402,6 +402,13 @@ def format_pods(pods: list[V1Pod], namespace: Optional[str]) -> list[str]: else: return # cancelled by user + try: + pod = get_pod(pod_name, namespace=namespace) + except Exception as e: + click.secho(e) + click.secho(f"Could not get pod: {pod_name}: {namespace}") + return + try: pod = get_pod(pod_name, namespace=namespace) eligible_container_names = [BITCOINCORE_CONTAINER, COMMANDER_CONTAINER] From 433aeb8de58b99fff5ee7f5e71a8a1aeaabadbf8 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 15:42:43 -0500 Subject: [PATCH 5/8] control: wrangle multiple namespaces When the user runs `warnet logs SOME_POD --namespace NS`, we can actually just try to grab the pod from that namespace. This is handled in the "else" section towards the end of this commit. However, when the user does not specify a namespace (`warnet logs SOME_POD`), then we get to query all the namepaces available to the user, whittle them down, and give the user what they want. We also ask the user to specify the namespace if we find their desired pod in more than one namespace. --- src/warnet/control.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/warnet/control.py b/src/warnet/control.py index 82c880b24..85680ac36 100644 --- a/src/warnet/control.py +++ b/src/warnet/control.py @@ -409,8 +409,43 @@ def format_pods(pods: list[V1Pod], namespace: Optional[str]) -> list[str]: click.secho(f"Could not get pod: {pod_name}: {namespace}") return + else: + pod = None + if not namespace: + namespaces = [] + for v1namespace in get_namespaces(): + namespace_candidate = v1namespace.metadata.name + try: + pod = get_pod(pod_name, namespace=namespace_candidate) + namespaces.append(namespace_candidate) + except Exception: + pass + + if len(namespaces) > 1: + click.secho(f"The pod '{pod.metadata.name}' is found in these namespaces:") + for ns in namespaces: + click.secho(f" - {ns}") + click.secho("Please limit your search to one of those namespaces.") + return + + if not namespaces: + click.echo(f"Could not find pod in any namespaces: {pod_name}") + + namespace = namespaces[0] + + else: + try: + pod = get_pod(pod_name, namespace=namespace) + except Exception as e: + click.secho(e) + click.secho(f"Could not get pod: {pod_name}: {namespace}") + return + + if not pod: + click.secho(f"Could not find: {pod_name}", fg="yellow") + sys.exit(1) + try: - pod = get_pod(pod_name, namespace=namespace) eligible_container_names = [BITCOINCORE_CONTAINER, COMMANDER_CONTAINER] available_container_names = [container.name for container in pod.spec.containers] container_name = next( From 5eb0c8104a60b07d47b56f92b4fee1f4ab473697 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 15:45:08 -0500 Subject: [PATCH 6/8] namespace_admin_test: add `warnet logs` logic --- test/namespace_admin_test.py | 53 ++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/namespace_admin_test.py b/test/namespace_admin_test.py index 5dd4cdfef..5ca77e90b 100755 --- a/test/namespace_admin_test.py +++ b/test/namespace_admin_test.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import Callable, Optional +import pexpect from scenarios_test import ScenariosTest from test_base import TestBase @@ -49,7 +50,9 @@ def run_test(self): self.setup_namespaces() self.setup_service_accounts() self.setup_network() + self.admin_checks_logs() self.authenticate_and_become_bob() + self.bob_checks_logs() self.bob_runs_scenario_tests() finally: self.return_to_initial_context() @@ -176,6 +179,56 @@ def bob_runs_scenario_tests(self): super().run_test() assert self.this_is_the_current_context(self.bob_context) + def bob_checks_logs(self): + assert self.this_is_the_current_context(self.bob_context) + self.log.info("Bob will check the logs") + bitcoin_version_slug = "Bitcoin Core version v27.0.0" + self.sut = pexpect.spawn("warnet logs", maxread=4096 * 10) + self.sut.expect("Please choose a pod", timeout=10) + self.sut.sendline("") + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.sut = pexpect.spawn(f"warnet logs --namespace {self.red_namespace}", maxread=4096 * 10) + self.sut.expect("Please choose a pod", timeout=10) + self.sut.sendline("") + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.sut = pexpect.spawn("warnet logs tank-0008", maxread=4096 * 10) + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.sut = pexpect.spawn( + f"warnet logs tank-0008 --namespace {self.red_namespace}", maxread=4096 * 10 + ) + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.log.info("Bob has checked the logs") + assert self.this_is_the_current_context(self.bob_context) + + def admin_checks_logs(self): + assert self.this_is_the_current_context(self.initial_context) + self.log.info("The admin will check the logs") + bitcoin_version_slug = "Bitcoin Core version v27.0.0" + self.sut = pexpect.spawn("warnet logs", maxread=4096 * 10) + self.sut.expect("Please choose a pod", timeout=10) + self.sut.sendline("") + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.sut = pexpect.spawn(f"warnet logs --namespace {self.red_namespace}", maxread=4096 * 10) + self.sut.expect("Please choose a pod", timeout=10) + self.sut.sendline("") + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.sut = pexpect.spawn("warnet logs tank-0008", maxread=4096 * 10) + self.sut.expect("The pod 'tank-0008' is found in these namespaces", timeout=10) + self.sut.close() + self.sut = pexpect.spawn( + f"warnet logs tank-0008 --namespace {self.red_namespace}", maxread=4096 * 10 + ) + self.sut.expect(bitcoin_version_slug, timeout=10) + self.sut.close() + self.log.info("The admin has checked the logs") + assert self.this_is_the_current_context(self.initial_context) + def remove_user(kubeconfig_data: dict, username: str) -> dict: kubeconfig_data["users"] = [ From 94a8429c8590520b82ad6aa2073281676425d874 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 21:03:02 -0500 Subject: [PATCH 7/8] add missing `return` I noticed that I did not include a `return` if the user simply searches for a non-existent pod. So, I added the `return` statement. In order to make a test for this, I realized I would need to parse the output of `warnet logs` to search for "Traceback (" in order to see if there is some kind of error. Otherwise, if I simple "expect" a message, the test will pass even though the program errors out with some kind of stack trace. That is why I made `expect_without_traceback`. It will catch the missing `return` statement. I will add expect_without_traceback to my other pexpect statements in the next commit. --- src/warnet/control.py | 1 + test/namespace_admin_test.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/warnet/control.py b/src/warnet/control.py index 85680ac36..981bd724a 100644 --- a/src/warnet/control.py +++ b/src/warnet/control.py @@ -430,6 +430,7 @@ def format_pods(pods: list[V1Pod], namespace: Optional[str]) -> list[str]: if not namespaces: click.echo(f"Could not find pod in any namespaces: {pod_name}") + return namespace = namespaces[0] diff --git a/test/namespace_admin_test.py b/test/namespace_admin_test.py index 5ca77e90b..baec605e7 100755 --- a/test/namespace_admin_test.py +++ b/test/namespace_admin_test.py @@ -201,6 +201,11 @@ def bob_checks_logs(self): ) self.sut.expect(bitcoin_version_slug, timeout=10) self.sut.close() + self.sut = pexpect.spawn("warnet logs this_does_not_exist", maxread=4096 * 10) + assert expect_without_traceback( + "Could not find pod in any namespaces", self.sut, timeout=10 + ) + self.sut.close() self.log.info("Bob has checked the logs") assert self.this_is_the_current_context(self.bob_context) @@ -226,6 +231,11 @@ def admin_checks_logs(self): ) self.sut.expect(bitcoin_version_slug, timeout=10) self.sut.close() + self.sut = pexpect.spawn("warnet logs this_does_not_exist", maxread=4096 * 10) + assert expect_without_traceback( + "Could not find pod in any namespaces", self.sut, timeout=10 + ) + self.sut.close() self.log.info("The admin has checked the logs") assert self.this_is_the_current_context(self.initial_context) @@ -244,6 +254,27 @@ def remove_context(kubeconfig_data: dict, context_name: str) -> dict: return kubeconfig_data +class StackTraceFoundException(Exception): + """Custom exception raised when a stack trace is found in the output.""" + + pass + + +def expect_without_traceback(expectation: str, sut: pexpect.spawn, timeout: int = 10) -> bool: + expectation_found = False + while True: + try: + sut.expect("\n", timeout=timeout) + line = sut.before.decode("utf-8") + if "Traceback (" in line: + raise StackTraceFoundException + if expectation in line: + expectation_found = True + except pexpect.exceptions.EOF: + break + return expectation_found + + if __name__ == "__main__": test = NamespaceAdminTest() test.run_test() From e0373c87e32b2231fca56699b2a89e7fbc047563 Mon Sep 17 00:00:00 2001 From: Grant Date: Wed, 16 Oct 2024 22:00:33 -0500 Subject: [PATCH 8/8] use `expect_without_traceback` in test I have extended my test to use `expect_without_traceback`. This means that I swapped out each `expect` instance with that function, and in doing so, noticed that I needed to watch for `\r` since we are using inquirer and click which seemingly terminate lines with `\r` as opposed to `\n`. I also decided to use a short timeout to give the program a chance to generate a Traceback. If a Traceback is not found within 2 seconds, I then move on to check if we found our expected string and return that information as a bool. I also promoted the bitcoin "slug" string to an instance attribute. I also "demoted" the `sut` from an instance attribute to simply a variable. --- test/namespace_admin_test.py | 102 +++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/test/namespace_admin_test.py b/test/namespace_admin_test.py index baec605e7..f9d9f422e 100755 --- a/test/namespace_admin_test.py +++ b/test/namespace_admin_test.py @@ -42,6 +42,8 @@ def __init__(self): self.blue_users = ["carol-warnettest", "default", "mallory-warnettest"] self.red_users = ["alice-warnettest", self.bob_user, "default"] + self.bitcoin_version_slug = "Bitcoin Core version v27.0.0" + def run_test(self): try: os.chdir(self.tmpdir) @@ -182,60 +184,66 @@ def bob_runs_scenario_tests(self): def bob_checks_logs(self): assert self.this_is_the_current_context(self.bob_context) self.log.info("Bob will check the logs") - bitcoin_version_slug = "Bitcoin Core version v27.0.0" - self.sut = pexpect.spawn("warnet logs", maxread=4096 * 10) - self.sut.expect("Please choose a pod", timeout=10) - self.sut.sendline("") - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn(f"warnet logs --namespace {self.red_namespace}", maxread=4096 * 10) - self.sut.expect("Please choose a pod", timeout=10) - self.sut.sendline("") - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn("warnet logs tank-0008", maxread=4096 * 10) - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn( + + sut = pexpect.spawn("warnet logs", maxread=4096 * 10) + assert expect_without_traceback("Please choose a pod", sut) + sut.sendline("") + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn(f"warnet logs --namespace {self.red_namespace}", maxread=4096 * 10) + assert expect_without_traceback("Please choose a pod", sut) + sut.sendline("") + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn("warnet logs tank-0008", maxread=4096 * 10) + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn( f"warnet logs tank-0008 --namespace {self.red_namespace}", maxread=4096 * 10 ) - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn("warnet logs this_does_not_exist", maxread=4096 * 10) - assert expect_without_traceback( - "Could not find pod in any namespaces", self.sut, timeout=10 - ) - self.sut.close() + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn("warnet logs this_does_not_exist", maxread=4096 * 10) + assert expect_without_traceback("Could not find pod in any namespaces", sut) + sut.close() + self.log.info("Bob has checked the logs") assert self.this_is_the_current_context(self.bob_context) def admin_checks_logs(self): assert self.this_is_the_current_context(self.initial_context) self.log.info("The admin will check the logs") - bitcoin_version_slug = "Bitcoin Core version v27.0.0" - self.sut = pexpect.spawn("warnet logs", maxread=4096 * 10) - self.sut.expect("Please choose a pod", timeout=10) - self.sut.sendline("") - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn(f"warnet logs --namespace {self.red_namespace}", maxread=4096 * 10) - self.sut.expect("Please choose a pod", timeout=10) - self.sut.sendline("") - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn("warnet logs tank-0008", maxread=4096 * 10) - self.sut.expect("The pod 'tank-0008' is found in these namespaces", timeout=10) - self.sut.close() - self.sut = pexpect.spawn( + + sut = pexpect.spawn("warnet logs", maxread=4096 * 10) + assert expect_without_traceback("Please choose a pod", sut) + sut.sendline("") + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn(f"warnet logs --namespace {self.red_namespace}", maxread=4096 * 10) + assert expect_without_traceback("Please choose a pod", sut) + sut.sendline("") + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn("warnet logs tank-0008", maxread=4096 * 10) + assert expect_without_traceback("The pod 'tank-0008' is found in these namespaces", sut) + sut.close() + + sut = pexpect.spawn( f"warnet logs tank-0008 --namespace {self.red_namespace}", maxread=4096 * 10 ) - self.sut.expect(bitcoin_version_slug, timeout=10) - self.sut.close() - self.sut = pexpect.spawn("warnet logs this_does_not_exist", maxread=4096 * 10) - assert expect_without_traceback( - "Could not find pod in any namespaces", self.sut, timeout=10 - ) - self.sut.close() + assert expect_without_traceback(self.bitcoin_version_slug, sut) + sut.close() + + sut = pexpect.spawn("warnet logs this_does_not_exist", maxread=4096 * 10) + assert expect_without_traceback("Could not find pod in any namespaces", sut) + sut.close() + self.log.info("The admin has checked the logs") assert self.this_is_the_current_context(self.initial_context) @@ -260,17 +268,17 @@ class StackTraceFoundException(Exception): pass -def expect_without_traceback(expectation: str, sut: pexpect.spawn, timeout: int = 10) -> bool: +def expect_without_traceback(expectation: str, sut: pexpect.spawn, timeout: int = 2) -> bool: expectation_found = False while True: try: - sut.expect("\n", timeout=timeout) + sut.expect(["\r", "\n"], timeout=timeout) # inquirer uses \r line = sut.before.decode("utf-8") if "Traceback (" in line: raise StackTraceFoundException if expectation in line: expectation_found = True - except pexpect.exceptions.EOF: + except (pexpect.exceptions.EOF, pexpect.exceptions.TIMEOUT): break return expectation_found