From 8f832046e258e2cb800894579b1b3b50c2d83492 Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Thu, 16 Apr 2020 03:51:55 +0530 Subject: [PATCH] Add checking of length of resourceList before creating of deleting A chart being installed which only contains CRDs and not any templates tries to install the resources by default. The resourceList which is used in this case does not check if there are resources present in it or not. This commit adds checks to those particular places where we need to check if the size of resourceList > 0 during installation and deletion. Signed-off-by: Vibhav Bobade (cherry picked from commit 47abe66fd29162a71627ee4c9d26a9c3712e24e1) (cherry picked from commit fe51cd1e31e6a202cba7dead9552a6d418ded79a) --- cmd/helm/install_test.go | 5 +++ .../chart-with-only-crds/.helmignore | 23 ++++++++++++ .../chart-with-only-crds/Chart.yaml | 21 +++++++++++ .../chart-with-only-crds/crds/test-crd.yaml | 19 ++++++++++ pkg/action/install.go | 36 ++++++++++--------- pkg/action/uninstall.go | 7 ++-- 6 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/chart-with-only-crds/.helmignore create mode 100644 cmd/helm/testdata/testcharts/chart-with-only-crds/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-only-crds/crds/test-crd.yaml diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index e3013d713..7a101940f 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -195,6 +195,11 @@ func TestInstall(t *testing.T) { cmd: "install aeneas testdata/testcharts/deprecated --namespace default", golden: "output/deprecated-chart.txt", }, + // Install chart with only crds + { + name: "install chart with only crds", + cmd: "install crd-test testdata/testcharts/chart-with-only-crds --namespace default", + }, } runTestActionCmd(t, tests) diff --git a/cmd/helm/testdata/testcharts/chart-with-only-crds/.helmignore b/cmd/helm/testdata/testcharts/chart-with-only-crds/.helmignore new file mode 100644 index 000000000..0e8a0eb36 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-only-crds/.helmignore @@ -0,0 +1,23 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*.orig +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ diff --git a/cmd/helm/testdata/testcharts/chart-with-only-crds/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-only-crds/Chart.yaml new file mode 100644 index 000000000..a8b4c2022 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-only-crds/Chart.yaml @@ -0,0 +1,21 @@ +apiVersion: v2 +name: crd-test +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. +appVersion: 1.16.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-only-crds/crds/test-crd.yaml b/cmd/helm/testdata/testcharts/chart-with-only-crds/crds/test-crd.yaml new file mode 100644 index 000000000..1d7350f1d --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-only-crds/crds/test-crd.yaml @@ -0,0 +1,19 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: tests.test.io +spec: + group: test.io + names: + kind: Test + listKind: TestList + plural: tests + singular: test + scope: Namespaced + versions: + - name : v1alpha2 + served: true + storage: true + - name : v1alpha1 + served: true + storage: false diff --git a/pkg/action/install.go b/pkg/action/install.go index 10a9644dd..351e0928c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -144,20 +144,24 @@ func (i *Install) installCRDs(crds []chart.CRD) error { } 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 + 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() } - // Make sure to force a rebuild of the cache. - discoveryClient.ServerGroups() return nil } @@ -264,7 +268,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // we'll end up in a state where we will delete those resources upon // deleting the release because the manifest will be pointing at that // resource - if !i.ClientOnly && !isUpgrade { + if !i.ClientOnly && !isUpgrade && len(resources) > 0 { toBeAdopted, err = existingResourceConflict(resources, rel.Name, rel.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") @@ -329,11 +333,11 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // At this point, we can do the install. Note that before we were detecting whether to // do an update, but it's not clear whether we WANT to do an update if the re-use is set // to true, since that is basically an upgrade operation. - if len(toBeAdopted) == 0 { + if len(toBeAdopted) == 0 && len(resources) > 0 { if _, err := i.cfg.KubeClient.Create(resources); err != nil { return i.failRelease(rel, err) } - } else { + } else if len(resources) > 0 { if _, err := i.cfg.KubeClient.Update(toBeAdopted, resources, false); err != nil { return i.failRelease(rel, err) } diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index dfaa98472..a51a283d6 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -169,6 +169,7 @@ func joinErrors(errs []error) string { // deleteRelease deletes the release and returns manifests that were kept in the deletion process func (u *Uninstall) deleteRelease(rel *release.Release) (string, []error) { + var errs []error caps, err := u.cfg.getCapabilities() if err != nil { return rel.Manifest, []error{errors.Wrap(err, "could not get apiVersions from Kubernetes")} @@ -194,11 +195,13 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (string, []error) { for _, file := range filesToDelete { builder.WriteString("\n---\n" + file.Content) } + resources, err := u.cfg.KubeClient.Build(strings.NewReader(builder.String()), false) if err != nil { return "", []error{errors.Wrap(err, "unable to build kubernetes objects for delete")} } - - _, errs := u.cfg.KubeClient.Delete(resources) + if len(resources) > 0 { + _, errs = u.cfg.KubeClient.Delete(resources) + } return kept, errs }