Make ConfigOK status messages more human readable by including the API path to…

pkg/kubelet/kubeletconfig/checkpoint/download.go

@@ -128,13 +128,13 @@ func (r *remoteConfigMap) Download(client clientset.Interface) (Checkpoint, stri
 	// get the ConfigMap via namespace/name, there doesn't seem to be a way to get it by UID
 	cm, err := client.CoreV1().ConfigMaps(r.source.ConfigMapRef.Namespace).Get(r.source.ConfigMapRef.Name, metav1.GetOptions{})
 	if err != nil {
-		reason = fmt.Sprintf(status.FailSyncReasonDownloadFmt, r.source.ConfigMapRef.Name, r.source.ConfigMapRef.Namespace)
+		reason = fmt.Sprintf(status.FailSyncReasonDownloadFmt, r.APIPath())
 		return nil, reason, fmt.Errorf("%s, error: %v", reason, err)
 	}
 
 	// ensure that UID matches the UID on the reference, the ObjectReference must be unambiguous
 	if r.source.ConfigMapRef.UID != cm.UID {
-		reason = fmt.Sprintf(status.FailSyncReasonUIDMismatchFmt, r.source.ConfigMapRef.UID, cm.UID)
+		reason = fmt.Sprintf(status.FailSyncReasonUIDMismatchFmt, r.source.ConfigMapRef.UID, r.APIPath(), cm.UID)
 		return nil, reason, fmt.Errorf(reason)
 	}
 

pkg/kubelet/kubeletconfig/checkpoint/download_test.go

@@ -131,11 +131,11 @@ func TestRemoteConfigMapDownload(t *testing.T) {
 		// object doesn't exist
 		{"object doesn't exist",
 			&remoteConfigMap{&apiv1.NodeConfigSource{ConfigMapRef: &apiv1.ObjectReference{Name: "bogus", Namespace: "namespace", UID: "bogus"}}},
-			nil, "failed to download ConfigMap"},
+			nil, "not found"},
 		// UID of downloaded object doesn't match UID of referent found via namespace/name
 		{"UID is incorrect for namespace/name",
 			&remoteConfigMap{&apiv1.NodeConfigSource{ConfigMapRef: &apiv1.ObjectReference{Name: "name", Namespace: "namespace", UID: "bogus"}}},
-			nil, "does not match UID"},
+			nil, "does not match"},
 		// successful download
 		{"object exists and reference is correct",
 			&remoteConfigMap{&apiv1.NodeConfigSource{ConfigMapRef: &apiv1.ObjectReference{Name: "name", Namespace: "namespace", UID: "uid"}}},

pkg/kubelet/kubeletconfig/configsync.go

@@ -139,7 +139,7 @@ func (cc *Controller) checkpointConfigSource(client clientset.Interface, source
 
 	// if the checkpoint already exists, skip downloading
 	if ok, err := cc.checkpointStore.Exists(uid); err != nil {
-		reason := fmt.Sprintf(status.FailSyncReasonCheckpointExistenceFmt, uid)
+		reason := fmt.Sprintf(status.FailSyncReasonCheckpointExistenceFmt, source.APIPath(), uid)
 		return reason, fmt.Errorf("%s, error: %v", reason, err)
 	} else if ok {
 		utillog.Infof("checkpoint already exists for object with UID %q, skipping download", uid)
@@ -155,7 +155,7 @@ func (cc *Controller) checkpointConfigSource(client clientset.Interface, source
 	// save
 	err = cc.checkpointStore.Save(checkpoint)
 	if err != nil {
-		reason := fmt.Sprintf(status.FailSyncReasonSaveCheckpointFmt, checkpoint.UID())
+		reason := fmt.Sprintf(status.FailSyncReasonSaveCheckpointFmt, source.APIPath(), checkpoint.UID())
 		return reason, fmt.Errorf("%s, error: %v", reason, err)
 	}
 
@@ -170,7 +170,7 @@ func (cc *Controller) setCurrentConfig(source checkpoint.RemoteConfigSource) (bo
 		if source == nil {
 			return false, status.FailSyncReasonSetCurrentLocal, err
 		}
-		return false, fmt.Sprintf(status.FailSyncReasonSetCurrentUIDFmt, source.UID()), err
+		return false, fmt.Sprintf(status.FailSyncReasonSetCurrentUIDFmt, source.APIPath(), source.UID()), err
 	}
 	return updated, "", nil
 }

pkg/kubelet/kubeletconfig/controller.go

@@ -137,7 +137,7 @@ func (cc *Controller) Bootstrap() (*kubeletconfig.KubeletConfiguration, error) {
 	if err == nil {
 		// set the status to indicate we will use the assigned config
 		if curSource != nil {
-			cc.configOK.Set(fmt.Sprintf(status.CurRemoteMessageFmt, curSource.UID()), reason, apiv1.ConditionTrue)
+			cc.configOK.Set(fmt.Sprintf(status.CurRemoteMessageFmt, curSource.APIPath()), reason, apiv1.ConditionTrue)
 		} else {
 			cc.configOK.Set(status.CurLocalMessage, reason, apiv1.ConditionTrue)
 		}
@@ -171,7 +171,7 @@ func (cc *Controller) Bootstrap() (*kubeletconfig.KubeletConfiguration, error) {
 
 	// set the status to indicate that we had to roll back to the lkg for the reason reported when we tried to load the assigned config
 	if lkgSource != nil {
-		cc.configOK.Set(fmt.Sprintf(status.LkgRemoteMessageFmt, lkgSource.UID()), reason, apiv1.ConditionFalse)
+		cc.configOK.Set(fmt.Sprintf(status.LkgRemoteMessageFmt, lkgSource.APIPath()), reason, apiv1.ConditionFalse)
 	} else {
 		cc.configOK.Set(status.LkgLocalMessage, reason, apiv1.ConditionFalse)
 	}
@@ -271,14 +271,14 @@ func (cc *Controller) loadAssignedConfig(local *kubeletconfig.KubeletConfigurati
 	// load from checkpoint
 	checkpoint, err := cc.checkpointStore.Load(curUID)
 	if err != nil {
-		return nil, src, fmt.Sprintf(status.CurFailLoadReasonFmt, curUID), err
+		return nil, src, fmt.Sprintf(status.CurFailLoadReasonFmt, src.APIPath()), err
 	}
 	cur, err := checkpoint.Parse()
 	if err != nil {
-		return nil, src, fmt.Sprintf(status.CurFailParseReasonFmt, curUID), err
+		return nil, src, fmt.Sprintf(status.CurFailParseReasonFmt, src.APIPath()), err
 	}
 	if err := validation.ValidateKubeletConfiguration(cur); err != nil {
-		return nil, src, fmt.Sprintf(status.CurFailValidateReasonFmt, curUID), err
+		return nil, src, fmt.Sprintf(status.CurFailValidateReasonFmt, src.APIPath()), err
 	}
 	return cur, src, status.CurRemoteOkayReason, nil
 }
@@ -301,14 +301,14 @@ func (cc *Controller) loadLastKnownGoodConfig(local *kubeletconfig.KubeletConfig
 	// load from checkpoint
 	checkpoint, err := cc.checkpointStore.Load(lkgUID)
 	if err != nil {
-		return nil, src, fmt.Errorf("%s, error: %v", fmt.Sprintf(status.LkgFailLoadReasonFmt, lkgUID), err)
+		return nil, src, fmt.Errorf("%s, error: %v", fmt.Sprintf(status.LkgFailLoadReasonFmt, src.APIPath()), err)
 	}
 	lkg, err := checkpoint.Parse()
 	if err != nil {
-		return nil, src, fmt.Errorf("%s, error: %v", fmt.Sprintf(status.LkgFailParseReasonFmt, lkgUID), err)
+		return nil, src, fmt.Errorf("%s, error: %v", fmt.Sprintf(status.LkgFailParseReasonFmt, src.APIPath()), err)
 	}
 	if err := validation.ValidateKubeletConfiguration(lkg); err != nil {
-		return nil, src, fmt.Errorf("%s, error: %v", fmt.Sprintf(status.LkgFailValidateReasonFmt, lkgUID), err)
+		return nil, src, fmt.Errorf("%s, error: %v", fmt.Sprintf(status.LkgFailValidateReasonFmt, src.APIPath()), err)
 	}
 	return lkg, src, nil
 }

pkg/kubelet/kubeletconfig/status/status.go

@@ -40,14 +40,14 @@ const (
 	NotDynamicLocalReason = "dynamic config is currently disabled by omission of --dynamic-config-dir Kubelet flag"
 
 	// CurLocalMessage indicates that the Kubelet is using its local config, which consists of defaults, flags, and/or local files
-	CurLocalMessage = "using current (local)"
+	CurLocalMessage = "using current: local"
 	// LkgLocalMessage indicates that the Kubelet is using its local config, which consists of defaults, flags, and/or local files
-	LkgLocalMessage = "using last-known-good (local)"
+	LkgLocalMessage = "using last-known-good: local"
 
 	// CurRemoteMessageFmt indicates the Kubelet is using its current config, which is from an API source
-	CurRemoteMessageFmt = "using current (UID: %q)"
+	CurRemoteMessageFmt = "using current: %s"
 	// LkgRemoteMessageFmt indicates the Kubelet is using its last-known-good config, which is from an API source
-	LkgRemoteMessageFmt = "using last-known-good (UID: %q)"
+	LkgRemoteMessageFmt = "using last-known-good: %s"
 
 	// CurLocalOkayReason indicates that the Kubelet is using its local config
 	CurLocalOkayReason = "when the config source is nil, the Kubelet uses its local config"
@@ -55,20 +55,20 @@ const (
 	CurRemoteOkayReason = "passing all checks"
 
 	// CurFailLoadReasonFmt indicates that the Kubelet failed to load the current config checkpoint for an API source
-	CurFailLoadReasonFmt = "failed to load current (UID: %q)"
+	CurFailLoadReasonFmt = "failed to load current: %s"
 	// CurFailParseReasonFmt indicates that the Kubelet failed to parse the current config checkpoint for an API source
-	CurFailParseReasonFmt = "failed to parse current (UID: %q)"
+	CurFailParseReasonFmt = "failed to parse current: %s"
 	// CurFailValidateReasonFmt indicates that the Kubelet failed to validate the current config checkpoint for an API source
-	CurFailValidateReasonFmt = "failed to validate current (UID: %q)"
+	CurFailValidateReasonFmt = "failed to validate current: %s"
 
 	// LkgFail*ReasonFmt reasons are currently used to print errors in the Kubelet log, but do not appear in Node.Status.Conditions
 
 	// LkgFailLoadReasonFmt indicates that the Kubelet failed to load the last-known-good config checkpoint for an API source
-	LkgFailLoadReasonFmt = "failed to load last-known-good (UID: %q)"
+	LkgFailLoadReasonFmt = "failed to load last-known-good: %s"
 	// LkgFailParseReasonFmt indicates that the Kubelet failed to parse the last-known-good config checkpoint for an API source
-	LkgFailParseReasonFmt = "failed to parse last-known-good (UID: %q)"
+	LkgFailParseReasonFmt = "failed to parse last-known-good: %s"
 	// LkgFailValidateReasonFmt indicates that the Kubelet failed to validate the last-known-good config checkpoint for an API source
-	LkgFailValidateReasonFmt = "failed to validate last-known-good (UID: %q)"
+	LkgFailValidateReasonFmt = "failed to validate last-known-good: %s"
 
 	// FailSyncReasonFmt is used when the system couldn't sync the config, due to a malformed Node.Spec.ConfigSource, a download failure, etc.
 	FailSyncReasonFmt = "failed to sync, reason: %s"
@@ -78,21 +78,21 @@ const (
 	FailSyncReasonPartialObjectReference = "invalid ObjectReference, all of UID, Name, and Namespace must be specified"
 	// FailSyncReasonUIDMismatchFmt is used when there is a UID mismatch between the referenced and downloaded ConfigMaps,
 	// this can happen because objects must be downloaded by namespace/name, rather than by UID
-	FailSyncReasonUIDMismatchFmt = "invalid ObjectReference, UID %q does not match UID of downloaded ConfigMap %q"
+	FailSyncReasonUIDMismatchFmt = "invalid ConfigSource.ConfigMapRef.UID: %s does not match %s.UID: %s"
 	// FailSyncReasonDownloadFmt is used when the download fails, e.g. due to network issues
-	FailSyncReasonDownloadFmt = "failed to download ConfigMap with name %q from namespace %q"
+	FailSyncReasonDownloadFmt = "failed to download: %s"
 	// FailSyncReasonInformer is used when the informer fails to report the Node object
 	FailSyncReasonInformer = "failed to read Node from informer object cache"
 	// FailSyncReasonReset is used when we can't reset the local configuration references, e.g. due to filesystem issues
 	FailSyncReasonReset = "failed to reset to local config"
 	// FailSyncReasonCheckpointExistenceFmt is used when we can't determine if a checkpoint already exists, e.g. due to filesystem issues
-	FailSyncReasonCheckpointExistenceFmt = "failed to determine whether object with UID %q was already checkpointed"
+	FailSyncReasonCheckpointExistenceFmt = "failed to determine whether object %s with UID %s was already checkpointed"
 	// FailSyncReasonSaveCheckpointFmt is used when we can't save a checkpoint, e.g. due to filesystem issues
-	FailSyncReasonSaveCheckpointFmt = "failed to save config checkpoint for object with UID %q"
+	FailSyncReasonSaveCheckpointFmt = "failed to save config checkpoint for object %s with UID %s"
 	// FailSyncReasonSetCurrentDefault is used when we can't set the current config checkpoint to the local default, e.g. due to filesystem issues
 	FailSyncReasonSetCurrentLocal = "failed to set current config checkpoint to local config"
 	// FailSyncReasonSetCurrentUIDFmt is used when we can't set the current config checkpoint to a checkpointed object, e.g. due to filesystem issues
-	FailSyncReasonSetCurrentUIDFmt = "failed to set current config checkpoint to object with UID %q"
+	FailSyncReasonSetCurrentUIDFmt = "failed to set current config checkpoint to object %s with UID %s"
 
 	// EmptyMessage is a placeholder in the case that we accidentally set the condition's message to the empty string.
 	// Doing so can result in a partial patch, and thus a confusing status; this makes it clear that the message was not provided.

test/e2e_node/dynamic_kubelet_config_test.go

@@ -86,7 +86,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 					Namespace: originalConfigMap.Namespace,
 					Name:      originalConfigMap.Name}},
 				expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionTrue,
-					Message: fmt.Sprintf(status.CurRemoteMessageFmt, originalConfigMap.UID),
+					Message: fmt.Sprintf(status.CurRemoteMessageFmt, configMapAPIPath(originalConfigMap)),
 					Reason:  status.CurRemoteOkayReason},
 				expectConfig: originalKC,
 			}, false)
@@ -162,7 +162,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Name:      correctConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionFalse,
 							Message: "",
-							Reason:  fmt.Sprintf(status.FailSyncReasonFmt, fmt.Sprintf(status.FailSyncReasonUIDMismatchFmt, "foo", correctConfigMap.UID))},
+							Reason:  fmt.Sprintf(status.FailSyncReasonFmt, fmt.Sprintf(status.FailSyncReasonUIDMismatchFmt, "foo", configMapAPIPath(correctConfigMap), correctConfigMap.UID))},
 						expectConfig: nil,
 						event:        false,
 					},
@@ -174,7 +174,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: correctConfigMap.Namespace,
 							Name:      correctConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionTrue,
-							Message: fmt.Sprintf(status.CurRemoteMessageFmt, correctConfigMap.UID),
+							Message: fmt.Sprintf(status.CurRemoteMessageFmt, configMapAPIPath(correctConfigMap)),
 							Reason:  status.CurRemoteOkayReason},
 						expectConfig: correctKC,
 						event:        true,
