Merge pull request #42862 from caesarxuchao/sync-warning

Automatic merge from submit-queue (batch tested with PRs 38805, 42362, 42862)

Let GC print specific message for RESTMapping failure

Make the error messages reported in #39816 to be more specific, also only print the message once.

I'll also update the garbage collector's doc to clearly state we don't support tpr yet.

We'll wait for the watchable discovery feature (@sttts are you going to work on that?) to land in 1.7, and then enable the garbage collector to handle TPR.

cc @hongchaodeng @MikaelCluseau @djMax

pkg/controller/garbagecollector/garbagecollector.go

@@ -143,6 +143,12 @@ func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 	}
 	err := gc.attemptToDeleteItem(n)
 	if err != nil {
+		// TODO: remove this block when gc starts using dynamic RESTMapper.
+		if restMappingError, ok := err.(*restMappingError); ok {
+			utilruntime.HandleError(fmt.Errorf("Ignore syncing item %#v: %s", n, restMappingError.Message()))
+			// The RESTMapper is static, so no need to retry, otherwise we'll get the same error.
+			return true
+		}
 		utilruntime.HandleError(fmt.Errorf("Error syncing item %#v: %v", n, err))
 		// retry if garbage collection of an object failed.
 		gc.attemptToDelete.AddRateLimited(item)

pkg/controller/garbagecollector/operations.go

@@ -30,13 +30,36 @@ import (
 	"k8s.io/kubernetes/pkg/client/retry"
 )
 
+type restMappingError struct {
+	kind    string
+	version string
+}
+
+func (r *restMappingError) Error() string {
+	versionKind := fmt.Sprintf("%s/%s", r.version, r.kind)
+	return fmt.Sprintf("unable to get REST mapping for %s.", versionKind)
+}
+
+// Message prints more details
+func (r *restMappingError) Message() string {
+	versionKind := fmt.Sprintf("%s/%s", r.version, r.kind)
+	errMsg := fmt.Sprintf("unable to get REST mapping for %s.", versionKind)
+	errMsg += fmt.Sprintf(" If %s is a thirdparty resource (tpr), please note that the garbage collector doesn't support tpr yet. Once tpr is supported, object with ownerReferences referring non-existing tpr objects will be deleted by the garbage collector.", versionKind)
+	errMsg += fmt.Sprintf(" If %s is not a tpr, then you should remove ownerReferences that refer %s objects manually.", versionKind, versionKind)
+	return errMsg
+}
+
+func newRESTMappingError(kind, version string) *restMappingError {
+	return &restMappingError{kind: kind, version: version}
+}
+
 // apiResource consults the REST mapper to translate an <apiVersion, kind,
 // namespace> tuple to a unversioned.APIResource struct.
 func (gc *GarbageCollector) apiResource(apiVersion, kind string, namespaced bool) (*metav1.APIResource, error) {
 	fqKind := schema.FromAPIVersionAndKind(apiVersion, kind)
 	mapping, err := gc.restMapper.RESTMapping(fqKind.GroupKind(), apiVersion)
 	if err != nil {
-		return nil, fmt.Errorf("unable to get REST mapping for kind: %s, version: %s", kind, apiVersion)
+		return nil, newRESTMappingError(kind, apiVersion)
 	}
 	glog.V(5).Infof("map kind %s, version %s to resource %s", kind, apiVersion, mapping.Resource)
 	resource := metav1.APIResource{