Flux source integration #324
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
lblackstone
reviewed
Sep 20, 2022
Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
This adds FluxSource as an alternative to the (inline) GitSource. Signed-off-by: Michael Bridgen <[email protected]>
squaremo
force-pushed
the
flux-source-integration
branch
from
b8e5b55 to
f72adf4
Compare
|
Further test cases:
EDIT: I am going to bump "When the source has a new version, the stack is rerun". This is true, as a consequence of stacks being rescheduled, so it will work the same as a git source. It would be nice to keep a watch on sources and be more responsive than git sources (which poll the git repo for new commits on each run), but it can be done in a follow up PR, I think. |
squaremo
force-pushed
the
flux-source-integration
branch
2 times, most recently
from
d5ae2f3 to
3681dae
Compare
|
(I will square away the design doc -- aside from anything else, the fields in the proposed API change need to be renamed) |
lblackstone
reviewed
Oct 10, 2022
This adds handling of Flux sources into the controller, while trying to disturb the present behaviour as little as possible. Broadly this means branching in a couple of places to do source-specific things: in particular, - setting up the automation API workspace (SetupWorkdirFrom*) is mostly different up to the point you have fetched the source code, which in the GitSource case is done _by_ setting up the workspace; so, all the things you have to do _after_ you have an auto.Workspace go into another helper, which is called from each Setup func. The func signatures are brought into line so the return values can have a uniform treatment. - requeue handling is quite idiosyncratic (e.g,. why does ContinueResyncOnCommitMatch only matter if you are tracking a branch?). I have left the current behaviour for git sources intact, since it may be relied upon in deployments, and branched to analogous logic for Flux sources. This means some near-duplicated code, for the sake of playing it safe. Signed-off-by: Michael Bridgen <[email protected]>
A missing source is something that could just "come right"; record it in .status.lastUpdate as a failure, and in .status.conditions as "in progress (retrying)". Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
Many of the tests wait for a particular value of `.status.lastUpdate.State`, then check the values of conditions are as expected. However, between these two things, they fetch the object from the API server again (`refetch`), since the waitForStack* uses a temporary variable rather than mutating the stack given to it. That that admits the possibility that the status might have been updated again, by the controller reconciling it again. Instead, let waitForStack* write to the object it's given, and don't `refetch` -- then, the status observed will be the status at the point the wait succeeded. Signed-off-by: Michael Bridgen <[email protected]>
This standardises how the .status.artifact.X fields are accessed, so that (for the sake of a few extra lines) there's less repeated code, and errors are consistent. Signed-off-by: Michael Bridgen <[email protected]>
Previously, an if-then-else checked for the presence of source specifications (GitSource and FluxSource), and this made it possible to supply a stack object with both kinds of source (though only the first checked -- GitSource -- would be used). This commit makes that bit of code enforce the mutual exclusion of sources. Signed-off-by: Michael Bridgen <[email protected]>
squaremo
force-pushed
the
flux-source-integration
branch
from
531fe1c to
fc4f6fc
Compare
lblackstone
approved these changes
Oct 11, 2022
| // Check which kind of source we have. | ||
| gitSource, fluxSource := stack.GitSource, stack.FluxSource | ||
| switch { | ||
| case gitSource != nil && fluxSource == nil: |
There was a problem hiding this comment.
This logic is fine, but an alternative would be validating the mutually-exclusive condition ahead of the switch, and then keeping the simpler non-nil selection logic that you had previously. It probably doesn't matter for this PR, but it might be a little cleaner once the inline YAML case is added as well.
There was a problem hiding this comment.
I am ever hopeful that a switch will help make the code look more like a pattern match. One day it'll be true.
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addresses #158.