@@ -188,7 +188,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Name:      failParseConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionFalse,
 							Message: status.LkgLocalMessage,
-							Reason:  fmt.Sprintf(status.CurFailParseReasonFmt, failParseConfigMap.UID)},
+							Reason:  fmt.Sprintf(status.CurFailParseReasonFmt, configMapAPIPath(failParseConfigMap))},
 						expectConfig: nil,
 						event:        true,
 					},
@@ -201,7 +201,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Name:      failValidateConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionFalse,
 							Message: status.LkgLocalMessage,
-							Reason:  fmt.Sprintf(status.CurFailValidateReasonFmt, failValidateConfigMap.UID)},
+							Reason:  fmt.Sprintf(status.CurFailValidateReasonFmt, configMapAPIPath(failValidateConfigMap))},
 						expectConfig: nil,
 						event:        true,
 					},
@@ -244,7 +244,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: lkgConfigMap.Namespace,
 							Name:      lkgConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionTrue,
-							Message: fmt.Sprintf(status.CurRemoteMessageFmt, lkgConfigMap.UID),
+							Message: fmt.Sprintf(status.CurRemoteMessageFmt, configMapAPIPath(lkgConfigMap)),
 							Reason:  status.CurRemoteOkayReason},
 						expectConfig: lkgKC,
 						event:        true,
