✨ Use aggregated discovery if available

This change makes the `RESTMapper` use [AggredatedDiscovery][0] if available. `AggregatedDiscovery` is beta and thus enabled by default since Kube 1.28. What this means in particular is that during startup, we will always do a request to `/api` and `/apis`. If `AggregatedDiscovery` is enabled, that is all we have to do. If not, we have to do a third request. The behavior for reloading remains the same: Do one request unless the request didn't include a version. In that case we have to find the group. If the group is unknown, we will keep on doing a request to `/api` and one to `/apis` to find it. If it is found we are again done in the `AggregatedDiscovery` case and have to do a third request otherwise. All in all this means that with `AggregatedDiscovery` enabled, the only case where this is worse is if the client interacts with only one `GroupVersion`, as we end up doing one additional api request. If it is disabled, we effectively waste two api requests. [0]: kubernetes/enhancements#3352
kubernetes-sigs · Aug 3, 2024 · b0abe43 · b0abe43
1 parent abb2d86
commit b0abe43
Show file tree

Hide file tree

Showing 4 changed files with 850 additions and 729 deletions.
diff --git a/pkg/client/apiutil/apimachinery_test.go b/pkg/client/apiutil/apimachinery_test.go
@@ -18,6 +18,7 @@ package apiutil_test
 
 import (
 	"context"
+	"strconv"
 	"testing"
 
 	gmg "github.com/onsi/gomega"
@@ -32,127 +33,130 @@ import (
 )
 
 func TestApiMachinery(t *testing.T) {
-	restCfg, tearDownFn := setupEnvtest(t)
-	defer tearDownFn(t)
-
-	// Details of the GVK registered at initialization.
-	initialGvk := metav1.GroupVersionKind{
-		Group:   "crew.example.com",
-		Version: "v1",
-		Kind:    "Driver",
-	}
+	for _, aggregatedDiscovery := range []bool{true, false} {
+		t.Run("aggregatedDiscovery="+strconv.FormatBool(aggregatedDiscovery), func(t *testing.T) {
+			restCfg := setupEnvtest(t, !aggregatedDiscovery)
 
-	// A set of GVKs to register at runtime with varying properties.
-	runtimeGvks := []struct {
-		name   string
-		gvk    metav1.GroupVersionKind
-		plural string
-	}{
-		{
-			name: "new Kind and Version added to existing Group",
-			gvk: metav1.GroupVersionKind{
-				Group:   "crew.example.com",
-				Version: "v1alpha1",
-				Kind:    "Passenger",
-			},
-			plural: "passengers",
-		},
-		{
-			name: "new Kind added to existing Group and Version",
-			gvk: metav1.GroupVersionKind{
+			// Details of the GVK registered at initialization.
+			initialGvk := metav1.GroupVersionKind{
 				Group:   "crew.example.com",
 				Version: "v1",
-				Kind:    "Garage",
-			},
-			plural: "garages",
-		},
-		{
-			name: "new GVK",
-			gvk: metav1.GroupVersionKind{
-				Group:   "inventory.example.com",
-				Version: "v1",
-				Kind:    "Taxi",
-			},
-			plural: "taxis",
-		},
-	}
+				Kind:    "Driver",
+			}
+
+			// A set of GVKs to register at runtime with varying properties.
+			runtimeGvks := []struct {
+				name   string
+				gvk    metav1.GroupVersionKind
+				plural string
+			}{
+				{
+					name: "new Kind and Version added to existing Group",
+					gvk: metav1.GroupVersionKind{
+						Group:   "crew.example.com",
+						Version: "v1alpha1",
+						Kind:    "Passenger",
+					},
+					plural: "passengers",
+				},
+				{
+					name: "new Kind added to existing Group and Version",
+					gvk: metav1.GroupVersionKind{
+						Group:   "crew.example.com",
+						Version: "v1",
+						Kind:    "Garage",
+					},
+					plural: "garages",
+				},
+				{
+					name: "new GVK",
+					gvk: metav1.GroupVersionKind{
+						Group:   "inventory.example.com",
+						Version: "v1",
+						Kind:    "Taxi",
+					},
+					plural: "taxis",
+				},
+			}
+
+			t.Run("IsGVKNamespaced should report scope for GVK registered at initialization", func(t *testing.T) {
+				g := gmg.NewWithT(t)
+
+				httpClient, err := rest.HTTPClientFor(restCfg)
+				g.Expect(err).NotTo(gmg.HaveOccurred())
 
-	t.Run("IsGVKNamespaced should report scope for GVK registered at initialization", func(t *testing.T) {
-		g := gmg.NewWithT(t)
-
-		httpClient, err := rest.HTTPClientFor(restCfg)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		s := scheme.Scheme
-		err = apiextensionsv1.AddToScheme(s)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		// Query the scope of a GVK that was registered at initialization.
-		scope, err := apiutil.IsGVKNamespaced(
-			schema.GroupVersionKind(initialGvk),
-			lazyRestMapper,
-		)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-		g.Expect(scope).To(gmg.BeTrue())
-	})
-
-	for _, runtimeGvk := range runtimeGvks {
-		t.Run("IsGVKNamespaced should report scope for "+runtimeGvk.name, func(t *testing.T) {
-			g := gmg.NewWithT(t)
-			ctx := context.Background()
-
-			httpClient, err := rest.HTTPClientFor(restCfg)
-			g.Expect(err).NotTo(gmg.HaveOccurred())
-
-			lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
-			g.Expect(err).NotTo(gmg.HaveOccurred())
-
-			s := scheme.Scheme
-			err = apiextensionsv1.AddToScheme(s)
-			g.Expect(err).NotTo(gmg.HaveOccurred())
-
-			c, err := client.New(restCfg, client.Options{Scheme: s})
-			g.Expect(err).NotTo(gmg.HaveOccurred())
-
-			// Run a valid query to initialize cache.
-			scope, err := apiutil.IsGVKNamespaced(
-				schema.GroupVersionKind(initialGvk),
-				lazyRestMapper,
-			)
-			g.Expect(err).NotTo(gmg.HaveOccurred())
-			g.Expect(scope).To(gmg.BeTrue())
-
-			// Register a new CRD at runtime.
-			crd := newCRD(ctx, g, c, runtimeGvk.gvk.Group, runtimeGvk.gvk.Kind, runtimeGvk.plural)
-			version := crd.Spec.Versions[0]
-			version.Name = runtimeGvk.gvk.Version
-			version.Storage = true
-			version.Served = true
-			crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{version}
-			crd.Spec.Scope = apiextensionsv1.NamespaceScoped
-
-			g.Expect(c.Create(ctx, crd)).To(gmg.Succeed())
-			t.Cleanup(func() {
-				g.Expect(c.Delete(ctx, crd)).To(gmg.Succeed())
-			})
+				lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
+				g.Expect(err).NotTo(gmg.HaveOccurred())
+
+				s := scheme.Scheme
+				err = apiextensionsv1.AddToScheme(s)
+				g.Expect(err).NotTo(gmg.HaveOccurred())
 
-			// Wait until the CRD is registered.
-			g.Eventually(func(g gmg.Gomega) {
-				isRegistered, err := isCrdRegistered(restCfg, runtimeGvk.gvk)
+				// Query the scope of a GVK that was registered at initialization.
+				scope, err := apiutil.IsGVKNamespaced(
+					schema.GroupVersionKind(initialGvk),
+					lazyRestMapper,
+				)
 				g.Expect(err).NotTo(gmg.HaveOccurred())
-				g.Expect(isRegistered).To(gmg.BeTrue())
-			}).Should(gmg.Succeed(), "GVK should be available")
-
-			// Query the scope of the GVK registered at runtime.
-			scope, err = apiutil.IsGVKNamespaced(
-				schema.GroupVersionKind(runtimeGvk.gvk),
-				lazyRestMapper,
-			)
-			g.Expect(err).NotTo(gmg.HaveOccurred())
-			g.Expect(scope).To(gmg.BeTrue())
+				g.Expect(scope).To(gmg.BeTrue())
+			})
+
+			for _, runtimeGvk := range runtimeGvks {
+				t.Run("IsGVKNamespaced should report scope for "+runtimeGvk.name, func(t *testing.T) {
+					g := gmg.NewWithT(t)
+					ctx := context.Background()
+
+					httpClient, err := rest.HTTPClientFor(restCfg)
+					g.Expect(err).NotTo(gmg.HaveOccurred())
+
+					lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
+					g.Expect(err).NotTo(gmg.HaveOccurred())
+
+					s := scheme.Scheme
+					err = apiextensionsv1.AddToScheme(s)
+					g.Expect(err).NotTo(gmg.HaveOccurred())
+
+					c, err := client.New(restCfg, client.Options{Scheme: s})
+					g.Expect(err).NotTo(gmg.HaveOccurred())
+
+					// Run a valid query to initialize cache.
+					scope, err := apiutil.IsGVKNamespaced(
+						schema.GroupVersionKind(initialGvk),
+						lazyRestMapper,
+					)
+					g.Expect(err).NotTo(gmg.HaveOccurred())
+					g.Expect(scope).To(gmg.BeTrue())
+
+					// Register a new CRD at runtime.
+					crd := newCRD(ctx, g, c, runtimeGvk.gvk.Group, runtimeGvk.gvk.Kind, runtimeGvk.plural)
+					version := crd.Spec.Versions[0]
+					version.Name = runtimeGvk.gvk.Version
+					version.Storage = true
+					version.Served = true
+					crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{version}
+					crd.Spec.Scope = apiextensionsv1.NamespaceScoped
+
+					g.Expect(c.Create(ctx, crd)).To(gmg.Succeed())
+					t.Cleanup(func() {
+						g.Expect(c.Delete(ctx, crd)).To(gmg.Succeed())
+					})
+
+					// Wait until the CRD is registered.
+					g.Eventually(func(g gmg.Gomega) {
+						isRegistered, err := isCrdRegistered(restCfg, runtimeGvk.gvk)
+						g.Expect(err).NotTo(gmg.HaveOccurred())
+						g.Expect(isRegistered).To(gmg.BeTrue())
+					}).Should(gmg.Succeed(), "GVK should be available")
+
+					// Query the scope of the GVK registered at runtime.
+					scope, err = apiutil.IsGVKNamespaced(
+						schema.GroupVersionKind(runtimeGvk.gvk),
+						lazyRestMapper,
+					)
+					g.Expect(err).NotTo(gmg.HaveOccurred())
+					g.Expect(scope).To(gmg.BeTrue())
+				})
+			}
 		})
 	}
 }