Skip to content

Commit

Permalink
Properly invalidate client after CRD install
Browse files Browse the repository at this point in the history
As the CRDs are installed before the capabilities are gathered, the
current call to invalidate the discovery client is premature and
expensive.

What actually is required is an invalidation of the REST mapper, as
otherwise the Helm install action may later on fail with a `resource
mapping not found` error. More specifically when the caller of the
action is making use of a persisting[1] `RESTClientGetter`.

Which is not something done by the Helm CLI (albeit it could, and this
would potentially save quite some resources?). But is a default
configuration offered by the Helm SDK via `kube.New` when a nil value
is provided as the `getter`.

[1]: https://github.com/kubernetes/cli-runtime/blob/v0.26.2/pkg/genericclioptions/config_flags.go#L118

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
  • Loading branch information
hiddeco committed Mar 7, 2023
1 parent e007c90 commit 68f7b1f
Showing 1 changed file with 28 additions and 11 deletions.
39 changes: 28 additions & 11 deletions pkg/action/install.go
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/pkg/errors"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/cli-runtime/pkg/resource"
"sigs.k8s.io/yaml"
Expand Down Expand Up @@ -159,22 +160,38 @@ func (i *Install) installCRDs(crds []chart.CRD) error {
totalItems = append(totalItems, res...)
}
if len(totalItems) > 0 {
// Invalidate the local cache, since it will not have the new CRDs
// present.
discoveryClient, err := i.cfg.RESTClientGetter.ToDiscoveryClient()
if err != nil {
return err
}
i.cfg.Log("Clearing discovery cache")
discoveryClient.Invalidate()
// Give time for the CRD to be recognized.

if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil {
return err
}

// Make sure to force a rebuild of the cache.
discoveryClient.ServerGroups()
// If we have already gathered the capabilities, we need to invalidate
// the cache so that the new CRDs are recognized. This should only be
// the case when an action configuration is reused for multiple actions,
// as otherwise it is later loaded by ourselves when getCapabilities
// is called later on in the installation process.
if i.cfg.Capabilities != nil {
discoveryClient, err := i.cfg.RESTClientGetter.ToDiscoveryClient()
if err != nil {
return err
}

i.cfg.Log("Clearing discovery cache")
discoveryClient.Invalidate()

_, _ = discoveryClient.ServerGroups()
}

// Invalidate the REST mapper, since it will not have the new CRDs
// present.
restMapper, err := i.cfg.RESTClientGetter.ToRESTMapper()
if err != nil {
return err
}
if resettable, ok := restMapper.(meta.ResettableRESTMapper); ok {
i.cfg.Log("Clearing REST mapper cache")
resettable.Reset()
}
}
return nil
}
Expand Down

0 comments on commit 68f7b1f

Please sign in to comment.