@@ -257,8 +257,8 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: badConfigMap.Namespace,
 							Name:      badConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionFalse,
-							Message: fmt.Sprintf(status.LkgRemoteMessageFmt, lkgConfigMap.UID),
-							Reason:  fmt.Sprintf(status.CurFailParseReasonFmt, badConfigMap.UID)},
+							Message: fmt.Sprintf(status.LkgRemoteMessageFmt, configMapAPIPath(lkgConfigMap)),
+							Reason:  fmt.Sprintf(status.CurFailParseReasonFmt, configMapAPIPath(badConfigMap))},
 						expectConfig: lkgKC,
 						event:        true,
 					},
@@ -294,7 +294,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: cm1.Namespace,
 							Name:      cm1.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionTrue,
-							Message: fmt.Sprintf(status.CurRemoteMessageFmt, cm1.UID),
+							Message: fmt.Sprintf(status.CurRemoteMessageFmt, configMapAPIPath(cm1)),
 							Reason:  status.CurRemoteOkayReason},
 						expectConfig: kc1,
 						event:        true,
@@ -306,7 +306,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: cm2.Namespace,
 							Name:      cm2.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionTrue,
-							Message: fmt.Sprintf(status.CurRemoteMessageFmt, cm2.UID),
+							Message: fmt.Sprintf(status.CurRemoteMessageFmt, configMapAPIPath(cm2)),
 							Reason:  status.CurRemoteOkayReason},
 						expectConfig: kc2,
 						event:        true,
@@ -488,3 +488,8 @@ func checkEvent(f *framework.Framework, desc string, expect *apiv1.NodeConfigSou
 		return nil
 	}, timeout, interval).Should(BeNil())
 }
+
+// constructs the expected SelfLink for a config map
+func configMapAPIPath(cm *apiv1.ConfigMap) string {
+	return fmt.Sprintf("/api/v1/namespaces/%s/configmaps/%s", cm.Namespace, cm.Name)
+}