Merge 60fd496012 into d37e2e9097

pkg/action/action.go

@@ -24,10 +24,13 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"k8s.io/cli-runtime/pkg/resource"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -42,14 +45,14 @@ import (
 	"helm.sh/helm/v3/pkg/releaseutil"
 	"helm.sh/helm/v3/pkg/storage"
 	"helm.sh/helm/v3/pkg/storage/driver"
-	"helm.sh/helm/v3/pkg/time"
+	helmtime "helm.sh/helm/v3/pkg/time"
 )
 
 // Timestamper is a function capable of producing a timestamp.Timestamper.
 //
-// By default, this is a time.Time function from the Helm time package. This can
+// By default, this is a helmtime.Time function from the Helm time package. This can
 // be overridden for testing though, so that timestamps are predictable.
-var Timestamper = time.Now
+var Timestamper = helmtime.Now
 
 var (
 	// errMissingChart indicates that a chart was not provided.
@@ -228,6 +231,66 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 	return hs, b, notes, nil
 }
 
+// Install CRDs
+func (cfg *Configuration) installCRDs(crds []chart.CRD) 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 := cfg.KubeClient.Build(bytes.NewBuffer(obj.File.Data), false)
+		if err != nil {
+			return errors.Wrapf(err, "failed to install CRD %s", obj.Name)
+		}
+
+		// Send them to Kube
+		if _, err := cfg.KubeClient.Create(res); err != nil {
+			// If the error is CRD already exists, continue.
+			if apierrors.IsAlreadyExists(err) {
+				crdName := res[0].Name
+				cfg.Log("CRD %s is already present. Skipping.", crdName)
+				continue
+			}
+			return errors.Wrapf(err, "failed to install CRD %s", obj.Name)
+		}
+		totalItems = append(totalItems, res...)
+	}
+	if len(totalItems) > 0 {
+		// Give time for the CRD to be recognized.
+		if err := cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil {
+			return err
+		}
+
+		// 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 cfg.Capabilities != nil {
+			discoveryClient, err := cfg.RESTClientGetter.ToDiscoveryClient()
+			if err != nil {
+				return err
+			}
+
+			cfg.Log("Clearing discovery cache")
+			discoveryClient.Invalidate()
+
+			_, _ = discoveryClient.ServerGroups()
+		}
+
+		// Invalidate the REST mapper, since it will not have the new CRDs
+		// present.
+		restMapper, err := cfg.RESTClientGetter.ToRESTMapper()
+		if err != nil {
+			return err
+		}
+		if resettable, ok := restMapper.(meta.ResettableRESTMapper); ok {
+			cfg.Log("Clearing REST mapper cache")
+			resettable.Reset()
+		}
+	}
+	return nil
+}
+
 // RESTClientGetter gets the rest client
 type RESTClientGetter interface {
 	ToRESTConfig() (*rest.Config, error)
@@ -293,8 +356,8 @@ func (cfg *Configuration) KubernetesClientSet() (kubernetes.Interface, error) {
 // Now generates a timestamp
 //
 // If the configuration has a Timestamper on it, that will be used.
-// Otherwise, this will use time.Now().
+// Otherwise, this will use helmtime.Now().
-func (cfg *Configuration) Now() time.Time {
+func (cfg *Configuration) Now() helmtime.Time {
 	return Timestamper()
 }
 

pkg/action/install.go

@@ -34,9 +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"
 
 	"helm.sh/helm/v3/pkg/chart"
@@ -150,65 +148,6 @@ func (i *Install) GetRegistryClient() *registry.Client {
 	return i.ChartPathOptions.registryClient
 }
 
-func (i *Install) installCRDs(crds []chart.CRD) 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.File.Data), false)
-		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 install CRD %s", obj.Name)
-		}
-		totalItems = append(totalItems, res...)
-	}
-	if len(totalItems) > 0 {
-		// Give time for the CRD to be recognized.
-		if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil {
-			return err
-		}
-
-		// 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
-}
-
 // Run executes the installation
 //
 // If DryRun is set to true, this will prepare the release, but not install it
@@ -249,7 +188,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 		// On dry run, bail here
 		if i.isDryRun() {
 			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 {
+		} else if err := i.cfg.installCRDs(crds); err != nil {
 			return nil, err
 		}
 	}

pkg/action/upgrade.go

@@ -46,7 +46,7 @@ type Upgrade struct {
 
 	ChartPathOptions
 
-	// Install is a purely informative flag that indicates whether this upgrade was done in "install" mode.
+	// Install is a largely informative flag that indicates whether this upgrade was done in "install" mode.
 	//
 	// Applications may use this to determine whether this Upgrade operation was done as part of a
 	// pure upgrade (Upgrade.Install == false) or as part of an install-or-upgrade operation
@@ -55,12 +55,14 @@ type Upgrade struct {
 	// Setting this to `true` will NOT cause `Upgrade` to perform an install if the release does not exist.
 	// That process must be handled by creating an Install action directly. See cmd/upgrade.go for an
 	// example of how this flag is used.
+	//
+	// This flag does affect how CRDs are handled - CRDs will only be installed if this flag is set.
 	Install bool
 	// Devel indicates that the operation is done in devel mode.
 	Devel bool
 	// Namespace is the namespace in which this operation should be performed.
 	Namespace string
-	// SkipCRDs skips installing CRDs when install flag is enabled during upgrade
+	// SkipCRDs skips installing new CRDs during upgrade, if this upgrade was done in install mode. Note, CRDs will never be upgraded if they already exist.
 	SkipCRDs bool
 	// Timeout is the timeout for this operation
 	Timeout time.Duration
@@ -233,6 +235,23 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, err
 	}
 
+	// Determine whether or not to interact with remote
+	var interactWithRemote bool
+	if !u.isDryRun() || u.DryRunOption == "server" || u.DryRunOption == "none" || u.DryRunOption == "false" {
+		interactWithRemote = true
+	}
+
+	// Pre-install anything in the crd/ directory. We do this before Helm
+	// contacts the upstream server and builds the capabilities object.
+	if crds := chart.CRDObjects(); u.Install && !u.SkipCRDs && len(crds) > 0 {
+		// On dry run, bail here
+		if u.isDryRun() {
+			u.cfg.Log("WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.")
+		} else if err := u.cfg.installCRDs(crds); err != nil {
+			return nil, nil, err
+		}
+	}
+
 	// Increment revision count. This is passed to templates, and also stored on
 	// the release object.
 	revision := lastRelease.Version + 1
@@ -253,12 +272,6 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, err
 	}
 
-	// Determine whether or not to interact with remote
-	var interactWithRemote bool
-	if !u.isDryRun() || u.DryRunOption == "server" || u.DryRunOption == "none" || u.DryRunOption == "false" {
-		interactWithRemote = true
-	}
-
 	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithRemote, u.EnableDNS)
 	if err != nil {
 		return nil, nil, err