Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Helm v3 support #156

Merged
merged 89 commits into from
Dec 20, 2019
Merged

Helm v3 support #156

merged 89 commits into from
Dec 20, 2019

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Dec 20, 2019

This PR merges helm-v3-dev into master, and thereby promotes the support for Helm v3 from alpha to beta.

All work done and available in the helm-v3-dev range will be available in the next release candidate (RC5).

hiddeco and others added 30 commits September 23, 2019 15:11
This adds alpha support for Helm v3 by adding a `helmVersion` to
the `HelmRelease` custom resource (valid choices `v2` and `v3`);
for backwards compatibility the default `helmVersion` is `v2`.

Support for both versions is achieved through a `helm.Client`
abstraction interface, with implementations for both versions.

In addition there have been some improvements with regard to
logging metadata, and the Helm v2 client has been upgraded to the
latest version.

Things that still require our attention:

- tests for the client implementations
- extended e2e tests for both Helm versions
- split of the synchronization of charts and helm releases
Alpha version of Helm v3 support
With techniques borrowed from `kubernetes-sigs/kind`.
Refactor `hack/` to speed up code generation
Enable Helm v3 prerelease image pushes
Do not prefix image tag in CircleCI config
To make Helm v3 a first class citizen, it would be better to mimic
the `--atomic` behaviour from Helm v3 in the v2 client code, so that
the release is removed automagically if the installation fails.

This is however outside the scope of the PR this commit is part of.
The previous metric kept track of the time it would take to perform
a Helm action, and some other actions around the synchronization of
the `HelmRelease` resource to the cluster, but would not give a real
insight in the total amount of time it would take to process a
`HelmRelease` resource.

This commit slightly tweaks the metric, so that it now measures the
total amount of time it takes to synchronize a single `HelmRelease`
resource to the cluster, and drops labels that due to this change
have become irrelevant.
* remove obsolete `runtime.HandleCrash()` which was still included
  because of historical reasons
* remove unnecessary mutex initialization
* flatten return into something more Golang idiomatic
Plus various alignments around naming things.
If the atomic option is set during upgrades, Helm v3 will automatically
rollback the release if it fails, which is not something we are after
as this can be dangerous for e.g. charts that make use of `StatefulSet`
resources.

During installation however we _do_ want to use this flag, as this
automatically removes a failed release for us, so that we do not have
to perform or implement this logic ourselves.
Plus remove the explicit structure and instead return the `*git.Export`
and HEAD reference string explicitly.
This commit brings the leakage of mirrors close to zero, as the
mirroring will be stopped if there are no references to it from
`HelmRelease` resources on the first signal received.
Decouple release reconciliation from chart source sync
This commit adds proper support for enabling Helm client versions
and configuring the default Helm client version.

The enabled versions can be set by either setting the
`--enabled-helm-versions` flag, or the `HELM_VERSION` environment
variable.

The first version given will become the default client for
`HelmRelease` resources that do not have a `helmVersion` declared in
their spec.
Proper support for enabling Helm client versions
hiddeco and others added 21 commits December 13, 2019 14:44
chartsync: use target helm version for download
e2e: parallelize end-to-end tests
This ensures the `status.HasRolledBack` check (and especially the
timestamp comparison that is made there) is accurate, as without this
change it will be updated whenever a release is processed, resulting
in spurious rollbacks for `RepoChartSource` releases.
The validators were no longer beneficial to the user as they are passed
regardless of the version targeted by the release, due to the refactor
of the `release` package.

The amount of files was also getting out of hand and making it
difficult to navigate through the package. Therefor all operation
options have been bundled in `options.go`, and a descriptive comment
has been added about how they should be implemented.
As this better reflects its purpose. The switch statement has also
been changed to a simpler if statement, as `StatusSuperseded` would
give a false positive (in case the release status is superseded, the
status of the latest release should be consulted).
Due to the server side validation Helm v3 introduced, we were forced
into running it with the `ClientOnly` option enabled as else it would
complain about cluster wide resources already existing in the cluster.

The goal of the dry-run is to determine if it changes anything to
what we are currently running, and thus if we should proceed with
the release. This commit changes the dry-run from install to upgrade,
with the configured release name instead of the UID of the
`v1.HelmRelease`.

For releases targeting Helm v3 this also ensures that we only perform
an upgrade if there are no validation errors which restores the rollback
logic. As before this change validation errors would not result in an
actual release but we would still attempt to perform a rollback,
resulting in either a rollback to an incorrect version, or worse, an
accidental uninstall.
e2e: test git and chart repository rollbacks
To ensure chart changes without a version bump to e.g. charts from
git sources are detected.
helm: include chart files, templates and deps
@hiddeco hiddeco added the helm v3 Issue or PR related to Helm v3 label Dec 20, 2019
@2opremio
Copy link
Contributor

🏅 🎖 🥇 Awesome job getting this out!

@stefanprodan
Copy link
Member

I see "Validation error does not trigger rollback" is the test flaky?

@hiddeco
Copy link
Member Author

hiddeco commented Dec 20, 2019

@stefanprodan it suddenly started failing after the merge to helm-v3-dev, it succeeds however locally (ran it multiple times) so I am clueless at this point.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM 🎖

@hiddeco hiddeco merged commit df100c5 into master Dec 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
helm v3 Issue or PR related to Helm v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants