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

Skip TestFQDNCacheMinTTL if cluster has Windows Nodes. Fixes #6891 #6918

Closed
wants to merge 1 commit into from

Conversation

devc007
Copy link
Contributor

@devc007 devc007 commented Jan 10, 2025

The TestFQDNCacheMinTTL e2e test currently does not support Windows. We skip it if any Node in the test cluster is a Windows Node, which is also consistent with other AntreaPolicy e2e tests.

As @antoninbas suggested, I have relocated the skipX condition to the beginning of TestFQDNCacheMinTTL. Now, the skip condition will execute before any subset of TestFQDNCacheMinTTL, making testWithFQDNCacheMinTTL shorter and possibly more efficient.

The TestFQDNCacheMinTTL e2e test currently does not support Windows.
We skip it if any Node in the test cluster is a Windows Node, which is also
consistent with other AntreaPolicy e2e tests.
@devc007 devc007 changed the title Fixes #6891 "Skip TestFQDNCacheMinTTL if cluster has Windows Nodes." Skip TestFQDNCacheMinTTL if cluster has Windows Nodes #6891 Jan 10, 2025
@devc007 devc007 changed the title Skip TestFQDNCacheMinTTL if cluster has Windows Nodes #6891 Skip TestFQDNCacheMinTTL if cluster has Windows Nodes. Fixes #6891 Jan 10, 2025
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

Fixes #6891 should be part of the commit message body

skipIfIPv6Cluster(t)
skipIfNotRequired(t, "mode-irrelevant")


Copy link
Contributor

Choose a reason for hiding this comment

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

is this line not empty?

skipIfIPv6Cluster(t)
skipIfNotRequired(t, "mode-irrelevant")


Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra empty line

@antoninbas
Copy link
Contributor

@devc007 I'm closing this PR. You already have #6913, please do not open a duplicate PR. You should update the existing one.

@antoninbas antoninbas closed this Jan 10, 2025
@devc007 devc007 deleted the antreatest branch January 10, 2025 19:41
@devc007
Copy link
Contributor Author

devc007 commented Jan 10, 2025

@antoninbas Could you please close #6913? I'll create a fresh PR instead, as I'm not entirely sure how to make a commit in the existing one.

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.

2 participants