In controller-runtime v0.10.0, the client is updated to clean any stale
data in the target object when performing any operation. This results in
test failure for the code that constructs an object with both spec and
status, and creates the object and updates status it with the same
object. The fix is to set the status separately on the object before
updating it.
Refer: https://github.com/kubernetes-sigs/controller-runtime/pull/1640
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit rewrites the controller code so it uses the conditions
helpers from fluxcd/pkg/runtime/conditions.
It needs the fluxcd/source-controller/api module from the
reconcilers-dev branch, otherwise they will disagree on the methods in
fluxcd/pkg/apis/meta. Hence, variations on
go get github.com/fluxcd/source-controller{,api}@reconcilers-dev
until I got both ./go.mod and ./api/go.mod to use modules from the
branch.
Signed-off-by: Michael Bridgen <michael@weave.works>
libgit2's Push method will succeed even when ref updates are rejected,
meaning it can silently fail if you e.g., use branch protection in
GitHub.
To make these errors visible, a callback is supplied to Push, which
checks for a non-empty status (on the advice of
https://libgit2.org/libgit2/#HEAD/group/callback/git_push_update_reference_cb).
For whatever reason, gogit seems overly sensitive to hook errors (in a
way that `git` and libgit2 aren't), and reports "invalid pkg-len
found" when it sees a rejected ref message. This doesn't affect the
runtime code, since that uses libgit2 -- but it does affect the test
code, which initialises the git repo used in many tests, so more care
is needed to push only the main branch, so as not to trigger a
rejection.
Signed-off-by: Michael Bridgen <michael@weave.works>
This bumps the version of the image reflector types to v1beta1. This
doesn't technically make a difference for the minute, since the
Kubernetes API server would convert between versions, but is tidier.
Signed-off-by: Michael Bridgen <michael@weave.works>
This does the following:
- copies the type definitions from v1alpha2 to v1beta1
- changes the "stored" version to v1beta1
- gives the CRD a conversion strategy of None, meaning just rewrite the version
- switches the controller to use v1beta1
- moves the generated documentation to v1beta1
This effectively rebadges the v1alpha2 version of this part of the
image API to v1beta1. The v1alpha2 version is left in place; there are
no conversion issues, as with v1alpha1->v1alpha2. The CRD specifies
that converting between v1alpha2 and v1beta1 just means changing the
version (i.e., the schema and semantics are the same).
Signed-off-by: Michael Bridgen <michael@weave.works>
source-controller/pkg/git does shallow clones when using the go-git
implementation, and apparently this causes problems when fetching a
branch that has been merged at the origin:
https://github.com/fluxcd/image-automation-controller/issues/164
So far as I can tell, getting a shallow clone breaks the automation,
no matter whether go-git or libgit2 is used for operations after
cloning. So: just use libgit2 for cloning, which means non-shallow
clones; and, for fetch and push, since there's no functional
difference between the implementations for those.
Signed-off-by: Michael Bridgen <michael@weave.works>
As `golang.org/x/crypto/openpgp` has been deprecated (see
https://github.com/golang/go/issues/44226 for details), and this is the
most active/used fork.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This changes the API so that the checkout field has a ref, the same as
GItRepository. This means you can check out a branch or a tag or a
particular commit. Most of these won't work unless you supply a branch
to push to as well.
An addtional change is that you can leave out the checkout altogether,
and the ref will default to that given in the GitRepository, or its
default. In the latter case, again you will need to provide a push
branch.
Signed-off-by: Michael Bridgen <michael@weave.works>
This is a bit neater to read and write, and since I'm making breaking
changes anyway.
The name is now optional; an email is enough.
Signed-off-by: Michael Bridgen <michael@weave.works>
This finishes the v1alpha2 API, and rewrites everything needed so that
the controller supports it and the tests pass. For the most part, that
is just changing the location of fields. However, there's a few
notable extras:
- check that the `sourceRef` is a git repo (that's the default), and
that a `.spec.git` is supplied;
- change a test that blindly patched an update object, so that it
first gets the object it's patching. Previously, it succeeded
because it was OK to patch everything to empty strings, but that's
no longer the case since SourceReference.Kind is an enum.
Signed-off-by: Michael Bridgen <michael@weave.works>
There is a bug in go-git which leads to it reporting broken, absolute
symlinks as modified whether they are or not:
https://github.com/go-git/go-git/issues/253
To date, the controller checks whether the repo it has run an update
on is Clean, and as a consequence will run into the bug above if a
broken symlink is in the repo. The result is that it makes and pushes
an empty commit every interval.
To work around the problem, this commit adds a more careful check of
the repo status. Each file reported as modified is validated by
checking specifically that it's not a broken symlink: if `os.Lstat`
says it's a symlink and `os.Stat` reports the (target) file is
missing, it can be ignored. (Why not just ignore any missing file?
Because a missing file might indicate some other problem, so better to
let it fail).
For convenience, I have moved a few procedures around so they can be
used more readily by go tests.
Signed-off-by: Michael Bridgen <michael@weave.works>
This adds a test to check that should there be a further update to
make, another commit is pushed to the "push branch". In this case, the
image policy gets a new latest image.
The test fails at present because the controller is not watching image
policies (and will not run again on the long interval specified).
Signed-off-by: Michael Bridgen <michael@weave.works>
This adapts the controller so that it will honour the
`.spec.push.branch` field.
The behaviour _without_ that field is to check out the branch given in
`.spec.checkout.branch`, commit, and push to the origin.
With `.spec.push.branch` present, it will try to check out that
branch; if it doesn't exist, it'll create it, starting from
`.spec.checkout.branch`. Either way it'll commit to that branch and
push to the origin.
The effect is that all automation will happen on the "push" branch,
and (most likely) not be applied into the cluster until merged into
whichever branch is synced. When the push branch is deleted, it'll be
created anew; otherwise, commits will pile up there as more changes
are made.
Signed-off-by: Michael Bridgen <michael@weave.works>
This commit:
- passes a value including the update result to the commit message
template
- gives the template result a method for enumerating the
objects regardless of file
This means you can access the images updated either by file
(`.Files`), by object (`.Objects()`), or just as a list
(`.Images()`). The additional test case shows how to use these.
Signed-off-by: Michael Bridgen <michael@weave.works>
There is a core chuck of testing that is repeated for {SSH,HTTP} x
{go-git,libgit2}, which is done by repeating a func value in different
contexts. Instead of mutating variables in the func's closure, it's a
bit clearer (and shorter) to pass them to a higher-order func.
Signed-off-by: Michael Bridgen <michael@weave.works>
This commit rearranges update tests so that those that check that
updates are made can be run against a git server using SSH as well as
HTTP.
The local clone, used to provoke automated updates and to check
results, still uses HTTP. Those operations are not under test.
libgit2 wants to be asked for authentication when using SSH, and will
balk if it's not requested by the server. To avoid that, auth must be
switched on for the git test server.
This also switches auth on for HTTP, so it's necessary to use a git
URL that includes credentials for setting things up with a local
clone. I have also used that URL for the git-over-HTTP tests -- it's
arguable whether it's necessary to test that works, here.
Signed-off-by: Michael Bridgen <michael@weave.works>
It's convenient to be able to leave out the update strategy, since
there is only one possible value at present; and if there were
alternatives, the present choice would still be a reasonable
default. However, with the format as it is, this doesn't work with
OpenAPIv3 schema, so you have to supply a value, even though there are
no parameters:
```yaml
spec:
update:
setters: {}
```
A more self-explanatory format which _does_ work with defaulting is to
name the strategy rather than relying on the presence of a field:
```yaml
spec:
update:
strategy: Setters
```
The whole `update` field can be elided and left to default. This
doesn't preclude having other strategies later, even those with
parameters, e.g.,
```yaml
spec:
update:
strategy: Foo
fooParam: 5
```
This commit changes the API types and code that uses them, and the CRD
manifest, and adds a test that checks the defaulting actually works.
Signed-off-by: Michael Bridgen <michael@weave.works>
This follows up the stage-setting in prior commits, by respecting the
GitImplementation field given in the GitRepository object. NB it only
matters for cloning and pushing, so gogit is used in the "middle" to
record the commit in the local checkout.
Signed-off-by: Michael Bridgen <michael@weave.works>
This commit upgrades the `controller-runtime` dependency to `v0.7.0`,
including all changes required to make all wiring work again.
- Upgrade `runtime` to v0.6.2 to include `controller-runtime` changes.
- Logger has been removed from the reconciler, and is now retrieved
from the `context.Context` passed to the `Reconcile` method and
downwards functions.
- Logger configuration flags are now bound to the flag set using
`BindFlags` from `runtime/logger`, ensuring the same contract across
GitOps Toolkit controllers, and the `--log-json` flag has been
deprecated in favour of the `--log-encoding=json` default.
- The `ChangePredicate` from `runtime` has changed to a
`ReconcileRequestedPredicate`, and is now chained with the
`GenerationChangedPredicate` from `controller-runtime` using
`predicate.Or`.
- Signatures that made use of `runtime.Object` have changed to
`client.Object`, removing the requirement to e.g. call
`runtime.Object#Object`.
- The `leader-election-role` was changed, as leader election now works
via the `coordination/v1` API.
Other notable changes:
- Upgrade of `image-reflector-controller` API package to include
controller-runtime changes.
- Upgrade of `source-controller` API package to v0.6.1.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This adds fields to the ImageUpdateAutomation status for recording the
commit last pushed; handy to see when you are expecting a change.
It also adapts the "steady state" message of the ready condition to
mention the last commit, in case that's where people are looking.
Signed-off-by: Michael Bridgen <michael@weave.works>
After a change has been made, the ready condition should have a
message indicating that it pushed a commit.
Signed-off-by: Michael Bridgen <michael@weave.works>
Instead of having an arbitrary default branch, make the checkout
branch mandatory. This needed a little finessing in the tests, since
they did not cover using different branches (though did cover using a
non-standard branch).
Signed-off-by: Michael Bridgen <michael@weave.works>
The convention is now to simply exit, when a GOTK object is
suspended. Previously this would update the status to indicate that it
was unready; now it just leaves it in whichever state it was before.
This also applies to the reconcile request annotation; it will _not_
be marked as seen if the object is suspended. The effect of this is
any change to the object will be passed by the predicate and therefore
reach Reconcile, until the object is unsuspended. Since it will _also_
exit early until unsuspended, this is harmless except for some extra
log lines. (But changing that ordering might be worth considering in
the future.)
This change required a few changes to tests:
- to check that suspend makes the reconciliation exit without doing
anything, explicitly run `r.Reconcile(...)`.
- to avoid waiting for the reconciler's caching client to see
changes, use an uncached client.
- (fix a problem caused by comparing a time pointer with its alias)
Signed-off-by: Michael Bridgen <michael@weave.works>
The convention among GOTK controllers is to use a "reconcile request"
annotation to force a reconcilation, outside of spec or dependency
changes. This is used by e.g., the incoming webhooks handler. The
predicate `ChangePredicate`, already used by this controller, takes
this into account by allowing events that either caused the generation
to increment, _or_ changed the reconcile request annotation.
This commit adds a test that the automation will indeed run when the
annotation is set. This is a little delicate, because I have to rule
out _other_ reasons it might run. To do so, the test makes a change to
the git repo that will be overwritten by an automation run -- a commit
will not trigger a Reconcile call since it's entirely outside
Kubernetes.
Signed-off-by: Michael Bridgen <michael@weave.works>
This includes changes to:
* Bump the Kubernetes dependencies to v1.19.3 to align
with the other toolkit controllers.
* Update controller-runtime dependency to v0.6.3 to align
with the other toolkit controllers.
* Update the source-controller dependency to the most recent
v0.2.2 version to include the v1beta1 API.
* Add the `pkg/gittestserver` dependency for the Git tests,
as this package was removed in a newer source-controller version.
* Bump the Go version to v1.15.
Signed-off-by: Hidde Beydals <hello@hidde.co>
The initial implementation of image updates required a single image
policy, and updated every image field that used the image, in the git
repo. This mode has limited practical value, and rather than
elaborating on it, it would be better to concentrate on making the
more carefully thought-through "setters" mode.
This updates the package
github.com/squaremo/image-reflector-controller to a revision that also
uses controller-runtime 0.6.2 (and client-go etc., 0.18).
The ImagePolicy CRD has also changed, so I've also uodated the tests
involving that type.
This moves the specificiation for the checkout of the git repo to a
struct `GitCheckoutSpec`; and, renames the field that's a reference to
a GitRepository so it has the suffix `Ref` per gitops-toolkit
convention.
When I set my default branch for `git init` in ~/.gitconfig, tests
start failing. This is because go-git assumes a default of `master`
when e.g., pulling commits, but exec'ing git (as the test server does,
via gitkit) will get you a different branch.
The answer is to be explicit about the branch created in tests, and in
all operations like cloning. The natural end point is that the
automation object specifies a branch to use (or an empty value,
meaning let it default).
Sunny
Michael Bridgen
Hidde Beydals
Stefan Prodan
LWJ
Michael Bridgen