Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better namespace logic to warnet logs #659

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mplsgrant
Copy link
Collaborator

This addresses #657. It includes a test, and commentary is in each commit.

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.
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.
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.
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.
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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant