fix: clear the discovery cache after CRDs are installed (#6332)

* fix: clear the discovery cache after CRDs are installed

This fixes an issue in which a chart could not contain both a CRD and an instance of that CRD. It works around a stale cache by force cache invalidation whenever a CRD is added.

Closes #6316

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

* fix: wait for CRD to register before allowing CRDs to be installed

This fixes an issue with the previous version of this patch in which the CRD would not be available quickly enough.

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

* feat: use Wait() to wait for CRDs to be ready

This forward-ports the CRD wait logic to Helm 3, and then uses that to wait for CRDs to be registered.

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

* ref: moved the scheme modification to an appropriate place.

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

* fix: turned warnings into fatal errors, fixed spelling, clear cache once

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

pkg/action/install.go

@@ -30,6 +30,7 @@ import (
 	"github.com/Masterminds/sprig"
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/cli-runtime/pkg/resource"
 
 	"helm.sh/helm/pkg/chart"
 	"helm.sh/helm/pkg/chartutil"
@@ -103,6 +104,45 @@ func NewInstall(cfg *Configuration) *Install {
 	}
 }
 
+func (i *Install) installCRDs(crds []*chart.File) error {
+	// We do these one file at a time in the order they were read.
+	totalItems := []*resource.Info{}
+	for _, obj := range crds {
+		// Read in the resources
+		res, err := i.cfg.KubeClient.Build(bytes.NewBuffer(obj.Data))
+		if err != nil {
+			return errors.Wrapf(err, "failed to install CRD %s", obj.Name)
+		}
+
+		// Send them to Kube
+		if _, err := i.cfg.KubeClient.Create(res); err != nil {
+			// If the error is CRD already exists, continue.
+			if apierrors.IsAlreadyExists(err) {
+				crdName := res[0].Name
+				i.cfg.Log("CRD %s is already present. Skipping.", crdName)
+				continue
+			}
+			return errors.Wrapf(err, "failed to instal CRD %s", obj.Name)
+		}
+		totalItems = append(totalItems, res...)
+	}
+	// 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()
+	return nil
+}
+
 // Run executes the installation
 //
 // If DryRun is set to true, this will prepare the release, but not install it
@@ -115,6 +155,17 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.
 		return nil, err
 	}
 
+	// Pre-install anything in the crd/ directory. We do this before Helm
+	// contacts the upstream server and builds the capabilities object.
+	if crds := chrt.CRDs(); !i.ClientOnly && !i.SkipCRDs && len(crds) > 0 {
+		// On dry run, bail here
+		if i.DryRun {
+			i.cfg.Log("WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.")
+		} else if err := i.installCRDs(crds); err != nil {
+			return nil, err
+		}
+	}
+
 	if i.ClientOnly {
 		// Add mock objects in here so it doesn't use Kube API server
 		// NOTE(bacongobbler): used for `helm template`
@@ -148,34 +199,6 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.
 
 	rel := i.createRelease(chrt, vals)
 
-	// Pre-install anything in the crd/ directory
-	if crds := chrt.CRDs(); !i.SkipCRDs && len(crds) > 0 {
-		// We do these one at a time in the order they were read.
-		for _, obj := range crds {
-			// Read in the resources
-			res, err := i.cfg.KubeClient.Build(bytes.NewBuffer(obj.Data))
-			if err != nil {
-				// We bail out immediately
-				return nil, errors.Wrapf(err, "failed to install CRD %s", obj.Name)
-			}
-			// On dry run, bail here
-			if i.DryRun {
-				i.cfg.Log("WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.")
-				continue
-			}
-			// Send them to Kube
-			if _, err := i.cfg.KubeClient.Create(res); err != nil {
-				// If the error is CRD already exists, continue.
-				if apierrors.IsAlreadyExists(err) {
-					crdName := res[0].Name
-					i.cfg.Log("CRD %s is already present. Skipping.", crdName)
-					continue
-				}
-				return i.failRelease(rel, err)
-			}
-		}
-	}
-
 	var manifestDoc *bytes.Buffer
 	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.OutputDir)
 	// Even for errors, attach this if available

pkg/kube/client.go

@@ -29,7 +29,9 @@ import (
 	"github.com/pkg/errors"
 	batch "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
+	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -37,6 +39,7 @@ import (
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/cli-runtime/pkg/resource"
+	"k8s.io/client-go/kubernetes/scheme"
 	watchtools "k8s.io/client-go/tools/watch"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
@@ -55,6 +58,11 @@ func New(getter genericclioptions.RESTClientGetter) *Client {
 	if getter == nil {
 		getter = genericclioptions.NewConfigFlags(true)
 	}
+	// Add CRDs to the scheme. They are missing by default.
+	if err := apiextv1beta1.AddToScheme(scheme.Scheme); err != nil {
+		// This should never happen.
+		panic(err)
+	}
 	return &Client{
 		Factory: cmdutil.NewFactory(getter),
 		Log:     nopLogger,

pkg/kube/wait.go

@@ -27,12 +27,14 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
+	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/scheme"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
 )
 
@@ -138,6 +140,17 @@ func (w *waiter) waitForResources(created ResourceList) error {
 				if !w.daemonSetReady(ds) {
 					return false, nil
 				}
+			case *apiextv1beta1.CustomResourceDefinition:
+				if err := v.Get(); err != nil {
+					return false, err
+				}
+				crd := &apiextv1beta1.CustomResourceDefinition{}
+				if err := scheme.Scheme.Convert(v.Object, crd, nil); err != nil {
+					return false, err
+				}
+				if !w.crdReady(*crd) {
+					return false, nil
+				}
 			case *appsv1.StatefulSet, *appsv1beta1.StatefulSet, *appsv1beta2.StatefulSet:
 				sts, err := w.c.AppsV1().StatefulSets(v.Namespace).Get(v.Name, metav1.GetOptions{})
 				if err != nil {
@@ -257,6 +270,26 @@ func (w *waiter) daemonSetReady(ds *appsv1.DaemonSet) bool {
 	return true
 }
 
+func (w *waiter) crdReady(crd apiextv1beta1.CustomResourceDefinition) bool {
+	for _, cond := range crd.Status.Conditions {
+		switch cond.Type {
+		case apiextv1beta1.Established:
+			if cond.Status == apiextv1beta1.ConditionTrue {
+				return true
+			}
+		case apiextv1beta1.NamesAccepted:
+			if cond.Status == apiextv1beta1.ConditionFalse {
+				// This indicates a naming conflict, but it's probably not the
+				// job of this function to fail because of that. Instead,
+				// we treat it as a success, since the process should be able to
+				// continue.
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func (w *waiter) statefulSetReady(sts *appsv1.StatefulSet) bool {
 	// If the update strategy is not a rolling update, there will be nothing to wait for
 	if sts.Spec.UpdateStrategy.Type != appsv1.RollingUpdateStatefulSetStrategyType {