Repository navigation
(pre-0.8) add-ons, cilium should have retries configuration #195
Description
The addons fail once in a while when I'm testing cluster creation, and land in a state where they can only be recovered by an admin (eg. with flux reconcile hr -n tenant-root kubernetes-cluster-cilium --force) and that is undesirable.
interval: 1m
install:
remediation:
retries: -1
upgrade:
remediation:
retries: -1This configuration I found in the system packages should solve it. I can try to submit a PR for this, I don't think we need the CRD bits for the addons, (or if they are needed I'm sure they are already in use - we could check)
I'd also like to suggest that a setting of interval: 1m is quite aggressive for 30 HelmReleases, it works OK, but it is putting a lot of load on the cluster's control plane for little benefit to run so often. A setting of interval: 10m across the board, with perhaps timeout: 5m is very reasonable.
This would enable us to dial the concurrency down for HelmReleases too (we're using --concurrent=20 now.) A concurrency setting of 10 or even less would be fine with these changes - there's nothing wrong with setting 20, but with 30 helmreleases on the cluster, and 1m interval, the Helm controller is going to be reconciling a HelmRelease on average once every 2s.
That's way too often! Once every 20s is way more reasonable. I bet if we look at the average dry-run time when there are no changes it's close to 2s (I haven't checked to be sure). So it's reconciling helmreleases literally continuously. I can tell from the way CPU fans are spinning 😵💫
The fact is Helm uses a lot of memory and is quite slow for what it does, fortunately there is not a long history in our index.yaml so the legacy HelmRepository is not gonna cost much, (plus later versions of Flux have resolved a lot of the performance issue)
I think on larger clusters you may not notice this at all, but my home lab cluster is a couple of potatoes strapped with 64GB RAM so I feel it.
Activity
kingdonb commentedon Jul 3, 2024
It looks like 0.8 is in the process of being tagged, maybe there's still time to address this 🤞
kingdonb commentedon Jul 3, 2024
The only downside that I can see of setting
interval: 10mis that helmrelease which times out will wait until 10m to retry, instead of retrying attimeout: 5m(the default timeout for HelmReleases)So I propose setting
interval: 5mas that will not wait the extra 5m to retry - I'm creating a PR for this now.kingdonb commentedon Jul 3, 2024
I've just tested the fix proposed in #196 from end to end locally (manual test)
It's A-OK. The ingress-nginx addon fails until cert-manager is ready to handle this:
It retries a few times on my cluster, (where the nodes are potatoes and most connections go out to the internet over Wi-Fi)
Then it succeeds, and no errors are shown any time.
If you think that
retries: -1is too much for cluster addons, I'd suggest at leastretries: 5maybe more - because ingress-nginx is clearly able to try and fail a couple of times before cert-manager is ready, it doesn't wait for timeout to fail. It terminates immediately upon theFailedMountwarning from above.But with the changes in #196 it seems to go smoothly! 🚀 for 0.8 hopefully
kingdonb commentedon Jul 3, 2024
I think we can solve the ingress-nginx addon errors with a conditional dependency. If cert-manager is enabled then ingress-nginx should depend on cert-manager so the failure conditions are avoided. This way we could set a lower (finite) number of retries, I have mixed feelings about indefinite retries, (but the failure remediation should be forgiving, per the docs)
Fine-tuning Flux configuration (cozystack#196)