Merge pull request #43235 from caesarxuchao/tolerator-tpr-monitor-failure

Automatic merge from submit-queue

construction of GC should not fail for restmapper error caused by tpr

Fix https://github.com/kubernetes/kubernetes/issues/43147.

The issue is that GC will fail its initialization due to an RESTMapper error cause by tpr. This PR lets GC log the error instead of failing.

pkg/controller/garbagecollector/BUILD

@@ -11,6 +11,7 @@ load(
 go_library(
     name = "go_default_library",
     srcs = [
+        "errors.go",
         "garbagecollector.go",
         "graph.go",
         "graph_builder.go",

pkg/controller/garbagecollector/errors.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package garbagecollector
+
+import (
+	"fmt"
+)
+
+const nonCoreMessage = `If %s is a non-core resource (e.g. thirdparty resource, custom resource from aggregated apiserver), please note that the garbage collector doesn't support non-core resources yet. Once they are supported, object with ownerReferences referring non-existing non-core objects will be deleted by the garbage collector.`
+
+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(nonCoreMessage, versionKind)
+	errMsg += fmt.Sprintf(" If %s is an invalid resource, then you should manually remove ownerReferences that refer %s objects.", versionKind, versionKind)
+	return errMsg
+}
+
+func newRESTMappingError(kind, version string) *restMappingError {
+	return &restMappingError{kind: kind, version: version}
+}

pkg/controller/garbagecollector/garbagecollector_test.go

@@ -50,7 +50,11 @@ func TestNewGarbageCollector(t *testing.T) {
 	metaOnlyClientPool := dynamic.NewClientPool(config, api.Registry.RESTMapper(), dynamic.LegacyAPIPathResolverFunc)
 	config.ContentConfig.NegotiatedSerializer = nil
 	clientPool := dynamic.NewClientPool(config, api.Registry.RESTMapper(), dynamic.LegacyAPIPathResolverFunc)
-	podResource := map[schema.GroupVersionResource]struct{}{schema.GroupVersionResource{Version: "v1", Resource: "pods"}: {}}
+	podResource := map[schema.GroupVersionResource]struct{}{
+		schema.GroupVersionResource{Version: "v1", Resource: "pods"}: {},
+		// no monitor will be constructed for non-core resource, the GC construction will not fail.
+		schema.GroupVersionResource{Group: "tpr.io", Version: "v1", Resource: "unknown"}: {},
+	}
 	gc, err := NewGarbageCollector(metaOnlyClientPool, clientPool, api.Registry.RESTMapper(), podResource)
 	if err != nil {
 		t.Fatal(err)

pkg/controller/garbagecollector/graph_builder.go

@@ -165,7 +165,9 @@ func (gb *GraphBuilder) monitorsForResources(resources map[schema.GroupVersionRe
 		}
 		kind, err := gb.restMapper.KindFor(resource)
 		if err != nil {
-			return err
+			nonCoreMsg := fmt.Sprintf(nonCoreMessage, resource)
+			utilruntime.HandleError(fmt.Errorf("%v. %s", err, nonCoreMsg))
+			continue
 		}
 		monitor, err := gb.controllerFor(resource, kind)
 		if err != nil {

pkg/controller/garbagecollector/operations.go

@@ -30,29 +30,6 @@ 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) {