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

Closed
wants to merge 4 commits into from

Conversation

doncicuto
Copy link
Contributor

closes #257

πŸ“‘ Description

This PR adds a button to show deployment and statefulset logs in the UI. Here are a couple of screenshots.

  • Deployment logs

Captura desde 2024-06-19 11-25-46

  • Statefulset logs

Captura desde 2024-06-19 11-25-23

βœ… Checks

  • I have updated the documentation as required
  • I have performed a self-review of my code

Additional context

I've updated the handler.go to re-enable the endpoints required for this PR:

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

@doncicuto doncicuto requested a review from a team as a code owner June 19, 2024 09:34
@petar-cvit
Copy link
Collaborator

Hey @doncicuto, sorry for the delay. Can we somehow move the Logs button so its not in a separate section? I'm thinking of moving it up alongside the Manifest button, but that manifest button is not defined by a resource component.

Could you maybe move the manifest button in each component so we can add Logs and Manifest in the same row?

@doncicuto
Copy link
Contributor Author

Hi @petar-cvit I'll try to update the PR following your requirements. Thanks!

@doncicuto
Copy link
Contributor Author

Hi @petar-cvit, I think the button issue is fixed:

image

image

Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

Thanks @doncicuto, looks good. I just left one comment on the component organization. Also, we merged one other PR so you will have to resolve some conflicts

Comment on lines +523 to +535
<Button
onClick={function () {
fetchManifest(
resource.group,
resource.version,
resource.kind,
resource.namespace,
resource.name,
);
}}
>
View Manifest
</Button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this button to specific resource components? The only reason why I would do it is so because we don't have one row implemented in different components

@doncicuto
Copy link
Contributor Author

Hi @petar-cvit, no problem, so you want me to move the Manifest button to all components found in the k8s-resources folder?

@petar-cvit
Copy link
Collaborator

yeah, that way we can add/remove buttons depending on the kind. I suppose it's not going to be a lot of copy pasted code if we extract those buttons into separate components and reuse them in k8s resources components

@petar-cvit
Copy link
Collaborator

@doncicuto, any updates on the PR?

@doncicuto
Copy link
Contributor Author

I'm deeply sorry Petar I've been pretty busy these months and I don't think I can resume my collaboration right now so to be honest it's better to unassign this issue to me. I like the project so I Will definitely help again on the future as soon as I get more free time.

@petar-cvit
Copy link
Collaborator

@doncicuto no worries, I will close this one then

@petar-cvit petar-cvit closed this Oct 6, 2024
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.

Add a button for Deployment/Statefulset logs
2 participants