Merge pull request #29171 from euank/determine-container-ip-args

Automatic merge from submit-queue

docker_manager: Correct determineContainerIP args

This could result in the network plugin not retrieving the pod ip in a
call to SyncPod when using the `exec` network plugin.
The CNI and kubenet network plugins ignore the name/namespace arguments,
so they are not impacted by this bug.

I verified the second included test failed prior to correcting the
argument order.

Fixes #29161 

cc @yujuhong

pkg/kubelet/dockertools/docker_manager.go

@@ -2031,7 +2031,8 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubec
 			}
 
 			// Overwrite the podIP passed in the pod status, since we just started the infra container.
-			podIP = dm.determineContainerIP(pod.Name, pod.Namespace, podInfraContainer)
+			podIP = dm.determineContainerIP(pod.Namespace, pod.Name, podInfraContainer)
+			glog.V(4).Infof("Determined pod ip after infra change: %q: %q", format.Pod(pod), podIP)
 		}
 	}
 

pkg/kubelet/dockertools/docker_manager_test.go

@@ -19,6 +19,7 @@ package dockertools
 import (
 	"fmt"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"os"
 	"path"
@@ -34,6 +35,7 @@ import (
 	dockertypes "github.com/docker/engine-api/types"
 	dockercontainer "github.com/docker/engine-api/types/container"
 	dockerstrslice "github.com/docker/engine-api/types/strslice"
+	"github.com/golang/mock/gomock"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/kubernetes/pkg/api"
@@ -43,6 +45,7 @@ import (
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
 	"k8s.io/kubernetes/pkg/kubelet/network"
+	"k8s.io/kubernetes/pkg/kubelet/network/mock_network"
 	nettest "k8s.io/kubernetes/pkg/kubelet/network/testing"
 	proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
 	"k8s.io/kubernetes/pkg/kubelet/types"
@@ -2214,3 +2217,91 @@ func TestPruneInitContainers(t *testing.T) {
 		t.Fatal(fake.Removed)
 	}
 }
+
+func TestGetPodStatusFromNetworkPlugin(t *testing.T) {
+	const (
+		containerID      = "123"
+		infraContainerID = "9876"
+		fakePodIP        = "10.10.10.10"
+	)
+	dm, fakeDocker := newTestDockerManager()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	fnp := mock_network.NewMockNetworkPlugin(ctrl)
+	dm.networkPlugin = fnp
+
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			UID:       "12345678",
+			Name:      "foo",
+			Namespace: "new",
+		},
+		Spec: api.PodSpec{
+			Containers: []api.Container{{Name: "container"}},
+		},
+	}
+
+	fakeDocker.SetFakeRunningContainers([]*FakeContainer{
+		{
+			ID:      containerID,
+			Name:    "/k8s_container_foo_new_12345678_42",
+			Running: true,
+		},
+		{
+			ID:      infraContainerID,
+			Name:    "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_42",
+			Running: true,
+		},
+	})
+
+	fnp.EXPECT().Name().Return("someNetworkPlugin")
+	fnp.EXPECT().GetPodNetworkStatus("new", "foo", kubecontainer.DockerID(infraContainerID).ContainerID()).Return(&network.PodNetworkStatus{IP: net.ParseIP(fakePodIP)}, nil)
+
+	podStatus, err := dm.GetPodStatus(pod.UID, pod.Name, pod.Namespace)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if podStatus.IP != fakePodIP {
+		t.Errorf("Got wrong ip, expected %v, got %v", fakePodIP, podStatus.IP)
+	}
+}
+
+func TestSyncPodGetsPodIPFromNetworkPlugin(t *testing.T) {
+	const (
+		containerID      = "123"
+		infraContainerID = "9876"
+		fakePodIP        = "10.10.10.10"
+	)
+	dm, fakeDocker := newTestDockerManager()
+	dm.podInfraContainerImage = "pod_infra_image"
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	fnp := mock_network.NewMockNetworkPlugin(ctrl)
+	dm.networkPlugin = fnp
+
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			UID:       "12345678",
+			Name:      "foo",
+			Namespace: "new",
+		},
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{Name: "bar"},
+			},
+		},
+	}
+
+	// Can be called multiple times due to GetPodStatus
+	fnp.EXPECT().Name().Return("someNetworkPlugin").AnyTimes()
+	fnp.EXPECT().GetPodNetworkStatus("new", "foo", gomock.Any()).Return(&network.PodNetworkStatus{IP: net.ParseIP(fakePodIP)}, nil).AnyTimes()
+	fnp.EXPECT().SetUpPod("new", "foo", gomock.Any()).Return(nil)
+
+	runSyncPod(t, dm, fakeDocker, pod, nil, false)
+	verifyCalls(t, fakeDocker, []string{
+		// Create pod infra container.
+		"create", "start", "inspect_container", "inspect_container",
+		// Create container.
+		"create", "start", "inspect_container",
+	})
+}