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

feat: adds ability to view pod logs #1122

Merged
merged 2 commits into from
Jan 9, 2023
Merged

Conversation

cdrage
Copy link
Contributor

@cdrage cdrage commented Jan 5, 2023

feat: adds ability to view pod logs

What does this PR do?

  • When viewing a pod, logs will show all containers log outputs with the
    container name appended to the beginning
  • By default, when clicking on Pod details, it will now show the pod
    logs

Screenshot/screencast of this PR

Screen.Recording.2023-01-05.at.3.04.08.PM.mov

What issues does this PR fix or reference?

Fixes 1/2 of #391

The other part (filter based on container name) will have to mock up
UX/UI before implementation

How to test this PR?

  1. Run two containers that echo the date: podman run -d alpine sh -c "while true; do date +%s; sleep 2; done"
  2. Podify it within Podman Desktop
  3. View the Pod details

@benoitf
Copy link
Collaborator

benoitf commented Jan 5, 2023

should we have different output color for each container ?

@cdrage
Copy link
Contributor Author

cdrage commented Jan 5, 2023

Sure, the name in the logs, not the output correct? Similar to Docker Compose?

@benoitf
Copy link
Collaborator

benoitf commented Jan 5, 2023

I was thinking also a different output
Like all lines of container1 in white, all lines of container 2 in yellow etc.

In addition yes the container name

### What does this PR do?
- When viewing a pod, logs will show all containers log outputs with the
  container name appended to the beginning
- By default, when clicking on Pod details, it will now show the pod
  logs

### Screenshot/screencast of this PR

<!-- Please include a screenshot or a screencast explaining what is doing this PR -->

### What issues does this PR fix or reference?

<!-- Please include any related issue from Podman Desktop repository (or from another issue tracker).
-->

Fixes 1/2 of containers#391

The other part (filter based on container name) will have to mock up
UX/UI before implementation

### How to test this PR?

<!-- Please explain steps to reproduce -->

1. Run two containers that echo the date: `podman run -d alpine sh -c
"while true; do date +%s; sleep 2; done"`
2. Podify it within Podman Desktop
3. View the Pod details

Signed-off-by: Charlie Drage <charlie@charliedrage.com>
@cdrage
Copy link
Contributor Author

cdrage commented Jan 5, 2023

I was thinking also a different output Like all lines of container1 in white, all lines of container 2 in yellow etc.

In addition yes the container name

I've updated the PR to colourize the container name. Made it similar to how Docker Compose does it with colourized names.

Perhaps we can update the colourized terminal output as well as the "filter by container name" in a separate PR after discussions with the UX/UI team?

@benoitf
Copy link
Collaborator

benoitf commented Jan 5, 2023

for sure it's not blocking the PR, was just thinking 🤔

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works fine

maybe colors should be adapted as it looks hard to read the text and few color difference between the two containers/output

image

@cdrage cdrage force-pushed the pod-logs branch 3 times, most recently from e9fb220 to 616e7f6 Compare January 6, 2023 13:37
@cdrage
Copy link
Contributor Author

cdrage commented Jan 6, 2023

works fine

maybe colors should be adapted as it looks hard to read the text and few color difference between the two containers/output

image

You're right, they don't appear great!

I've updated the code to more readable colours, as well as using the colours based upon the order in the array instead instead of calculating it based upon the container name. This makes the colours more predictable and consistent. I also do not include hard-to-read colours such as red or blue and rather use magenta and cyan.

@cdrage
Copy link
Contributor Author

cdrage commented Jan 6, 2023

Here's the updated example output:

Screenshot 2023-01-06 at 8 35 54 AM

Signed-off-by: Charlie Drage <charlie@charliedrage.com>
@benoitf
Copy link
Collaborator

benoitf commented Jan 6, 2023

the colors are looking way better now 🎉

@cdrage
Copy link
Contributor Author

cdrage commented Jan 6, 2023

Awesome! Thanks for the approval. Let's keep this open and merge on Monday, we all know how it's like to merge on a Friday with a new feature 🤣

@benoitf benoitf merged commit be91210 into containers:main Jan 9, 2023
@podman-desktop-bot podman-desktop-bot added this to the 0.11.0 milestone Jan 9, 2023
@slemeur
Copy link
Collaborator

slemeur commented Jan 9, 2023

Really cool !

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.

None yet

4 participants