Merge pull request #57524 from mtaufen/kc-status-selflink

Automatic merge from submit-queue (batch tested with PRs 57533, 57524). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Make ConfigOK status messages more human readable

This makes the ConfigOK status messages for dynamic config more human readable by including the path (e.g. SelfLink) to the object. The messages used to include the UID, but this was kind of useless, because there's no way to GET an object by UID. 

```release-note
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2018-01-03 18:56:44 -08:00 committed by GitHub
commit e823c473b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 45 additions and 40 deletions

View File

@ -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)
}

View File

@ -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"}}},

View File

@ -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
}

View File

@ -136,7 +136,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)
}
@ -166,7 +166,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)
}
@ -266,14 +266,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
}
@ -296,14 +296,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
}

View File

@ -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.

View File

@ -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)
}