Add some more ExpectNoError checks to the e2e tests #4623
Conversation
github-actions
bot
added
feature/egress-ip
martinkennelly
force-pushed
the
e2e-no-error
branch
from
ae664b6
to
1a0c8d8
Compare
|
control-plane's are failing.. |
| @@ -789,7 +789,8 @@ var _ = ginkgo.Describe("e2e control plane", func() { | |||
| pod.Name != "etcd-ovn-control-plane" && | |||
| !strings.HasPrefix(pod.Name, "ovs-node") { | |||
| framework.Logf("%q", pod.Namespace) | |||
| e2epod.DeletePodWithWaitByName(context.TODO(), f.ClientSet, pod.Name, ovnNs) | |||
| err = e2epod.DeletePodWithWaitByName(context.TODO(), f.ClientSet, pod.Name, ovnNs) | |||
| framework.ExpectNoError(err, fmt.Sprintf("failed to delete pod %s", pod.Name)) | |||
There was a problem hiding this comment.
LOL, this is causing e2e failures because of a bug in e2epod.DeletePodWithWaitByName; it deletes the pod, and then loops until no pod with the given name exists. But if something else recreates the pod immediately, it doesn't notice that the pod that exists isn't the same one as the one it deleted.
(So, without the ExpectNoError check, the failure mode would be "it deletes the pod successfully, then gets confused and loops for 5 minutes and then the function returns an error, but we ignore the error and keep going (which is fine, because the pod we meant to delete was deleted)", whereas with this PR, the failure mode is "it deletes the pod successfully, then gets confused and loops for 5 minutes and then the function returns an error, which causes us to fail the test".)
|
Filed kubernetes/kubernetes#126785. Let's see what happens there. This bug may be affecting other places where we kill control-plane pods... |
No consensus on a short-term fix, so I'll just fix this locally. |
danwinship
dismissed stale reviews from tssurya and martinkennelly
via
55c72d9
danwinship
force-pushed
the
e2e-no-error
branch
from
1a0c8d8
to
55c72d9
Compare
danwinship
force-pushed
the
e2e-no-error
branch
2 times, most recently
from
9f75c2d
to
514f3a6
Compare
…t restarted. Signed-off-by: Dan Winship <danwinship@redhat.com>
Signed-off-by: Dan Winship <danwinship@redhat.com> Co-authored-by: Martin Kennelly <mkennell@redhat.com>
danwinship
force-pushed
the
e2e-no-error
branch
from
514f3a6
to
46f2293
Compare
What this PR does and why is it needed
More fixes pulled out of @martinkennelly 's branch to start adding support for running ovn-k e2e downstream in OCP. This just takes a bunch of places where the test cases were doing something and ignoring possible errors, and changes them to assert that an error didn't occur instead.
(In theory, this should not result in us discovering any new bugs in the existing code, since if something actually went wrong, the test would presumably fail later on anyway. It just means that in case of future bugs, the tests should fail closer to the point where something actually went wrong, rather than failing more mysteriously later.)
Does this PR introduce a user-facing change?