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

Fix frontend crash in Summary page when data is missing #23

Conversation

antoninbas
Copy link
Contributor

When Antrea runs in networkPolicyOnly mode, the nodeSubnets field is missing (omitted because empty) from AntreaAgentInfo resources. The frontend needs to handle this case gracefully.

Fixes #22

When Antrea runs in networkPolicyOnly mode, the nodeSubnets field is
missing (omitted because empty) from AntreaAgentInfo resources. The
frontend needs to handle this case gracefully.

Fixes antrea-io#22

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas requested a review from xliuxu April 28, 2023 02:21

function refToString(ref: K8sRef): string {
function refToString(ref: K8sRef | undefined): string {
Copy link

Choose a reason for hiding this comment

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

Suggested change
function refToString(ref: K8sRef | undefined): string {
function refToString(ref?: K8sRef): string {

if (ref.namespace) return ref.namespace + '/' + ref.name;
return ref.name;
}

// returns status and last heartbeat time
function getConditionInfo(conditions: Condition[], name: string): [string, string] {
function getConditionInfo(conditions: Condition[] | undefined, name: string): [string, string] {
Copy link

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the 2 mean different things:

function getConditionInfo(conditions: Condition[] | undefined, name: string)
function getConditionInfo(conditions?: Condition[], name: string)

The second one is not valid because ?: indicates an optional parameter, and an optional parameter cannot come before a required parameter (name). Here conditions is not really an optional parameter, but a required parameter that can be undefined.

Copy link

@xliuxu xliuxu left a comment

Choose a reason for hiding this comment

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

LGTM with some nits.

@antoninbas
Copy link
Contributor Author

@xliuxu Thanks for the quick review. I'll merge as it is to fix the bug. If you have a better alternative, let me know. I was considering handling the undefined case at the caller, but it would have been more verbose. These are just helper functions used locally, so not too important.

@antoninbas antoninbas merged commit acfa914 into antrea-io:main Apr 28, 2023
@antoninbas antoninbas deleted the fix-frontend-crash-in-networkPolicyOnly-mode branch April 28, 2023 17:05
antoninbas added a commit that referenced this pull request May 4, 2023
When Antrea runs in networkPolicyOnly mode, the nodeSubnets field is
missing (omitted because empty) from AntreaAgentInfo resources. The
frontend needs to handle this case gracefully.

Fixes #22

Signed-off-by: Antonin Bas <abas@vmware.com>
This pull request was closed.
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.

UI crash for /summary page when Antrea runs in networkPolicyOnly mode
2 participants