Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use existing helm template logic to populate manifests instead of dry-run #1718

Merged
merged 9 commits into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## HEAD (Unreleased)
- Fix handling of charts with empty manifests (https://github.com/pulumi/pulumi-kubernetes/pull/1717)
- Use existing helm template logic to populate manifests instead of relying on `dry-run` support (https://github.com/pulumi/pulumi-kubernetes/pull/1718)

## 3.7.1 (September 10, 2021)
- Don't replace PVC on .spec.resources.requests or .limits change. (https://github.com/pulumi/pulumi-kubernetes/pull/1705)
Expand Down
85 changes: 59 additions & 26 deletions provider/pkg/provider/helm_release.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,11 @@ func (r *helmReleaseProvider) Check(ctx context.Context, req *pulumirpc.CheckReq
new.Keyring = os.ExpandEnv("$HOME/.gnupg/pubring.gpg")
}

var templateRelease bool
if len(olds.Mappable()) > 0 {
adoptOldNameIfUnnamed(new, old)
if err = r.helmUpdate(ctx, urn, news, new, old, true); err != nil {
if !strings.Contains(err.Error(), "failed to download") {
return nil, err
}
}

templateRelease = true
} else {
assignNameIfAutonameable(new, news, "release")
conf, err := r.getActionConfig(new.Namespace)
Expand All @@ -286,13 +284,50 @@ func (r *helmReleaseProvider) Check(ctx context.Context, req *pulumirpc.CheckReq
return nil, err
}
if !exists {
if err = r.helmCreate(ctx, urn, news, new, true); err != nil {
return nil, err
}
templateRelease = true
}
// If resource exists, we are likely doing an import. We will just pass the inputs through.
}

if templateRelease {
helmHome := os.Getenv("HELM_HOME")

helmChartOpts := HelmChartOpts{
HelmFetchOpts: HelmFetchOpts{
CAFile: new.RepositoryOpts.CAFile,
CertFile: new.RepositoryOpts.CertFile,
Devel: new.Devel,
Home: helmHome,
KeyFile: new.RepositoryOpts.KeyFile,
Keyring: new.Keyring,
Password: new.RepositoryOpts.Password,
Repo: new.RepositoryOpts.Repo,
Username: new.RepositoryOpts.Password,
Version: new.Version,
},
APIVersions: nil,
Chart: new.Chart,
IncludeTestHookResources: true,
SkipCRDRendering: new.SkipCrds,
Namespace: new.Namespace,
Path: "",
ReleaseName: new.Name,
Values: new.Values,
Version: new.Version,
}
templ, err := helmTemplate(helmChartOpts)
if err != nil {
return nil, err
}

_, resources, err := convertYAMLManifestToJSON(templ)
if err != nil {
return nil, err
}

new.ResourceNames = resources
}

autonamed := resource.NewPropertyMap(new)
annotateSecrets(autonamed, news)
autonamedInputs, err := plugin.MarshalProperties(autonamed, plugin.MarshalOptions{
Expand All @@ -309,7 +344,7 @@ func (r *helmReleaseProvider) Check(ctx context.Context, req *pulumirpc.CheckReq
return &pulumirpc.CheckResponse{Inputs: autonamedInputs}, nil
}

func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN, news resource.PropertyMap, newRelease *Release, dryrun bool) error {
func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN, news resource.PropertyMap, newRelease *Release) error {
conf, err := r.getActionConfig(newRelease.Namespace)
if err != nil {
return err
Expand Down Expand Up @@ -361,7 +396,6 @@ func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN,

client.ChartPathOptions = *cpo
client.ClientOnly = false
client.DryRun = dryrun // Dry-run == preview.
client.DisableHooks = newRelease.DisableWebhooks
client.Wait = !newRelease.SkipAwait
client.WaitForJobs = !newRelease.SkipAwait && newRelease.WaitForJobs
Expand All @@ -381,7 +415,7 @@ func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN,
client.Description = newRelease.Description
client.CreateNamespace = newRelease.CreateNamespace

if cmd := newRelease.Postrender; cmd != "" && !dryrun {
if cmd := newRelease.Postrender; cmd != "" {
pr, err := postrender.NewExec(cmd)

if err != nil {
Expand All @@ -408,7 +442,7 @@ func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN,
return err
}

if err := setReleaseAttributes(newRelease, rel, dryrun); err != nil {
if err := setReleaseAttributes(newRelease, rel, false); err != nil {
return err
}

Expand All @@ -417,11 +451,11 @@ func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN,

}

err = setReleaseAttributes(newRelease, rel, dryrun)
err = setReleaseAttributes(newRelease, rel, false)
return err
}

func (r *helmReleaseProvider) helmUpdate(ctx context.Context, urn resource.URN, news resource.PropertyMap, newRelease, oldRelease *Release, dryrun bool) error {
func (r *helmReleaseProvider) helmUpdate(ctx context.Context, urn resource.URN, news resource.PropertyMap, newRelease, oldRelease *Release) error {
cpo, chartName, err := chartPathOptions(newRelease)
if err != nil {
return err
Expand Down Expand Up @@ -473,7 +507,6 @@ func (r *helmReleaseProvider) helmUpdate(ctx context.Context, urn resource.URN,
client.Namespace = newRelease.Namespace
client.Timeout = time.Duration(newRelease.Timeout) * time.Second
client.Wait = !newRelease.SkipAwait
client.DryRun = dryrun // do not apply changes
client.DisableHooks = newRelease.DisableCRDHooks
client.Atomic = newRelease.Atomic
client.SubNotes = newRelease.RenderSubchartNotes
Expand All @@ -489,7 +522,7 @@ func (r *helmReleaseProvider) helmUpdate(ctx context.Context, urn resource.URN,
client.CleanupOnFail = newRelease.CleanupOnFail
client.Description = newRelease.Description

if cmd := newRelease.Postrender; cmd != "" && !dryrun {
if cmd := newRelease.Postrender; cmd != "" {
pr, err := postrender.NewExec(cmd)

if err != nil {
Expand All @@ -503,10 +536,10 @@ func (r *helmReleaseProvider) helmUpdate(ctx context.Context, urn resource.URN,
logger.V(9).Infof("No existing release found.")
return err
} else if err != nil {
return fmt.Errorf("error running dry run update: %w", err)
return fmt.Errorf("error running update: %w", err)
}

err = setReleaseAttributes(newRelease, rel, dryrun)
err = setReleaseAttributes(newRelease, rel, false)
return err
}

Expand Down Expand Up @@ -576,10 +609,6 @@ func (r *helmReleaseProvider) Diff(ctx context.Context, req *pulumirpc.DiffReque
}
newRelease.Status.Status = release.StatusDeployed.String()

if err = r.helmUpdate(ctx, urn, news, newRelease, oldRelease, true); err != nil {
return nil, err
}

oldInputsJSON, err := json.Marshal(oldInputs.Mappable())
if err != nil {
return nil, err
Expand Down Expand Up @@ -712,8 +741,10 @@ func (r *helmReleaseProvider) Create(ctx context.Context, req *pulumirpc.CreateR
return nil, err
}

if err = r.helmCreate(ctx, urn, news, newRelease, req.GetPreview()); err != nil {
return nil, err
if !req.GetPreview() {
if err = r.helmCreate(ctx, urn, news, newRelease); err != nil {
return nil, err
}
}

obj := checkpointRelease(news, newRelease)
Expand Down Expand Up @@ -874,8 +905,10 @@ func (r *helmReleaseProvider) Update(ctx context.Context, req *pulumirpc.UpdateR
return nil, err
}

if err = r.helmUpdate(ctx, urn, newResInputs, newRelease, oldRelease, req.GetPreview()); err != nil {
return nil, err
if !req.GetPreview() {
if err = r.helmUpdate(ctx, urn, newResInputs, newRelease, oldRelease); err != nil {
return nil, err
}
}

checkpointed := checkpointRelease(newResInputs, newRelease)
Expand Down
4 changes: 2 additions & 2 deletions provider/pkg/provider/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
package provider

import (
"github.com/pulumi/pulumi-kubernetes/provider/v3/pkg/clients"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/client-go/discovery"
memcached "k8s.io/client-go/discovery/cached/memory"
"k8s.io/client-go/rest"
"k8s.io/client-go/restmapper"
"k8s.io/client-go/tools/clientcmd"
Expand All @@ -24,7 +24,7 @@ func (k *KubeConfig) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, er
// double it just so we don't end up here again for a while. This config is only used for discovery.
k.restConfig.Burst = 100

return memcached.NewMemCacheClient(discovery.NewDiscoveryClientForConfigOrDie(k.restConfig)), nil
return clients.NewMemCacheClient(discovery.NewDiscoveryClientForConfigOrDie(k.restConfig)), nil
}

// ToRESTConfig implemented interface method
Expand Down
21 changes: 21 additions & 0 deletions tests/sdk/nodejs/examples/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,27 @@ func TestHelmRelease(t *testing.T) {
integration.ProgramTest(t, &test)
}

func TestHelmReleaseCRD(t *testing.T) {
// Validate that Helm charts with CRDs work across create/update/refresh/delete cycles.
// https://github.com/pulumi/pulumi-kubernetes/issues/1712
skipIfShort(t)
test := getBaseOptions(t).
With(integration.ProgramTestOptions{
Dir: filepath.Join(getCwd(t), "helm-release-crd", "step1"),
SkipRefresh: false,
Verbose: true,
EditDirs: []integration.EditDir{
{
Dir: filepath.Join(getCwd(t), "helm-release-crd", "step2"),
Additive: true,
},
},
})

integration.ProgramTest(t, &test)
}


func skipIfShort(t *testing.T) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
Expand Down
3 changes: 3 additions & 0 deletions tests/sdk/nodejs/examples/helm-release-crd/step1/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: helm-release-crd
runtime: nodejs
description: A minimal Kubernetes TypeScript Pulumi program
12 changes: 12 additions & 0 deletions tests/sdk/nodejs/examples/helm-release-crd/step1/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as k8s from "@pulumi/kubernetes";

const prometheusRelease = new k8s.helm.v3.Release("prometheus", {
name: "kube-prometheus-stack",
chart: "kube-prometheus-stack",
version: "18.0.10",
repositoryOpts: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably pin to a specific version just in case upstream breaks something in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinned version.

repo: "https://prometheus-community.github.io/helm-charts",
},
values: {},
});

11 changes: 11 additions & 0 deletions tests/sdk/nodejs/examples/helm-release-crd/step1/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "helm-release-crd",
"devDependencies": {
"@types/node": "^10.0.0"
},
"dependencies": {
"@pulumi/pulumi": "^3.0.0",
"@pulumi/kubernetes": "latest",
"@pulumi/kubernetesx": "^0.1.5"
}
}
18 changes: 18 additions & 0 deletions tests/sdk/nodejs/examples/helm-release-crd/step1/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"compilerOptions": {
"strict": true,
"outDir": "bin",
"target": "es2016",
"module": "commonjs",
"moduleResolution": "node",
"sourceMap": true,
"experimentalDecorators": true,
"pretty": true,
"noFallthroughCasesInSwitch": true,
"noImplicitReturns": true,
"forceConsistentCasingInFileNames": true
},
"files": [
"index.ts"
]
}
16 changes: 16 additions & 0 deletions tests/sdk/nodejs/examples/helm-release-crd/step2/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import * as k8s from "@pulumi/kubernetes";

const prometheusRelease = new k8s.helm.v3.Release("prometheus", {
name: "kube-prometheus-stack",
chart: "kube-prometheus-stack",
version: "18.0.10",
repositoryOpts: {
repo: "https://prometheus-community.github.io/helm-charts",
},
values: {
commonLabels: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: add a comment somewhere about the purpose of this test (what step2 is verifying)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"test": "true", // Add an additional label to pods
}
},
});