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 a button for Deployment/Statefulset logs #257

Open
petar-cvit opened this issue May 5, 2024 · 6 comments · May be fixed by #348
Open

Add a button for Deployment/Statefulset logs #257

petar-cvit opened this issue May 5, 2024 · 6 comments · May be fixed by #348
Assignees
Labels
good first issue Good for newcomers react UI Update on the UI

Comments

@petar-cvit
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
We recently implemented an endpoint for fetching aggregated logs for Deployments and Statefulsets. It would be useful to add a button for viewing logs on Deployments and Statefulsets.

Here is the modal for viewing logs on Deployment logs. The scope of the issue is to add a button on the Deployment level that would fetch logs from /resources/deployments/:namespace/:deployment/:container/logs

@petar-cvit petar-cvit added good first issue Good for newcomers UI Update on the UI react labels May 5, 2024
@petar-cvit
Copy link
Collaborator Author

Hey @yuvarajrece. Can you link the PR and the issue by commenting closes #257 to it? Also, can you rename the PR to be more descriptive?

@doncicuto
Copy link
Contributor

Hi @petar-cvit, would you assign this issue to me? Thanks!

@petar-cvit
Copy link
Collaborator Author

Sure @doncicuto

@doncicuto
Copy link
Contributor

Hi @petar-cvit, I've some doubts about the current backend implementation for Deployment/Statefulset logs

I assume that you want Cyclops UI to show all the logs for every single container for every pod found in the deployment/statefulset. This would be the screenshot for the button to check that the UI would be fine:

image

Right now I've readded the following endpoints that were removed by @hanshal101 (accidentally in a subsequent PR) :

h.router.GET("/resources/deployments/:namespace/:deployment/:container/logs", modulesController.GetDeploymentLogs)
h.router.GET("/resources/statefulsets/:namespace/:name/:container/logs", modulesController.GetStatefulSetsLogs)

The thing is that the endpoint requires to pass the name of the container that you want to get the logs. I've no problem to use the first container for the first pod in the deployment, but you'd miss the rest of the logs.

In summary, I'd like to know if you're ok with getting only the logs of the first container for the first pod in the deployment or you want to show all the logs something like https://github.com/stern/stern provides.

If you prefer the second then the backend should be modified in order to get logs for all the containers and pods and therefore the container name would not be passed to the endpoint. In this case maybe it'd be better to create a new issue and either @hanshal101 or me would make the needed changes to the backend.

Just let me know which path should I follow. Thanks!

@petar-cvit
Copy link
Collaborator Author

I was thinking of having the same approach as with pod logs. Pod logs show different containers in different tabs. Could we do that?
Screenshot 2024-06-17 at 14 03 56

But yeah, you should add these back

h.router.GET("/resources/deployments/:namespace/:deployment/:container/logs", modulesController.GetDeploymentLogs)
h.router.GET("/resources/statefulsets/:namespace/:name/:container/logs", modulesController.GetStatefulSetsLogs)

@doncicuto
Copy link
Contributor

Thanks @petar-cvit, I'm preparing the changes for the PR right now

@doncicuto doncicuto linked a pull request Jun 19, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers react UI Update on the UI
Projects
None yet
2 participants