fix e2e conformance test predicates conflict hostport

[aojea]

What type of PR is this?

/kind cleanup
/kind failing-test

What this PR does / why we need it:

The e2e test, included as part of Conformance,
"validates that there is no conflict between
pods with same hostPort but different hostIP and protocol"
was only testing that the pods were scheduled without conflict
but was never testing the functionality.

The test should check that pods with containers forwarding the same
hostPort can be scheduled without conflict, and that those exposed
HostPort are forwarding the ports to the corresponding pods.

Which issue(s) this PR fixes:

Special notes for your reviewer:

The Conformance test "validates that there is no conflict between pods with same hostPort but different hostIP and protocol" now validates the connectivity to each hostPort, in addition to the functionality.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

@aojea: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions 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.

[aojea]

/assign @BenTheElder @spiffxp @johnbelamaric

[aojea]

/retest

[aojea]

/retest
🤔 this only touches e2e

[aojea]

for context, the actual test is wrong because it succeeds despite the pods are not doing portmapping, we hit that in Openshift CI

[spiffxp]

/area conformance
/approve
/lgtm
/milestone v1.20
FYI @kubernetes/ci-signal

I'm allowing this in because we're not at test freeze, but if we see an increase in flakiness in this test over the weekend, or in PR's next week, we should revert this rather than try to fix-forward.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, spiffxp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• test/e2e/scheduling/OWNERS [spiffxp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ravisantoshgudimetla]

I think this is causing a failure in case of Windows as hostNetwork is not supported on Windows hosts.

Ref: https://testgrid.k8s.io/sig-windows-releases#aks-engine-azure-windows-master-staging-serial-slow

@aojea - I think the job of scheduler is done when the pod has properly assigned a node, actual testing of networking to me should have been a networking test in test/e2e/network instead of where we have it currentlytest/e2e/scheduler. I think a decent compromise would be to skip running the ping/curl tests in case of Windows for now. Let me know if you want me to handle it.

This actually brings another interesting point. This is another scenario where running Windows tests earlier (atleast a subset) would have caught the problem, can we move to having the Windows tests as optional tests as a starting point, considering we have been deflaking the CI in the past few weeks.

cc @jsturtevant @marosset @m2 @BenTheElder

[aojea]

@aojea - I think the job of scheduler is done when the pod has properly assigned a node, actual testing of networking to me should have been a networking test in test/e2e/network instead of where we have it currentlytest/e2e/scheduler. I think a decent compromise would be to skip running the ping/curl tests in case of Windows for now. Let me know if you want me to handle it.

This is an e2e and conformance test, I personally think that it should verify that ALL works, not only the scheduler job. If the scheduler is able to schedule the pods, but once they are scheduled, they are not working or not exposing the ports, it is not useful for any user :)

I'm not familiar with windows at all but that testgrid link that you pasted only shows jobs running 56 tests, and not being able to run hostnetwork pods makes me think that it will fail in a lot of e2e and conformance test.

I personally think that we should not modify a conformance test to have a different behaviour per platform, and I would suggest to create new scheduler test with the previous behavior if this is really important for windows, but this is my opinion, I have to defer to the sig-testing and conformance people Ben, @spiffxp , @dims, @johnbelamaric ,.

[ravisantoshgudimetla]

This is an e2e and conformance test, I personally think that it should verify that ALL works, not only the scheduler job. If the scheduler is able to schedule the pods, but once they are scheduled, they are not working or not exposing the ports, it is not useful for any user :)

Right, I am not saying we shouldn't test if the exposed ports are working or not. I am just saying that it's not part of scheduler job and perhaps this test should live elsewhere.

I'm not familiar with windows at all but that testgrid link that you pasted only shows jobs running 56 tests, and not being able to run hostnetwork pods makes me think that it will fail in a lot of e2e and conformance test.

Yeah, the dashboard link I shared is for serial tests

I personally think that we should not modify a conformance test to have a different behaviour per platform, and I would suggest to create new scheduler test with the previous behavior if this is really important for windows, but this is my opinion, I have to defer to the sig-testing and conformance people Ben, @spiffxp , @dims, @johnbelamaric ,.

I agree but hostNetwork is not clearly supported on Windows, I think the question is what should we do in these type of scenarios where different platforms behave differently in case of conformance tests.

[aojea]

Ahh ok, thanks for the clarification, agree with you on the points and the questions that you are raising 😄

[marosset]

@adelina-t - She was also looking at some hostport issues raised by tests for Windows nodes.

[BenTheElder]

Firstly: Windows "conformance" does not exist as of yet. Conformance testing targets linux.
If windows cannot support a test, we have existing tags for filtering out linux only tests.

The conformance test should not alter behavior based on platform.
For one thing the existing e2e.test platform detection mechanism is extremely broken / misguided, clusters can be linux + windows (in fact all of them are, you can't run the control plane on windows) but it detects the entire cluster are linux or windows when it's actually nodes that may be windows.

AFAIK hostNetwork is fair game for conformance tests but that would be up to the conformance project, if it is fair game and it can't work on windows then the linux only tag is appropriate rather than altering the behavior.

We are still trying to reduce the reliance on presubmit. AIUI windows tests are already running in post-submit, but people need to monitor the results.

[marosset]

AIUI windows tests are already running in post-submit, but people need to monitor the results.

@BenTheElder we've added a few pre-submit jobs to verify Windows functionality but currently they must be triggered manually.
A majority of the test coverage is covered post-submit tho.

https://github.com/kubernetes/test-infra/blob/750d12f6f39e286c8c590eb0c28d50b92cb33e02/config/jobs/kubernetes-sigs/sig-windows/sig-windows-config.yaml#L22 contains the triggers / job names.

[BenTheElder]

👍 thanks, I also see some of the periodics have email alerting enabled 🎉
The alerting specifics also have a few knobs (like recency considered) that can be tuned as needed. Unfortunately many jobs in general do not have anyone subscribed to the alerts, so we lean pretty heavily on the release team's CI signal group ..
https://github.com/kubernetes/test-infra/blob/750d12f6f39e286c8c590eb0c28d50b92cb33e02/config/jobs/kubernetes-sigs/sig-windows/sig-windows-config.yaml#L366

[ravisantoshgudimetla]

Firstly: Windows "conformance" does not exist as of yet. Conformance testing targets linux.
If windows cannot support a test, we have existing tags for filtering out linux only tests.

Thank you for the clarification @BenTheElder. I was under the impression that conformance tests are for both Linux and Windows.

For one thing the existing e2e.test platform detection mechanism is extremely broken / misguided, clusters can be linux + windows (in fact all of them are, you can't run the control plane on windows) but it detects the entire cluster are linux or windows when it's actually nodes that may be windows.

To be clear, I was thinking of using --node-os-distro=windows and then use NodeDistroIs flag in our e2e code but I think it violates the point you made earlier that conformance is for linux only. So, I am fine making the test Linux only for now.

I think we can change the ownership of this test to networking in a separate PR

[liggitt]

is the mechanism this used to verify connectivity required/guaranteed to work on all conformant clusters?

[aojea]

is the mechanism this used to verify connectivity required/guaranteed to work on all conformant clusters?

we found that this test was passing for weeks despite it must be failing, because one of the pods failed to star with a conflict, so it restarted, cleaning the other hostports and removing the conflict. The result is that the pods are scheduled but they are not working as expected, you can not reach them in the HostPort expected, well, only one of them.

You have to verify that the host ports exposed are really working, so you can be sure that the each hostPort is being forwarded to the corresponding HostIP,

[aojea]

I have a followup PR to move the ownership to sig-network #98299