This is a regression bug fix due to the introduction of the default
values in the source-controller CRDs.
Signed-off-by: Hidde Beydals <hello@hidde.co>
The reason for this is the `EnqueueRequestsFromMapFunc` calling the
enqueuer for _both_ the old and the new object, and we only want to act
on the ones that contain a revision different from the one that we have
recorded in the status object of the `HelmRelease`.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This prevents the resources from getting annotated, and instead uses
the `handler.EnqueueRequestsFromMapFunc` to queue requests based on
changes to the source objects.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This is an initial implementation for cross-cluster Helm release
support that relies on a KubeConfig secret, and a reference to it in
the HelmRelease resource.
If set, all actions taken by the Helm runner are executed using the
KubeConfig from the secret. The Helm storage is stored on the remote
cluster in a namespace that equals to the namespace of the HelmRelease
in the managing cluster, the release itself is made in either this
namespace, or the configured TargetNamespace. In any case, both are
expected to exist and/or created beforehand.
Other references to Kubernetes resources in the HelmRelease, like
ValuesReference resources, are expected to exist on the managing
cluster.
By confirming the observed generation is up-to-date before checking
the `Ready` condition state after chart reconciliation, we guarantee
the chart we use for the release always is on par with the state of
the `HelmRelease`.
In addition, we requeue to prevent the `HelmRelease` getting stuck
in a "HelmChart is not ready" state. Previously triggered by a no-op
chart update without a revision change that caused the watcher to
not request a reconciliation for the `HelmRelease`.
As `ParseInto` expects the destination to be initialized, which led
to an `unable to parse key: assignment to entry in nil map` error in
configurations where the first `ValuesReference` had a `targetPath`
defined.
As the existing logic relied on other conditions that caused the
`released` to never be nil, but this may not be true for future
iterations, which makes this a case of better safe than sorry.
This makes it possible for e.g. the GOTK CLI to observe if the
controller has handled the resource since the manual reconciliation
request was made. It replaces the `LastObservedTime` status field,
as this was prone to time skew issues and does not offer much additional
value over the timestamps of the conditions.
We were logging the spec interval duration, which was incorrect:
1. On failures, which use exponential backoff
2. On dependency not ready, which uses a separately defined static interval.
This changes to log result.RequeueAfter directly when set.
This ensures that a subsequent update to a values reference during
the merging process does not result in an unexpected mixture of old(er)
and new values.
* Move `ReleaseRevision` function to util
* Rename `release` method to `reconileRelease` to match
`reconcileChart`
* Refactor chart artifact download to make use of a temporary file,
which is removed as soon as the tarbal has been loaded into memory
This commit adds support for optional values references, as discussions
have brought to light that there are some valid use cases in which
having this option is desirable. For example to allow a user to extend
behaviour at a later date with their own repository without modifying
the HelmRelease object.
When a values reference is marked as optional, not found errors for the
value reference are ignored, but any ValuesKey, TargetPath or transient
error will still result in a reconciliation failure.
This also removes the sorting from the `HelmChartWatcher`, as with
the current `HelmChartTemplateSpec` a chart is only used by a single
`HelmRelease`. Rendering the action obsolete.
This removes:
- Installed, Upgraded, RolledBack, and Uninstalled status conditions
since they did not represent current state, but rather actions
taken, which are already recorded by events.
- status.observedStateReconciled since it solved the problem of
remembering past release (install/upgrade/test) success, but not
past release failures, after other subsequent failures such as
dependency failures, k8s API failures, etc.
This adds:
- Remediated status condition which records whether the release is
currently in a remediated state. It is used to prevent release retries
after remediation failures. We were previously not doing this for
rollback failures.
- Released status condition which records whether the current state
has been successfully released (install/upgrade/test). This is used to
remember the last release attempt status, regardless of any subsequent
other failures such as dependency failures, k8s API failures, etc.
This renames:
- Tested > TestsSuccess status condition, for forward compatibility
with interval based helm tests.
During high custom resource count / low interval tests, I was greated
with a `cannot patch resource "events"` message. This happened due to
event compaction, where it will perform a patch instead of a create.
By giving the role the permission to do so this should no longer pose
a problem.
This adds a .status.lastObservedTime field which records when the
HelmRelease was last observed by the controller. This allows the user
to observe whether the spec.interval and reconcileAt annotations are
triggering reconciliation attempts as desired.
- Ensure upgrade actually occurs if known state was not reached
for any reason (other than install failure).
- After transient failures not tied to new state application, ensure
spurious upgrades do not occur and ready state is again reached,
by remembering that the known state was already successfully applied.
- Reset failure counts after success so they're not stale.
- Only lookup post-deployment release revision on remediation,
since otherwise we already have it.
- Push status update after finding new state so user can observe.
This commit adds support for conditional remediation, enabling the user
to:
* configure if test failures should be ignored
* configure what action should taken when a Helm install or upgrade
action fails (e.g. rollback, uninstall)
* configure if a failed Helm action should be retried
* configure if a failed release should be kept for debugging purposes
The previous behaviour where failed Helm tests did not mark the
`HelmRelease` as not `Ready` has changed, it now marks them as failed
by default.
Co-authored-by: Hidde Beydals <hello@hidde.co>
After taking a closer look at the Helm code, this is the behaviour
we are actually after, given we want to mimic the merging behaviour
of the -f flag available to Helm install and/or upgrade commands.
To allow the Helm Operator and helm-controller HelmReleases to
co-exist in the cluster, while being handled by separate controllers
during e.g. the migration period.
This is not possible without using another domain due to how Custom
Resource Definitions work, as newer API versions are seen as a
replacement of older versions, and are expected to be handled by a
single controller.
As "condition type names should describe the current observed state of
the resource, rather than describing the current state transitions".
Described by the draft convention for update conditions in
kubernetes/community#4521.
Hidde Beydals
Michael Bridgen
Stefan Prodan
Sean Eagan
Sean Eagan