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

pod/List: Fix IPs being displayed as "undefined" when not set #1994

Merged
merged 1 commit into from
May 29, 2024

Conversation

joaquimrocha
Copy link
Collaborator

@joaquimrocha joaquimrocha commented May 27, 2024

Just noticed that after the table changes we have pod IPs as being undefined (when there's no IP defined) in the pod list.

How to test:

  • Go to a pod that has no IP set to them; check how it shows nothing in the IP column (as opposed to undefined)

@joaquimrocha joaquimrocha requested review from vyncent-t and sniok May 27, 2024 13:46
frontend/src/components/pod/Details.tsx Outdated Show resolved Hide resolved
@vyncent-t
Copy link
Contributor

showed undefined for a minute when in the browser (may have been a weird pod I created or was behind on the render) but the storybook comp shows empty, current build error keeps the browser view away

@joaquimrocha joaquimrocha force-pushed the no-undefined-ip branch 2 times, most recently from aa0d66a to 2304f05 Compare May 29, 2024 16:33
@illume
Copy link
Collaborator

illume commented May 29, 2024

Rerunning the container e2e tests, which failed.

@illume
Copy link
Collaborator

illume commented May 29, 2024

showed undefined for a minute when in the browser (may have been a weird pod I created or was behind on the render) but the storybook comp shows empty, current build error keeps the browser view away

I did some digging on this... The API reference says the status field should always be there. Could be a kubernetes bug though where the status field is missing.

@illume
Copy link
Collaborator

illume commented May 29, 2024

I reran the container job a few times, and it keeps failing. It's passing recently in other PRs, so I guess it's a real issue.

@joaquimrocha
Copy link
Collaborator Author

The problem is that if we do not merge, then this is a real issue as it could show undefined for the IP (the state must exist but the IP could be undefined).

@illume
Copy link
Collaborator

illume commented May 29, 2024

This is the last one that passed the container build from this PR: https://github.com/headlamp-k8s/headlamp/actions/runs/9288417462/job/25559819347 With this commit fc6578a But I don't see any difference in the code, except for something in the storyhelper file... which should not affect the test.

So... I'm pretty sure it is a temporary error with the github runners somehow.

Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

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

👍

Signed-off-by: Joaquim Rocha <joaquim.rocha@microsoft.com>
@illume illume added bug Something isn't working frontend Issues related to the frontend Table Table component issues labels May 29, 2024
@joaquimrocha joaquimrocha merged commit d81fad0 into main May 29, 2024
15 of 16 checks passed
@joaquimrocha joaquimrocha deleted the no-undefined-ip branch May 29, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working frontend Issues related to the frontend Table Table component issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants