-
Notifications
You must be signed in to change notification settings - Fork 462
test/e2e: Drop TestFIPS #1244
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
test/e2e: Drop TestFIPS #1244
Conversation
IIRC we did this just to speed up these tests because updating workers 1 by 1 blew out our hour budget. The router requires a minimum of two workers though, and we're just going to be fighting its PDB. Since customers can't sanely do this, let's stop doing it in our tests. If our tests take too long...we'll have to either cut down the tests or make them a periodic, etc.
|
/skip |
|
e2e-gcp-upgrade failure reported: |
|
/skip |
|
99 failures and the failure im looking for ain't one. =( /retest |
|
Also testing in my own cluster just in case this run fails too.. |
|
ok locally on my own cluster, the e2e runs as-is (minus max unavailable bump and minus FIPs test). fyi: |
|
So running the tests I saw: CustomPool test ran for 90s in comparision, in Colin's PR it ran for..1200s?! |
|
In colin's PR total test time was 5567.370s TestCustomPool took 1206s compared to my 90s?! @cgwalters Something is super weird... I'd expect this obviously to have shorter time bc i took out FIPs test, but why is that other test that much faster? |
|
Why are the tests run in CI so much slower across the board? TestSSH 260 : 650 |
|
im going to try to run the whole suite in my cluster (with none of our changes).. is gcp just slow? |
31d7e6d to
a51bd11
Compare
|
stop bumping max unavailable + dropping FIPS => tests pass Now trying to run FIPS test last and bumping the wait for poll time bc locally it failed but was still well under our e2e timeout of 120 minutes (failed runs were ~90), it timed out bc of our 20 minute wait time... stop bumping max unavailable + bump wait for pool timeout+10 + Run Fips last (when no FIPs, TestCustomPool takes 90s so run it first!) => FIPS fails |
|
/skip |
1 similar comment
|
/skip |
|
So I think I see the problem and it's not anything to do with our test.. changing the order of the tests kind of made it clear. From a preexisting test there is an infra pool and that gets updated wiht the fips config ok:
If you look in workers journal it never begins applying |
|
It hits this container_runtime_config section and never seems to move on the same way that other machineconfigs are. Something is happening between rendering the worker config (which the MCO does correctly) and the actual application of the MC. |
|
Removed all tests but the FIPs test and increased the waiteForPool to 60 min. If this can't pass now we have big problems. |
|
/skip |
|
Based on my local testing, TestFips will keep failing bc something is wrong with Day 2 Fips - this is not a matter of increasing test duration. I propose that we drop TestFips to unblock CI (for non-fips prs) and merge #1233 dropping Day 2 Fips altogether. |
|
FTR: The run testing ONLY TestFIPs with a timeout of 60 minutes never proceeds past, just like every other run... There's not change we can make to get this test to pass. |
Day 2 FIPS is broken and this test is consistently failing. Day 2 FIPS will be dropped in openshift#1233 so this test will be unneeded. Related-to: openshift#1233
3cd2f6b to
a686c4a
Compare
|
/skip |
ashcrow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the old, unsupported implementation here is being replaced there is no need to test it. 👍
|
/retest |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, kikisdeliveryservice The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@kikisdeliveryservice: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
After a lot of testing TestFips is broken bc Day2 Fips is broken.
This test will be unneeded once #1233 merges and will unblock our approved PRs that can't get past our e2e test.
Also kept PDB changes from #1238 - we are well within our testing time, so there's no issue there.
Related-to: #1233