Add .spec.wait field to HelmRelease

[nweedo3]

This field instructs the operator to wait for successful installation
and/or upgrade of the specified Helm chart before signalling a result
in the owned HelmRelease resource. This also fixes a bug which only
enforces a wait on upgrade using the .spec.rollback.enable field.

[hiddeco]

This also fixes a bug which only enforces a wait on upgrade using the .spec.rollback.enable field.

This is not a bug but something that has been put thought into. I do however not know the reason from the top of my head, will report back about this later.

[nweedo3]

@hiddeco yeah, when I took a second look at it earlier I couldn't be sure either way. I'll leave the PR in this state for now until you get back to me. When you know the initial reason for setting the upgrade wait to hr.Spec.Rollback.Enable, I'll update the PR with this knowledge and any extra changes you recommend.

Thanks for the initial review!

[hiddeco]

@niall-weedon I discussed this with other team members. The reason we haven't included this is because for not so long ago the Helm operator only had a single worker, and this would block the processing of releases for the whole queue for ~300 sec.

Today we support running multiple workers, and have a default of 2, but even with a multiple, statically determined number of workers, it's not a great idea to block for arbitrary durations.

Please note that the status of the HelmRelease resources is updated by a background process, so the status of the release, although already synced by the operator, will eventually be updated may the status change post-sync.

[nweedo3]

@hiddeco thanks for getting back to me. I understand that waiting will block the process - in this case how would I be able to check that the resources the HelmRelease deploys are ready? As far as I understand, on a fresh deployment each HelmRelease will go to HelmReleaseReleased as soon as it is pushed into Helm without waiting for the helm-generated resources to become healthy.

[nweedo3]

@hiddeco do you have any suggestions on my question above? Thanks.

[hiddeco]

@niall-weedon for the current version in master I do not have a direct answer, except for watching the resources that are the result of the release itself (this is painful, I know).

For the upcoming Helm operator version with v3 support, which is in development in the helm-v3-dev branch, I think we will actually wait by default to utilize the --atomic capabilities during installation. This option prunes a release if the installation fails (and thus also automatically waits for it), which is something we do ourselves right now in the thus far released versions of the operator.

My plan is to backport this feature in our v2 client implementation, to make v3 a first class citizen and reduce our own code complexity (see commit message and TODO here).

For upgrades --atomic is not desired as this performs an automatic rollback, which is not something an operator can safely do without opting-in because of e.g. StatefulSet resources. Given that Helm v3 performs much better, and the wait time is likely decreased for most charts compared to v2, I am willing to rethink what I said in my previous comment, and will take your note about observability into account.

Please bear with me until then 🌞

[hiddeco]

Given the recent developments it makes no sense to hold this back so I have rebased the PR and made some small adjustments to make it work again.

Thanks for your contribution and patience 🥇