Skip TestFQDNCacheMinTTL if cluster has Windows Nodes. Fixes #6891 #6913
Conversation
There was a problem hiding this comment.
Please use a better commit message / PR title.
You are skipping TestFQDNCacheMinTTL here, not TestAntreaPolicy.
A good example would be:
Skip TestFQDNCacheMinTTL if cluster has Windows Nodes
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.
|
Your commit message should also include |
|
@antoninbas can i close this PR and then create a new fresh PR? |
It is not acceptable to open a new PR every time you want to make a change. It creates unnecessary noise and we lose all of the review history. Please read about PR-based development online. It is straightforward to update an existing PR with additional changes (rebase / commit / push). |
test/e2e/antreapolicy_test.go
Outdated
| t.Run("minTTLUnset", func(t *testing.T) { testWithFQDNCacheMinTTL(t, 0) }) | ||
| t.Run("minTTL20s", func(t *testing.T) { testWithFQDNCacheMinTTL(t, 20) }) | ||
| } | ||
|
|
||
| func testWithFQDNCacheMinTTL(t *testing.T, fqdnCacheMinTTL int) { | ||
|
|
There was a problem hiding this comment.
nit: remove extra empty line
There was a problem hiding this comment.
@antoninbas pls have a look at new PR. Thanks for your patience.
devc007
changed the title
|
/test-windows-e2e |
|
@devc007 The CI job that checks for code formatting is failing, please take a look. You can run it locally with |
|
i have run it locally and it worked fine should i commit again? |
https://github.com/antrea-io/antrea/actions/runs/12754960077/job/35553600585?pr=6913 After fixing the issue, yes you will need to commit again. |
|
@devc007 your commit is not signed, it is failing the DCO check |
|
/test-windows-e2e |
1 similar comment
|
/test-windows-e2e |
|
@devc007 we are ready to merge this, but we cannot do it until you sign your commit |
…o#6891 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. Signed-off-by: devesh <[email protected]>
|
/test-all |
|
@antoninbas Could we move forward this fix to unblock the Windows e2e failures on main branch? |
|
@wenyingd Did we get a successful run of |
|
/test-windows-e2e |
Yes, windows-e2e for this PR used to succeed. Regarding the latest retry, it is still waiting in the queue now. |
|
/test-windows-e2e |
|
Provide a successful link for windows-e2e: http://10.164.243.223/view/Windows/job/antrea-windows-e2e-for-pull-request/91/ |
No description provided.