skip not ready nodes in networking test #9676
Conversation
|
LGTM |
| @@ -1144,6 +1144,21 @@ func waitForNodeToBeNotReady(c *client.Client, name string, timeout time.Duratio | |||
| return waitForNodeToBe(c, name, false, timeout) | |||
| } | |||
|
|
|||
| func isNodeReadySetAsExpected(node *api.Node, wantReady bool) bool { | |||
There was a problem hiding this comment.
suggest just "isNodeReady" and omit the wantReady parameter. YAGNI
marekbiskup
force-pushed
the
skipNotReadyNodesInNetworkTest
branch
from
9d09d65
to
d89e129
Compare
|
GCE e2e build/test failed for commit d89e129. |
|
@quinton-hoole, Is Jenkins stable? Shall I take the results seriously? |
|
LGTM - I think the failures are not related to this PR. |
lavalamp
added
the
lgtm
|
Agreed, LGTM |
|
Please can this be merged soon-ish since I am blocked on it for fixing the Elasticsearch logging test? Thanks. |
skip not ready nodes in networking test
|
I suppose that, previously, the test did no checks with one node, and now it is failing. That's the reason for @jayunit100's objections. @lavalamp, you said this test cannot work with 1 node. Maybe I can create two pods on the same node, if there is only one? Will this test also be reasonable (I understand it will be limited). |
|
The whole point of this test is to verify that the network works between two pods on different machines. I can see making another version of the test that makes sure the network works on a single machine, but that would be a different test. |
|
Since when? This test generally was used to confirm that all nodes could contact one another over the service endpoint... Iirc that is still a valid test on a single node, and indeed, by breaking ip tables or other underlying services you can toggle the passing of the test on or off , making it a great test for 1node dev clusters. |
|
Since I wrote this test originally, I feel qualified to claim that the purpose of this test is to verify that packets correctly travel between nodes. :) Please note that the test load reads the endpoints and tries to dial the other pods directly. https://github.com/GoogleCloudPlatform/kubernetes/blob/master/contrib/for-tests/network-tester/webserver.go#L218 The service is only used for discovery. |
|
I do see that there may be value in a version of this test that runs on a single node. But in our continuous e2e runs, for GCE, GKE, and other cloud providers--we absolutely need to be failing if we can't get packets between two nodes. |
|
touche` :) but indeed, it also contacts itself through the service endpoint. hence it may have been written for the purpose of 2 nodes, but its an awesome test even on a single node ! security, networking, and certificates are all tested by the single node networking test. You're baby is more useful than you originally thought ! :) |
|
Yes, it exercises a lot of useful stuff. I'm fine with, e.g., gating the two node requirement with a comparison to the cloud provider. |
|
ok awesome . that is a good way to do it. shall i bundle that in with #9052 ? |
|
I created #10012 for the purpose of allowing the test to run on a (local) single node |
fixes #9345