Do not rely on previous observations

[hiddeco]

Filled as a separate issue, but could be handled as part of #323.

At present, and as also observed by a drive-by contributor, we do not always make true observations on the Helm storage, but rather piggy back on past observations. This is not inline with the Kubernetes API conventions, but also more prone to vague errors because the world as observed may (unexpectedly) change.

An example of a state determination based on a previous observation can be found here: https://github.com/fluxcd/helm-controller/blob/main/controllers/helmrelease_controller.go#L313

It should instead confirm the Helm storage matches the expected state, and probably requires the introduction of a "storage observer" to properly capture all errors that may happen during a release. Reason this observer is required, is because Helm actions do not consistently return the (failed) release object on an error (or a revision number).

[phillebaba]

@hiddeco what is the benefit of using the old storage implementation vs the one in the Helm package? https://github.com/helm/helm/blob/main/pkg/storage/storage.go

[hiddeco]

The one from helm/helm does not provide you insights into last persisted state as performed by the Helm action. This is required to make good observations, as some actions do not return the Release object as a result (nor a revision number), making it impossible to be aware of the precise result of your own actions.

This happens in a lot of different areas, e.g. actions running into errors and persisting more information than they return. Tests being unable to target a specific revision (making it important to validate the result of the test action actually is for the same revision as you think you are reconciling, etc.

Helm's base storage wrapper can (and is, I think?) still be used together with the observer, but takes care of garbage collection in this case.