From 0f332b67f156eaff32cb67cec843260f7edba760 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Wed, 4 Sep 2019 08:32:24 -0600 Subject: [PATCH] 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 * 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 * 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 * ref: moved the scheme modification to an appropriate place. Signed-off-by: Matt Butcher * fix: turned warnings into fatal errors, fixed spelling, clear cache once Signed-off-by: Matt Butcher --- pkg/action/install.go | 79 ++++++++++++++++++++++++++++--------------- pkg/kube/client.go | 8 +++++ pkg/kube/wait.go | 33 ++++++++++++++++++ 3 files changed, 92 insertions(+), 28 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index f5c1a347e..24006b69e 100644 --- a/pkg/action/install.go +++ b/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 diff --git a/pkg/kube/client.go b/pkg/kube/client.go index dc91ff9f8..1a183f582 100644 --- a/pkg/kube/client.go +++ b/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, diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index b040f2d26..48ae4c1b5 100644 --- a/pkg/kube/wait.go +++ b/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 {