vcluster icon indicating copy to clipboard operation
vcluster copied to clipboard

keep internalIP for fakeKubelet node, since some e2e test will get it…

Open caden2016 opened this issue 2 years ago • 5 comments

keep internalIP for fakeKubelet node, since some e2e test will get it as the access IP of some NodePort service and users may use it like e2e test case.

What issue type does this pull request address? (keep at least one, remove the others) /kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible) resolves # when i use vcluster(v0.14.0 + k8s v1.20.15) and host cluster (k8s v1.19.9) to run k8s e2e conformance of v1.20.15, I found that some case will failed because the case try to get the node internalIP as the access IP of NodePort service. I think user application run on the vcluster may do it the same as test case does. so I try to fix this by keeping internalIP for fakeKubelet node.

k8s v1.20.15 e2e conformance test case run in vcluster(v0.14.0 + k8s v1.20.15) failed list as follows:

  • [sig-scheduling] SchedulerPredicates [Serial] validates that there exists conflict between pods with same hostPort and protocol but one using 0.0.0.0 hostIP [Conformance]
  • [sig-network] Services should have session affinity timeout work for NodePort service [LinuxOnly] [Conformance]
  • [sig-network] Services should have session affinity work for NodePort service [LinuxOnly] [Conformance]
  • [sig-network] Services should be able to switch session affinity for NodePort service [LinuxOnly] [Conformance]
  • [sig-scheduling] SchedulerPredicates [Serial] validates that there is no conflict between pods with same hostPort but different hostIP and protocol [Conformance]

Please provide a short message that should be published in the vcluster release notes Fixed an issue where vcluster failed running k8s e2e conformance test

What else do we need to know?

caden2016 avatar Mar 01 '23 10:03 caden2016

@caden2016 Hello, thank you for the PR! We did not see this problem when running Kubernetes 1.26 conformance tests, the "Hostname" type of address seems to have been sufficient.

The problem with leaving the "internalIP" address is that it would not correctly serve the kubelet metrics and stats. Do you have examples of apps that rely solely on the "internalIP"?

matskiv avatar Mar 13 '23 09:03 matskiv

@matskiv thanks for your review. I found in running Kubernetes 1.26 conformance tests you set disable-fake-kubelets, this will not works for metrics-server in vcluster. I just want to see the e2e conformance will pass with fake kubelets, so that metrics-server works too. so I think to leaving the "internalIP" and for metrics-server we can suggest by prefer Hostname first --kubelet-preferred-address-types=Hostname,InternalIP . what do you think?

caden2016 avatar Mar 15 '23 05:03 caden2016

@caden2016 Good point! But I think there might be some "low-level" conformance tests that won't pass with the fake kubelets. I will run them and see how that goes. The --kubelet-preferred-address-types=Hostname is also a good idea :+1:

matskiv avatar Mar 15 '23 08:03 matskiv

@caden2016 we can not merge this change because it will break some applications that collect metrics from the nodes. For example Prometheus prefers InternalIP when discovering Node address, and that would mean that it will not display the metrics correctly in the vcluster. We are open to changes that would support all use cases, but for now, we will wait until there are reports of specific apps that are not working because of the missing InternalIP.

matskiv avatar Mar 16 '23 13:03 matskiv

@matskiv ok, I know what you warry about (like Prometheus example). I pull this is try to fix conformance test when vcluster with fake-kubelet. Now I didn't found any reports of specific apps that are not working because of the missing InternalIP, except e2e conformance test.

caden2016 avatar Mar 17 '23 02:03 caden2016

Outdated

heiko-braun avatar Mar 14 '24 14:03 heiko-braun