Merge pull request #53303 from liggitt/discovery-logging

Automatic merge from submit-queue (batch tested with PRs 47039, 53681, 53303, 53181, 53781). 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>.

Avoid fetching entire discovery tree when possible

For specific commands, we use discovery to determine whether a particular resource is available.

We should avoid fetching the entire discovery tree to check a single resource group version. As the number of groups grows, the performance hit and potential to encounter an error also grows.

```release-note
NONE
```

pkg/kubectl/cmd/create_deployment.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cmd
 
 import (
-	"fmt"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -109,17 +108,15 @@ func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer,
 	if err != nil {
 		return err
 	}
-	resourcesList, err := clientset.Discovery().ServerResources()
-	// ServerResources ignores errors for old servers do not expose discovery
-	if err != nil {
-		return fmt.Errorf("failed to discover supported resources: %v", err)
-	}
 
 	generatorName := cmdutil.GetFlagString(cmd, "generator")
 
 	// It is possible we have to modify the user-provided generator name if
 	// the server does not have support for the requested generator.
-	generatorName = cmdutil.FallbackGeneratorNameIfNecessary(generatorName, resourcesList, cmdErr)
+	generatorName, err = cmdutil.FallbackGeneratorNameIfNecessary(generatorName, clientset.Discovery(), cmdErr)
+	if err != nil {
+		return err
+	}
 
 	imageNames := cmdutil.GetFlagStringSlice(cmd, "image")
 	generator, ok := generatorFromName(generatorName, imageNames, deploymentName)

pkg/kubectl/cmd/run.go

@@ -214,16 +214,15 @@ func RunRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *c
 	if err != nil {
 		return err
 	}
-	resourcesList, err := clientset.Discovery().ServerResources()
-	// ServerResources ignores errors for old servers do not expose discovery
-	if err != nil {
-		return fmt.Errorf("failed to discover supported resources: %v", err)
-	}
 
 	generatorName := cmdutil.GetFlagString(cmd, "generator")
 	schedule := cmdutil.GetFlagString(cmd, "schedule")
 	if len(schedule) != 0 && len(generatorName) == 0 {
-		if cmdutil.Contains(resourcesList, batchv1beta1.SchemeGroupVersion.WithResource("cronjobs")) {
+		hasResource, err := cmdutil.HasResource(clientset.Discovery(), batchv1beta1.SchemeGroupVersion.WithResource("cronjobs"))
+		if err != nil {
+			return err
+		}
+		if hasResource {
 			generatorName = cmdutil.CronJobV1Beta1GeneratorName
 		} else {
 			generatorName = cmdutil.CronJobV2Alpha1GeneratorName
@@ -234,13 +233,21 @@ func RunRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *c
 		case api.RestartPolicyAlways:
 			// TODO: we need to deprecate this along with extensions/v1beta1.Deployments
 			// in favor of the new generator for apps/v1beta1.Deployments
-			if cmdutil.Contains(resourcesList, extensionsv1beta1.SchemeGroupVersion.WithResource("deployments")) {
+			hasResource, err := cmdutil.HasResource(clientset.Discovery(), extensionsv1beta1.SchemeGroupVersion.WithResource("deployments"))
+			if err != nil {
+				return err
+			}
+			if hasResource {
 				generatorName = cmdutil.DeploymentV1Beta1GeneratorName
 			} else {
 				generatorName = cmdutil.RunV1GeneratorName
 			}
 		case api.RestartPolicyOnFailure:
-			if cmdutil.Contains(resourcesList, batchv1.SchemeGroupVersion.WithResource("jobs")) {
+			hasResource, err := cmdutil.HasResource(clientset.Discovery(), batchv1.SchemeGroupVersion.WithResource("jobs"))
+			if err != nil {
+				return err
+			}
+			if hasResource {
 				generatorName = cmdutil.JobV1GeneratorName
 			} else {
 				generatorName = cmdutil.RunPodV1GeneratorName
@@ -250,7 +257,10 @@ func RunRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *c
 		}
 	}
 
-	generatorName = cmdutil.FallbackGeneratorNameIfNecessary(generatorName, resourcesList, cmdErr)
+	generatorName, err = cmdutil.FallbackGeneratorNameIfNecessary(generatorName, clientset.Discovery(), cmdErr)
+	if err != nil {
+		return err
+	}
 
 	generators := f.Generators("run")
 	generator, found := generators[generatorName]

pkg/kubectl/cmd/util/cached_discovery.go

@@ -67,7 +67,7 @@ func (d *CachedDiscoveryClient) ServerResourcesForGroupVersion(groupVersion stri
 	if err == nil {
 		cachedResources := &metav1.APIResourceList{}
 		if err := runtime.DecodeInto(api.Codecs.UniversalDecoder(), cachedBytes, cachedResources); err == nil {
-			glog.V(6).Infof("returning cached discovery info from %v", filename)
+			glog.V(10).Infof("returning cached discovery info from %v", filename)
 			return cachedResources, nil
 		}
 	}
@@ -114,7 +114,7 @@ func (d *CachedDiscoveryClient) ServerGroups() (*metav1.APIGroupList, error) {
 	if err == nil {
 		cachedGroups := &metav1.APIGroupList{}
 		if err := runtime.DecodeInto(api.Codecs.UniversalDecoder(), cachedBytes, cachedGroups); err == nil {
-			glog.V(6).Infof("returning cached discovery info from %v", filename)
+			glog.V(10).Infof("returning cached discovery info from %v", filename)
 			return cachedGroups, nil
 		}
 	}

pkg/kubectl/cmd/util/factory_client_access.go

@@ -34,6 +34,7 @@ import (
 
 	appsv1beta1 "k8s.io/api/apps/v1beta1"
 	batchv2alpha1 "k8s.io/api/batch/v2alpha1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -582,24 +583,30 @@ func DefaultGenerators(cmdName string) map[string]kubectl.Generator {
 // know.
 func FallbackGeneratorNameIfNecessary(
 	generatorName string,
-	resourcesList []*metav1.APIResourceList,
+	discoveryClient discovery.DiscoveryInterface,
 	cmdErr io.Writer,
-) string {
+) (string, error) {
 	switch generatorName {
 	case DeploymentBasicAppsV1Beta1GeneratorName:
-		if !Contains(resourcesList, appsv1beta1.SchemeGroupVersion.WithResource("deployments")) {
+		hasResource, err := HasResource(discoveryClient, appsv1beta1.SchemeGroupVersion.WithResource("deployments"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
 			warning(cmdErr, DeploymentBasicAppsV1Beta1GeneratorName, DeploymentBasicV1Beta1GeneratorName)
-
+			return DeploymentBasicV1Beta1GeneratorName, nil
-			return DeploymentBasicV1Beta1GeneratorName
 		}
 	case CronJobV2Alpha1GeneratorName:
-		if !Contains(resourcesList, batchv2alpha1.SchemeGroupVersion.WithResource("cronjobs")) {
+		hasResource, err := HasResource(discoveryClient, batchv2alpha1.SchemeGroupVersion.WithResource("cronjobs"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
 			warning(cmdErr, CronJobV2Alpha1GeneratorName, JobV1GeneratorName)
-
+			return JobV1GeneratorName, nil
-			return JobV1GeneratorName
 		}
 	}
-	return generatorName
+	return generatorName, nil
 }
 
 func warning(cmdErr io.Writer, newGeneratorName, oldGeneratorName string) {
@@ -611,6 +618,24 @@ func warning(cmdErr io.Writer, newGeneratorName, oldGeneratorName string) {
 	)
 }
 
+func HasResource(client discovery.DiscoveryInterface, resource schema.GroupVersionResource) (bool, error) {
+	resources, err := client.ServerResourcesForGroupVersion(resource.GroupVersion().String())
+	if apierrors.IsNotFound(err) {
+		// entire group is missing
+		return false, nil
+	}
+	if err != nil {
+		// other errors error
+		return false, fmt.Errorf("failed to discover supported resources: %v", err)
+	}
+	for _, serverResource := range resources.APIResources {
+		if serverResource.Name == resource.Resource {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 func Contains(resourcesList []*metav1.APIResourceList, resource schema.GroupVersionResource) bool {
 	resources := discovery.FilteredBy(discovery.ResourcePredicateFunc(func(gv string, r *metav1.APIResource) bool {
 		return resource.GroupVersion().String() == gv && resource.Resource == r.Name