Merge pull request #18305 from yujuhong/podfullname

pkg/kubelet/container/helpers.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/client/record"
+	"k8s.io/kubernetes/pkg/kubelet/util/format"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util"
 	"k8s.io/kubernetes/third_party/golang/expansion"
@@ -44,8 +45,6 @@ type RunContainerOptionsGenerator interface {
 // ShouldContainerBeRestarted checks whether a container needs to be restarted.
 // TODO(yifan): Think about how to refactor this.
 func ShouldContainerBeRestarted(container *api.Container, pod *api.Pod, podStatus *PodStatus) bool {
-	podFullName := GetPodFullName(pod)
-
 	// Get all dead container status.
 	var resultStatus []*ContainerStatus
 	for _, containerStatus := range podStatus.ContainerStatuses {
@@ -57,14 +56,14 @@ func ShouldContainerBeRestarted(container *api.Container, pod *api.Pod, podStatu
 	// Check RestartPolicy for dead container.
 	if len(resultStatus) > 0 {
 		if pod.Spec.RestartPolicy == api.RestartPolicyNever {
-			glog.V(4).Infof("Already ran container %q of pod %q, do nothing", container.Name, podFullName)
+			glog.V(4).Infof("Already ran container %q of pod %q, do nothing", container.Name, format.Pod(pod))
 			return false
 		}
 		if pod.Spec.RestartPolicy == api.RestartPolicyOnFailure {
 			// Check the exit code of last run. Note: This assumes the result is sorted
 			// by the created time in reverse order.
 			if resultStatus[0].ExitCode == 0 {
-				glog.V(4).Infof("Already successfully ran container %q of pod %q, do nothing", container.Name, podFullName)
+				glog.V(4).Infof("Already successfully ran container %q of pod %q, do nothing", container.Name, format.Pod(pod))
 				return false
 			}
 		}
@@ -74,8 +73,6 @@ func ShouldContainerBeRestarted(container *api.Container, pod *api.Pod, podStatu
 
 // TODO (random-liu) This should be removed soon after rkt implements GetPodStatus.
 func ShouldContainerBeRestartedOldVersion(container *api.Container, pod *api.Pod, podStatus *api.PodStatus) bool {
-	podFullName := GetPodFullName(pod)
-
 	// Get all dead container status.
 	var resultStatus []*api.ContainerStatus
 	for i, containerStatus := range podStatus.ContainerStatuses {
@@ -87,14 +84,14 @@ func ShouldContainerBeRestartedOldVersion(container *api.Container, pod *api.Pod
 	// Check RestartPolicy for dead container.
 	if len(resultStatus) > 0 {
 		if pod.Spec.RestartPolicy == api.RestartPolicyNever {
-			glog.V(4).Infof("Already ran container %q of pod %q, do nothing", container.Name, podFullName)
+			glog.V(4).Infof("Already ran container %q of pod %q, do nothing", container.Name, format.Pod(pod))
 			return false
 		}
 		if pod.Spec.RestartPolicy == api.RestartPolicyOnFailure {
 			// Check the exit code of last run. Note: This assumes the result is sorted
 			// by the created time in reverse order.
 			if resultStatus[0].State.Terminated.ExitCode == 0 {
-				glog.V(4).Infof("Already successfully ran container %q of pod %q, do nothing", container.Name, podFullName)
+				glog.V(4).Infof("Already successfully ran container %q of pod %q, do nothing", container.Name, format.Pod(pod))
 				return false
 			}
 		}

pkg/kubelet/kubelet.go

@@ -1595,8 +1595,6 @@ func (kl *Kubelet) makePodDataDirs(pod *api.Pod) error {
 }
 
 func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecontainer.Pod, updateType kubetypes.SyncPodType) (syncErr error) {
-	podFullName := kubecontainer.GetPodFullName(pod)
-	uid := pod.UID
 	start := time.Now()
 	var firstSeenTime time.Time
 	if firstSeenTimeStr, ok := pod.Annotations[kubetypes.ConfigFirstSeenAnnotationKey]; !ok {
@@ -1609,7 +1607,7 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 	defer func() {
 		status, err := kl.generatePodStatus(pod)
 		if err != nil {
-			glog.Errorf("Unable to generate status for pod with name %q and uid %q info with error(%v)", podFullName, uid, err)
+			glog.Errorf("Unable to generate status for pod %q with error(%v)", format.Pod(pod), err)
 			// Propagate the error upstream.
 			syncErr = err
 		} else {
@@ -1632,18 +1630,19 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 
 	// Create Mirror Pod for Static Pod if it doesn't already exist
 	if kubepod.IsStaticPod(pod) {
+		podFullName := kubecontainer.GetPodFullName(pod)
 		if mirrorPod != nil && !kl.podManager.IsMirrorPodOf(mirrorPod, pod) {
 			// The mirror pod is semantically different from the static pod. Remove
 			// it. The mirror pod will get recreated later.
-			glog.Errorf("Deleting mirror pod %q because it is outdated", podFullName)
+			glog.Errorf("Deleting mirror pod %q because it is outdated", format.Pod(mirrorPod))
 			if err := kl.podManager.DeleteMirrorPod(podFullName); err != nil {
-				glog.Errorf("Failed deleting mirror pod %q: %v", podFullName, err)
+				glog.Errorf("Failed deleting mirror pod %q: %v", format.Pod(mirrorPod), err)
 			}
 		}
 		if mirrorPod == nil {
-			glog.V(3).Infof("Creating a mirror pod for static pod %q", podFullName)
+			glog.V(3).Infof("Creating a mirror pod for static pod %q", format.Pod(pod))
 			if err := kl.podManager.CreateMirrorPod(pod); err != nil {
-				glog.Errorf("Failed creating a mirror pod %q: %v", podFullName, err)
+				glog.Errorf("Failed creating a mirror pod for %q: %v", format.Pod(pod), err)
 			}
 
 			_, ok := kl.podManager.GetMirrorPodByPod(pod)
@@ -1654,7 +1653,7 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 	}
 
 	if err := kl.makePodDataDirs(pod); err != nil {
-		glog.Errorf("Unable to make pod data directories for pod %q (uid %q): %v", podFullName, uid, err)
+		glog.Errorf("Unable to make pod data directories for pod %q: %v", format.Pod(pod), err)
 		return err
 	}
 
@@ -1663,8 +1662,8 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 	if err != nil {
 		ref, errGetRef := api.GetReference(pod)
 		if errGetRef == nil && ref != nil {
-			kl.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.FailedMountVolume, "Unable to mount volumes for pod %q: %v", podFullName, err)
-			glog.Errorf("Unable to mount volumes for pod %q: %v; skipping pod", podFullName, err)
+			kl.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.FailedMountVolume, "Unable to mount volumes for pod %q: %v", format.Pod(pod), err)
+			glog.Errorf("Unable to mount volumes for pod %q: %v; skipping pod", format.Pod(pod), err)
 			return err
 		}
 	}
@@ -1706,11 +1705,11 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 			Name:      pod.Name,
 			Namespace: pod.Namespace,
 		}
-		glog.V(3).Infof("Not generating pod status for new pod %q", podFullName)
+		glog.V(3).Infof("Not generating pod status for new pod %q", format.Pod(pod))
 	} else {
 		podStatusPtr, apiPodStatusPtr, err := kl.containerRuntime.GetPodStatusAndAPIPodStatus(pod)
 		if err != nil {
-			glog.Errorf("Unable to get status for pod %q (uid %q): %v", podFullName, uid, err)
+			glog.Errorf("Unable to get status for pod %q: %v", format.Pod(pod), err)
 			return err
 		}
 		apiPodStatus = *apiPodStatusPtr
@@ -1719,7 +1718,7 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 
 	pullSecrets, err := kl.getPullSecretsForPod(pod)
 	if err != nil {
-		glog.Errorf("Unable to get pull secrets for pod %q (uid %q): %v", podFullName, uid, err)
+		glog.Errorf("Unable to get pull secrets for pod %q: %v", format.Pod(pod), err)
 		return err
 	}
 
@@ -1912,8 +1911,7 @@ func (kl *Kubelet) cleanupTerminatedPods(pods []*api.Pod, runningPods []*kubecon
 				}
 			}
 			if found {
-				podFullName := kubecontainer.GetPodFullName(pod)
-				glog.V(5).Infof("Keeping terminated pod %q and uid %q, still running", podFullName, pod.UID)
+				glog.V(5).Infof("Keeping terminated pod %q, still running", format.Pod(pod))
 				continue
 			}
 			terminating = append(terminating, pod)
@@ -3079,8 +3077,7 @@ func (kl *Kubelet) generatePodStatus(pod *api.Pod) (api.PodStatus, error) {
 		metrics.PodStatusLatency.Observe(metrics.SinceInMicroseconds(start))
 	}()
 
-	podFullName := kubecontainer.GetPodFullName(pod)
-	glog.V(3).Infof("Generating status for %q", podFullName)
+	glog.V(3).Infof("Generating status for %q", format.Pod(pod))
 
 	// TODO: Consider include the container information.
 	if kl.pastActiveDeadline(pod) {
@@ -3097,7 +3094,7 @@ func (kl *Kubelet) generatePodStatus(pod *api.Pod) (api.PodStatus, error) {
 
 	if err != nil {
 		// Error handling
-		glog.Infof("Query container info for pod %q failed with error (%v)", podFullName, err)
+		glog.Infof("Query container info for pod %q failed with error (%v)", format.Pod(pod), err)
 		if strings.Contains(err.Error(), "resource temporarily unavailable") {
 			// Leave upstream layer to decide what to do
 			return api.PodStatus{}, err

pkg/kubelet/rkt/rkt.go

@@ -1154,8 +1154,6 @@ func (r *Runtime) IsImagePresent(image kubecontainer.ImageSpec) (bool, error) {
 
 // SyncPod syncs the running pod to match the specified desired pod.
 func (r *Runtime) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus api.PodStatus, _ *kubecontainer.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error {
-	podFullName := format.Pod(pod)
-
 	// Add references to all containers.
 	unidentifiedContainers := make(map[kubecontainer.ContainerID]*kubecontainer.Container)
 	for _, c := range runningPod.Containers {
@@ -1183,14 +1181,14 @@ func (r *Runtime) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus
 		// TODO(yifan): Take care of host network change.
 		containerChanged := c.Hash != 0 && c.Hash != expectedHash
 		if containerChanged {
-			glog.Infof("Pod %q container %q hash changed (%d vs %d), it will be killed and re-created.", podFullName, container.Name, c.Hash, expectedHash)
+			glog.Infof("Pod %q container %q hash changed (%d vs %d), it will be killed and re-created.", format.Pod(pod), container.Name, c.Hash, expectedHash)
 			restartPod = true
 			break
 		}
 
 		liveness, found := r.livenessManager.Get(c.ID)
 		if found && liveness != proberesults.Success && pod.Spec.RestartPolicy != api.RestartPolicyNever {
-			glog.Infof("Pod %q container %q is unhealthy, it will be killed and re-created.", podFullName, container.Name)
+			glog.Infof("Pod %q container %q is unhealthy, it will be killed and re-created.", format.Pod(pod), container.Name)
 			restartPod = true
 			break
 		}

pkg/kubelet/runonce.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/container"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
+	"k8s.io/kubernetes/pkg/kubelet/util/format"
 )
 
 const (
@@ -109,10 +110,9 @@ func (kl *Kubelet) runPod(pod *api.Pod, retryDelay time.Duration) error {
 		}
 		glog.Infof("pod %q containers not running: syncing", pod.Name)
 
-		podFullName := kubecontainer.GetPodFullName(pod)
-		glog.Infof("Creating a mirror pod for static pod %q", podFullName)
+		glog.Infof("Creating a mirror pod for static pod %q", format.Pod(pod))
 		if err := kl.podManager.CreateMirrorPod(pod); err != nil {
-			glog.Errorf("Failed creating a mirror pod %q: %v", podFullName, err)
+			glog.Errorf("Failed creating a mirror pod %q: %v", format.Pod(pod), err)
 		}
 		mirrorPod, _ := kl.podManager.GetMirrorPodByPod(pod)
 

pkg/kubelet/util/format/pod.go

@@ -25,26 +25,18 @@ import (
 
 type podHandler func(*api.Pod) string
 
-// Pod returns a string representating a pod in a human readable
-// format. This function currently is the same as GetPodFullName in
-// kubelet/containers, but may differ in the future. As opposed to
-// GetPodFullName, this function is mainly used for logging.
+// Pod returns a string reprenetating a pod in a human readable format,
+// with pod UID as part of the string.
 func Pod(pod *api.Pod) string {
 	// Use underscore as the delimiter because it is not allowed in pod name
 	// (DNS subdomain format), while allowed in the container name format.
-	return fmt.Sprintf("%s_%s", pod.Name, pod.Namespace)
-}
-
-// PodWithUID returns a string reprenetating a pod in a human readable format,
-// with pod UID as part of the string.
-func PodWithUID(pod *api.Pod) string {
-	return fmt.Sprintf("%s(%s)", Pod(pod), pod.UID)
+	return fmt.Sprintf("%s_%s(%s)", pod.Name, pod.Namespace, pod.UID)
 }
 
 // Pods returns a string representating a list of pods in a human
 // readable format.
 func Pods(pods []*api.Pod) string {
-	return aggregatePods(pods, PodWithUID)
+	return aggregatePods(pods, Pod)
 }
 
 func aggregatePods(pods []*api.Pod, handler podHandler) string {