diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index cdda9735962..abada5d5efb 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -39,7 +39,7 @@ type HandlerRunner interface { // RuntimeHelper wraps kubelet to make container runtime // able to get necessary informations like the RunContainerOptions, DNS settings. type RuntimeHelper interface { - GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*RunContainerOptions, error) + GenerateRunContainerOptions(pod *api.Pod, container *api.Container, podIP string) (*RunContainerOptions, error) GetClusterDNS(pod *api.Pod) (dnsServers []string, dnsSearches []string, err error) } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 1eee8ecef89..0c640bd764b 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -1491,7 +1491,7 @@ func (dm *DockerManager) applyOOMScoreAdj(container *api.Container, containerInf // Run a single container from a pod. Returns the docker container ID // If do not need to pass labels, just pass nil. -func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Container, netMode, ipcMode, pidMode string, restartCount int) (kubecontainer.ContainerID, error) { +func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Container, netMode, ipcMode, pidMode, podIP string, restartCount int) (kubecontainer.ContainerID, error) { start := time.Now() defer func() { metrics.ContainerManagerLatency.WithLabelValues("runContainerInPod").Observe(metrics.SinceInMicroseconds(start)) @@ -1502,7 +1502,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe glog.Errorf("Can't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err) } - opts, err := dm.runtimeHelper.GenerateRunContainerOptions(pod, container) + opts, err := dm.runtimeHelper.GenerateRunContainerOptions(pod, container, podIP) if err != nil { return kubecontainer.ContainerID{}, fmt.Errorf("GenerateRunContainerOptions: %v", err) } @@ -1635,7 +1635,7 @@ func (dm *DockerManager) createPodInfraContainer(pod *api.Pod) (kubecontainer.Do } // Currently we don't care about restart count of infra container, just set it to 0. - id, err := dm.runContainerInPod(pod, container, netNamespace, getIPCMode(pod), getPidMode(pod), 0) + id, err := dm.runContainerInPod(pod, container, netNamespace, getIPCMode(pod), getPidMode(pod), "", 0) if err != nil { return "", kubecontainer.ErrRunContainer, err.Error() } @@ -1832,6 +1832,19 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubec } } + // We pass the value of the podIP down to runContainerInPod, which in turn + // passes it to various other functions, in order to facilitate + // functionality that requires this value (hosts file and downward API) + // and avoid races determining the pod IP in cases where a container + // requires restart but the podIP isn't in the status manager yet. + // + // We default to the IP in the passed-in pod status, and overwrite it if the + // infra container needs to be (re)started. + podIP := "" + if podStatus != nil { + podIP = podStatus.IP + } + // If we should create infra container then we do it first. podInfraContainerID := containerChanges.InfraContainerId if containerChanges.StartInfraContainer && (len(containerChanges.ContainersToStart) > 0) { @@ -1884,9 +1897,8 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubec } } - // Find the pod IP after starting the infra container in order to expose - // it safely via the downward API without a race and be able to use podIP in kubelet-managed /etc/hosts file. - pod.Status.PodIP = dm.determineContainerIP(pod.Name, pod.Namespace, podInfraContainer) + // Overwrite the podIP passed in the pod status, since we just started the infra container. + podIP = dm.determineContainerIP(pod.Name, pod.Namespace, podInfraContainer) } } @@ -1934,7 +1946,7 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubec // and IPC namespace. PID mode cannot point to another container right now. // See createPodInfraContainer for infra container setup. namespaceMode := fmt.Sprintf("container:%v", podInfraContainerID) - _, err = dm.runContainerInPod(pod, container, namespaceMode, namespaceMode, getPidMode(pod), restartCount) + _, err = dm.runContainerInPod(pod, container, namespaceMode, namespaceMode, getPidMode(pod), podIP, restartCount) if err != nil { startContainerResult.Fail(kubecontainer.ErrRunContainer, err.Error()) // TODO(bburns) : Perhaps blacklist a container after N failures? diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 09e4826fde8..c5a6274157a 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -67,7 +67,7 @@ var _ kubecontainer.RuntimeHelper = &fakeRuntimeHelper{} var testPodContainerDir string -func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) { +func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *api.Pod, container *api.Container, podIP string) (*kubecontainer.RunContainerOptions, error) { var opts kubecontainer.RunContainerOptions var err error if len(container.TerminationMessagePath) != 0 { diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index a06021bcb29..2d4338cfef7 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1225,14 +1225,14 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap) return nil } -func makeMounts(pod *api.Pod, podDir string, container *api.Container, hostName, hostDomain string, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) { +func makeMounts(pod *api.Pod, podDir string, container *api.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) { // Kubernetes only mounts on /etc/hosts if : // - container does not use hostNetwork and // - container is not a infrastructure(pause) container // - container is not already mounting on /etc/hosts // When the pause container is being created, its IP is still unknown. Hence, PodIP will not have been set. - mountEtcHostsFile := (pod.Spec.SecurityContext == nil || !pod.Spec.SecurityContext.HostNetwork) && len(pod.Status.PodIP) > 0 - glog.V(3).Infof("container: %v/%v/%v podIP: %q creating hosts mount: %v", pod.Namespace, pod.Name, container.Name, pod.Status.PodIP, mountEtcHostsFile) + mountEtcHostsFile := (pod.Spec.SecurityContext == nil || !pod.Spec.SecurityContext.HostNetwork) && len(podIP) > 0 + glog.V(3).Infof("container: %v/%v/%v podIP: %q creating hosts mount: %v", pod.Namespace, pod.Name, container.Name, podIP, mountEtcHostsFile) mounts := []kubecontainer.Mount{} for _, mount := range container.VolumeMounts { mountEtcHostsFile = mountEtcHostsFile && (mount.MountPath != etcHostsPath) @@ -1259,7 +1259,7 @@ func makeMounts(pod *api.Pod, podDir string, container *api.Container, hostName, }) } if mountEtcHostsFile { - hostsMount, err := makeHostsMount(podDir, pod.Status.PodIP, hostName, hostDomain) + hostsMount, err := makeHostsMount(podDir, podIP, hostName, hostDomain) if err != nil { return nil, err } @@ -1361,7 +1361,7 @@ func generatePodHostNameAndDomain(pod *api.Pod, clusterDomain string) (string, s // GenerateRunContainerOptions generates the RunContainerOptions, which can be used by // the container runtime to set parameters for launching a container. -func (kl *Kubelet) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) { +func (kl *Kubelet) GenerateRunContainerOptions(pod *api.Pod, container *api.Container, podIP string) (*kubecontainer.RunContainerOptions, error) { var err error opts := &kubecontainer.RunContainerOptions{CgroupParent: kl.cgroupRoot} hostname, hostDomainName := generatePodHostNameAndDomain(pod, kl.clusterDomain) @@ -1382,11 +1382,11 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *api.Pod, container *api.Cont } } - opts.Mounts, err = makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, vol) + opts.Mounts, err = makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, vol) if err != nil { return nil, err } - opts.Envs, err = kl.makeEnvironmentVariables(pod, container) + opts.Envs, err = kl.makeEnvironmentVariables(pod, container, podIP) if err != nil { return nil, err } @@ -1463,7 +1463,7 @@ func (kl *Kubelet) getServiceEnvVarMap(ns string) (map[string]string, error) { } // Make the service environment variables for a pod in the given namespace. -func (kl *Kubelet) makeEnvironmentVariables(pod *api.Pod, container *api.Container) ([]kubecontainer.EnvVar, error) { +func (kl *Kubelet) makeEnvironmentVariables(pod *api.Pod, container *api.Container, podIP string) ([]kubecontainer.EnvVar, error) { var result []kubecontainer.EnvVar // Note: These are added to the docker.Config, but are not included in the checksum computed // by dockertools.BuildDockerName(...). That way, we can still determine whether an @@ -1510,7 +1510,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *api.Pod, container *api.Contain // Step 1b: resolve alternate env var sources switch { case envVar.ValueFrom.FieldRef != nil: - runtimeVal, err = kl.podFieldSelectorRuntimeValue(envVar.ValueFrom.FieldRef, pod) + runtimeVal, err = kl.podFieldSelectorRuntimeValue(envVar.ValueFrom.FieldRef, pod, podIP) if err != nil { return result, err } @@ -1557,14 +1557,14 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *api.Pod, container *api.Contain return result, nil } -func (kl *Kubelet) podFieldSelectorRuntimeValue(fs *api.ObjectFieldSelector, pod *api.Pod) (string, error) { +func (kl *Kubelet) podFieldSelectorRuntimeValue(fs *api.ObjectFieldSelector, pod *api.Pod, podIP string) (string, error) { internalFieldPath, _, err := api.Scheme.ConvertFieldLabel(fs.APIVersion, "Pod", fs.FieldPath, "") if err != nil { return "", err } switch internalFieldPath { case "status.podIP": - return pod.Status.PodIP, nil + return podIP, nil } return fieldpath.ExtractFieldPathAsString(pod, internalFieldPath) } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 887b86fe830..63391474c47 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -728,7 +728,7 @@ func TestMakeVolumeMounts(t *testing.T) { }, } - mounts, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", podVolumes) + mounts, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes) expectedMounts := []kubecontainer.Mount{ { @@ -1189,7 +1189,7 @@ func TestDNSConfigurationParams(t *testing.T) { for i, pod := range pods { var err error kubelet.volumeManager.SetVolumes(pod.UID, make(kubecontainer.VolumeMap, 0)) - options[i], err = kubelet.GenerateRunContainerOptions(pod, &api.Container{}) + options[i], err = kubelet.GenerateRunContainerOptions(pod, &api.Container{}, "") if err != nil { t.Fatalf("failed to generate container options: %v", err) } @@ -1210,7 +1210,7 @@ func TestDNSConfigurationParams(t *testing.T) { kubelet.resolverConfig = "/etc/resolv.conf" for i, pod := range pods { var err error - options[i], err = kubelet.GenerateRunContainerOptions(pod, &api.Container{}) + options[i], err = kubelet.GenerateRunContainerOptions(pod, &api.Container{}, "") if err != nil { t.Fatalf("failed to generate container options: %v", err) } @@ -1715,9 +1715,9 @@ func TestMakeEnvironmentVariables(t *testing.T) { Name: "dapi-test-pod-name", }, } - testPod.Status.PodIP = "1.2.3.4" + podIP := "1.2.3.4" - result, err := kl.makeEnvironmentVariables(testPod, tc.container) + result, err := kl.makeEnvironmentVariables(testPod, tc.container, podIP) if err != nil { t.Errorf("[%v] Unexpected error: %v", tc.name, err) } diff --git a/pkg/kubelet/rkt/fake_rkt_interface_test.go b/pkg/kubelet/rkt/fake_rkt_interface_test.go index 25e73e4b246..de71693622a 100644 --- a/pkg/kubelet/rkt/fake_rkt_interface_test.go +++ b/pkg/kubelet/rkt/fake_rkt_interface_test.go @@ -153,7 +153,7 @@ type fakeRuntimeHelper struct { err error } -func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) { +func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *api.Pod, container *api.Container, podIP string) (*kubecontainer.RunContainerOptions, error) { return nil, fmt.Errorf("Not implemented") } diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index ac0f6c9a90b..4d1119a9991 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -579,7 +579,8 @@ func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, c api.Container, pullSecrets [ return err } - opts, err := r.runtimeHelper.GenerateRunContainerOptions(pod, &c) + // TODO: determine how this should be handled for rkt + opts, err := r.runtimeHelper.GenerateRunContainerOptions(pod, &c, "") if err != nil { return err }