-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Make kube-proxy default to iptables (regression) #20464
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ package app | |
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"net" | ||
"net/http" | ||
_ "net/http/pprof" | ||
|
@@ -58,6 +59,7 @@ type ProxyServer struct { | |
Broadcaster record.EventBroadcaster | ||
Recorder record.EventRecorder | ||
Conntracker Conntracker // if nil, ignored | ||
ProxyMode string | ||
} | ||
|
||
const ( | ||
|
@@ -83,6 +85,7 @@ func NewProxyServer( | |
broadcaster record.EventBroadcaster, | ||
recorder record.EventRecorder, | ||
conntracker Conntracker, | ||
proxyMode string, | ||
) (*ProxyServer, error) { | ||
return &ProxyServer{ | ||
Client: client, | ||
|
@@ -92,6 +95,7 @@ func NewProxyServer( | |
Broadcaster: broadcaster, | ||
Recorder: recorder, | ||
Conntracker: conntracker, | ||
ProxyMode: proxyMode, | ||
}, nil | ||
} | ||
|
||
|
@@ -248,7 +252,7 @@ func NewProxyServerDefault(config *options.ProxyServerConfig) (*ProxyServer, err | |
|
||
conntracker := realConntracker{} | ||
|
||
return NewProxyServer(client, config, iptInterface, proxier, eventBroadcaster, recorder, conntracker) | ||
return NewProxyServer(client, config, iptInterface, proxier, eventBroadcaster, recorder, conntracker, proxyMode) | ||
} | ||
|
||
// Run runs the specified ProxyServer. This should never exit (unless CleanupAndExit is set). | ||
|
@@ -265,8 +269,11 @@ func (s *ProxyServer) Run() error { | |
|
||
s.Broadcaster.StartRecordingToSink(s.Client.Events("")) | ||
|
||
// Start up Healthz service if requested | ||
// Start up a webserver if requested | ||
if s.Config.HealthzPort > 0 { | ||
http.HandleFunc("/proxyMode", func(w http.ResponseWriter, r *http.Request) { | ||
fmt.Fprintf(w, "%s", s.ProxyMode) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we actually store what getProxyMode returns back into this? (I haven't checked) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More directly, this will say userspace if we don't have the right iptables version right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The NewProxyServer() call sets it. |
||
}) | ||
go util.Until(func() { | ||
err := http.ListenAndServe(s.Config.HealthzBindAddress+":"+strconv.Itoa(s.Config.HealthzPort), nil) | ||
if err != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,6 +197,10 @@ func (config *KubeProxyTestConfig) hitNodePort(epCount int) { | |
config.dialFromNode("udp", node2_IP, nodeUdpPort, tries, epCount) | ||
By("dialing(http) node1 --> node2:nodeHttpPort") | ||
config.dialFromNode("http", node2_IP, nodeHttpPort, tries, epCount) | ||
|
||
By("checking kube-proxy URLs") | ||
config.getSelfURL("/healthz", "ok") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this readiness? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To what end? The kube-proxy isn't behind a Service, so not being ready won't actually prevent anything from happening or enable anything, will it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can poll for ready though, from the test. I think the reboot tests do this and if you don't have a probe the kubelet just starts you off as ready. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this test does NO retries or readiness, I'd like to defer that On Tue, Feb 2, 2016 at 1:36 PM, Prashanth B notifications@github.com
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with the defer. The reboot test will poll till all pods are ready before declaring success. |
||
config.getSelfURL("/proxyMode", "iptables") // the default | ||
} | ||
|
||
func (config *KubeProxyTestConfig) hitEndpoints() { | ||
|
@@ -252,6 +256,13 @@ func (config *KubeProxyTestConfig) dialFromNode(protocol, targetIP string, targe | |
Expect(strconv.Atoi(strings.TrimSpace(stdout))).To(BeNumerically("==", expectedCount)) | ||
} | ||
|
||
func (config *KubeProxyTestConfig) getSelfURL(path string, expected string) { | ||
cmd := fmt.Sprintf("curl -s --connect-timeout 1 http://localhost:10249%s", path) | ||
By(fmt.Sprintf("Getting kube-proxy self URL %s", path)) | ||
stdout := RunHostCmdOrDie(config.f.Namespace.Name, config.hostTestContainerPod.Name, cmd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't retry and I don't think our kube-proxy has a readiness probe. So there's a chance the server isn't up if we run after a reboot test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of this test retries. Different issue. |
||
Expect(strings.Contains(stdout, expected)).To(BeTrue()) | ||
} | ||
|
||
func (config *KubeProxyTestConfig) createNetShellPodSpec(podName string, node string) *api.Pod { | ||
pod := &api.Pod{ | ||
TypeMeta: unversioned.TypeMeta{ | ||
|
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.
Can you set explicitly to componentconfig.ProxyModeIPTables? That will show the correct defaulting in the --help output
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.
I think the reason to leave it unset is to allow that to mean "use what's best", which in this case translates to iptables. Also iptables auto downgrades to userspace if the wrong version of iptables is present. I agree, we need a clearer way to surface this. And a better way to test it.
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.
Ok, it's probably worth adding a BestAvailable ProxyMode here. The doc is already wrong.
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.
What is wrong in the doc?
On Mon, Feb 1, 2016 at 10:21 PM, Mike Danese notifications@github.com
wrote:
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.
I think he's referring to https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/componentconfig/types.go#L68
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.
ahh, yes, will fix.