Skip to content

Commit

Permalink
✨ Use aggregated discovery if available
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alvaroaleman committed Sep 15, 2024
1 parent e6c3d13 commit 352b901
Show file tree
Hide file tree
Showing 4 changed files with 855 additions and 746 deletions.
234 changes: 119 additions & 115 deletions pkg/client/apiutil/apimachinery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package apiutil_test

import (
"context"
"strconv"
"testing"

gmg "github.com/onsi/gomega"
Expand All @@ -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())
})
}
})
}
}
Expand Down
Loading

0 comments on commit 352b901

Please sign in to comment.