This repository has been archived by the owner on Nov 1, 2022. It is now read-only.
Rewrite operator to use chartsync #1254
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@squaremo I've found a problem, if I do |
Kubernetes' client-go package uses glog, which obnoxiously installs its own flags into flag.CommandLine, then complains if you don't call `flag.Parse()`. Since we use pflag instead of flag, to shut glog up we have to give it what it wants by supplying a dummy set of args (just the one that makes it log only to stderr).
Both the chartsync package and the operator package need to install or upgrade helm releases depending on the definitions given in FluxHelmRelease resources. To deduplicate, drive these operations through the chartsync, by factoring out the bit which compares a FluxHelmRelease to the state of a release, and takes action if necessary. The operator package also deletes releases, when it's told a FluxHelmRelease has been deleted. So that we only have one thing through which we operate on the cluster, I've added that to chartsync as well. When starting, the operator is told about all existing FluxHelmRelease resources as though they had been added; previously this meant that it would (often redundantly) upgrade all the associated releases. Since the reconcilation compares what would be released with what _is_ released, before issuing an upgrade, a side-effect of this factoring is that the redundant upgrades aren't done.
If we just clone the repo at the branch HEAD whenever the operator wants to apply a change, we may end up undoing, or redoing, work, since the ChartChangeSync only advances the revision it looks at when it's ready to. To avoid that, pass the desired reconcilations to the ChartChangeSync and have it do the work, at the revision it's currently working from. The mechanism is to send the resource in question on an unbuffered channel, and receive it in the Run loop of ChartChangeSync. This has a couple of flaws: - the event handlers have to wait for the Run loop to get around to doing the work (or time out and retry); - there's not much point in running more than one worker to process events, since they all get serialised through the ChartChangeSync anyway (not quite true -- deletes are done straight away because they don't need to consult the git repo) But it is good enough for now.
squaremo
force-pushed
the
rewrite/run-operator-events-via-chartsync
branch
from
077939a
to
20ce84a
Compare
Before I rewrote things to drive all the syncing through chartsync, syncing used `release.Exists(...)` to determine whether to Install or Upgrade a given release. That worked by coincidence since any error, including but not only a "not found" error, was treated as a negative result. However: it was a useful coincidence, because _just_ checking the error return value means a "not found" will exit early rather than go on to install the (possibly) missing release. So, ignore the error value, and choose whether to install or upgrade based on whether there was a release value returned or not, as `Exists` did.
Using a RW lock means that we only have to serialise access when we swap the clone for another. It's also a bit easier to grok than running requests through a channel, in my opinion.
squaremo
changed the title
|
@stefanprodan Thanks for finding that problem -- I reproduced it, and fixed it in subsequent commits. |
|
(merged using my "Get out of alpha free" card) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.