This to facilitate improvements on the notification-controller side,
where annotations prefixed with the FQDN of the Group of the Involved
Object will be transformed into "fields".
Signed-off-by: Hidde Beydals <hello@hidde.co>
Prioritize StorageOperationFailedCondition over other artifact outdated
and unavailable conditions so that when artifact is failing due to
storage operation, it's visble in the ready status condition, making the
reason for not ready more accurate.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
libgit2 network operations are blocking and do not provide timeout nor context capabilities,
leading for several reports by users of the controllers hanging indefinitely.
By using managed transport, golang primitives such as http.Transport and net.Dial can be used
to ensure timeouts are enforced.
Co-Authored-by: Sunny <darkowlzz@protonmail.com>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Introduce new condition StorageOperationFailedCondition for all the
failures related to the storage. It is a negative polarity condition and
is considered in computing summary of reconciliation.
Also, introduce more granular event reasons related to
StorageOperationFailedCondition for precise reasoning behind failures.
These replace the vague StorageOperationFailedReason.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
- Update summarize helper to have patch field owner.
- Updated the controllers to set the patch field owner.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
- Update v1beta2 API descriptions and reconciling messages to be
consistent.
- Replace 'download' with 'fetch'. Since the status condition for
download failure is called FetchFailed, using the term 'fetch' makes
the messaging more consistent.
- Replace `BucketOperationSucceed` with `BucketOperationSucceeded` and
generate api docs.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
summarizeAndPatch() was used by all the reconcilers with their own
object type. This creates a generic SummarizeAndPatch helper that takes
a conditions.Setter object and performs the same operations. All the
reconcilers are updated to use SummarizeAndPatch(). The process of
summarize and patch can be configured using the HelperOptions.
Introduce ResultProcessor to allow injecting middlewares in the
SummarizeAndPatch process.
Introduce RuntimeResultBuilder to allow defining how the reconciliation
result is computed for specific reconciler. This enabled different
reconcilers to have different meanings of the reconciliation results.
Introduce Conditions in summary package to store all the status
conditions related information of a reconciler. This is passed to
SummarizeAndPatch() to be used for summary and patch calculation.
Remove all the redundant summarizeAndPatch() tests per reconciler.
Add package internal/object containing helpers for interacting with
runtime.Object needed by the generic SummarizeAndPatch().
Add tests for ComputeReconcileResult().
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Update Storage.RemoveAll() and Storage.RemoveAllButCurrent() to return
the details about the deleted items. This helps emit useful information
about garbage collection in the controllers and ignore no-op garbage
collections.
RemoveAll() returns the path of the deleted directory if any.
RemoveAllButCurrent() returns a slice of path of all the deleted items
from a resource's artifact dir.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Consolidate BuildRuntimeResult() into summarizeAndPatch() to simplify
where the results are computed, summarized and patched.
Move the event recording and logging of context specific errors into
RecordContextualError() and call it in summarizeAndPatch().
Introduce Waiting error for wait and requeue scenarios. Update
ComputeReconcileResult() and RecordContextualError() to consider Waiting
error.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
- Ensure all logged messages start with a lowercase.
- Make some pushed (and logged) events of type `EventTypeTrace` to
prevent them from being sinked to the external event recorder, to
prevent spam.
- Only log if artifact is up-to-date with upstream (instead of pushing
an event).
Signed-off-by: Hidde Beydals <hello@hidde.co>
The artifacts built in reconcileInclude() should be persisted by writing
it to includes. reconcileArtifact() later adds includes to the git repo
object status and persists it.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
- Remove ArtifactUnavailable condition and use Reconciling condition to
convey the same.
- Make Reconciling condition affect the ready condition.
- Introduce summarizeAndPatch() to calculate the final status conditions
and patch them.
- Introduce reconcile() to iterate through the sub-reconcilers and
execute them.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
While deleting, patching an object with new status results in "not
found" error because the object is already deleted. The patching
operation first patches the status conditions, the rest of the object
and, at the very end, the rest of the status. When an object is
deleted, the garbage collection results in the artifact in the status
to be updated, resulting in a diff that is attempted to be patched
when the deferred patch runs.
Since the status patching runs at the very end, the object gets deleted
before it can be patched.
Ignore "not found" error while patching when the delete timestamp is
set.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
SourceVerifiedCondition is a normal condition, remove it from negative
polarity conditions. Add SourceVerifiedCondition in patch option
WithOwnedConditions.
Also, Update the signature of reconcileInclude() to remove include
being passed and overwritten in the first line. Include is available
as part of the passed source object.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This wraps the errors which are returned instead of logging them, as
the returned error is logged at the end of the reconcile run.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit consolidates the `DownloadFailed` and `CheckoutFailed`
Condition types into a new more generic `FetchFailed` type to simplify
the API and observations by consumers.
Signed-off-by: Hidde Beydals <hello@hidde.co>
- Mention the current revision in the up-to-date log message.
- Ensure any error that is "swallowed" (not returned) is logged to
ensure they are visible within the logs, and not just by inspecting
the object.
Signed-off-by: Hidde Beydals <hello@hidde.co>
The problem with `GetInterval()` was that the returned type was of
`metav1.Duration`, while almost anywhere it was used, a type of
`time.Duration` was requested. The result of this was that we had to
call `GetInterval().Duration` all the time, which would become a bit
cumbersome after awhile.
To prevent this, we introduce a new `GetRequeueAfter() time.Duration`
method, which both results the right type, and bears a name that is
easier to remember where the value is used most; while setting the
`Result.RequeueAfter` during reconcile operations.
The introduced of this method deprecates `GetInterval()`, which should
be removed in a future MINOR release.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Fixes error returned from target path validation check and adds more
test cases for TestGitRepositoryReconciler_reconcileArtifact.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
NOTE: This should be amended with the previous commit which has
commented out tests.
Update reconcileSource() to work with the test case where no secret is
set. A minimal auth options is created and used for git checkout.
Update TestGitRepositoryReconciler_verifyCommitSignature() to use the
new git.Commit type.
Update TestGitRepositoryReconciler_reconcileSource_checkoutStrategy to
add skipForImplementation for branch commit test case.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit rewrites the `GitRepositoryReconciler` to new standards,
while implementing the newly introduced Condition types, and trying
to adhere better to Kubernetes API conventions.
More specifically it introduces:
- Implementation of more explicit Condition types to highlight
abnormalities.
- Extensive usage of the `conditions` subpackage from `runtime`.
- Better and more conflict-resilient (status)patching of reconciled
objects using the `patch` subpackage from runtime.
- Proper implementation of kstatus' `Reconciling` and `Stalled`
conditions.
- First (integration) tests that solely rely on `testenv` and do not
use Ginkgo.
There are a couple of TODOs marked in-code, these are suggestions for
the future and should be non-blocking.
In addition to the TODOs, more complex and/or edge-case test scenarios
may be added as well.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit ensures all API objects implement the interfaces used by
the runtime package to work with conditions, etc., and prepares the
test suite to work with the `pkg/runtime/testenv` wrapper.
Changes are made in a backwards compatible way (that being: the
existing code can still be build and works as expected), but without
proper dependency boundaries. The result of this is that the API
package temporary depends on the runtime package, which is resolved
when all reconcilers have been refactored and the API package does
no longer contain condition modifying functions.
Signed-off-by: Hidde Beydals <hello@hidde.co>
For signed commit verification, this code errors out on line 303:
✗ GitRepository reconciliation failed: ''PGP public keys secret error: expected pointer, but got nil
Pointer was not initialized with a concrete instance of the Secret struct
Signed-off-by: Kingdon Barrett <yebyen@gmail.com>
This commit changes the `gogit` behavior for commit checkouts,
now allowing one to reference to just a commit while omitting any
branch reference. Doing this creates an Artifact with a
`HEAD/<commit>` revision.
If both a `branch` and `commit` are defined, the commit is expected
to exist within the branch. This results in a more efficient clone
of just the target branch, and also makes this change backwards
compatible.
Fixes#407Fixes#315
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit refactors the previous `Commit` interface into a
standardised `Commit` struct. This object contains sufficient
information for referencing, observating and (PGP) verification.
- `libgit2` commit checkout does now return `HEAD/<SHA1>` as
the branch is not taken into account.
- `git2go` objects are now properly `Free`d everywhere
- `Verify` logic is tested.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit moves the previous `AuthStrategy` wiring to a more generic
`AuthOptions`, breaking free from implementation specific details in
the `git` package.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Unlike Bucket resources which are matched by key as presented by S3,
ignore rules for GitRepository objects do have a domain: the temporary
directory of the Git repository.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit makes the filtering applied during the archiving
configurable by introducing an optional `ArchiveFileFilter`
callback argument and a `SourceIgnoreFilter` implementation.
`SourceIgnoreFilter` filters out files matching
sourceignore.VCSPatterns and any of the provided patterns.
If an empty gitignore.Pattern slice is given, the matcher is set to
sourceignore.NewDefaultMatcher.
The `GitRepository` now loads the ignore patterns before archiving
the repository contents by calling `sourceignore.LoadIgnorePatterns`
and other helpers. The loading behavior is **breaking** as
`.sourceignore` files in the (subdirectories of the) repository are
now still taken into account if `spec.ignore` for a resource is
defined, overwriting is still possible by creating an overwriting
rule in the `spec.ignore` of the resource.
This change also makes it possible for the `BucketReconciler` to not
configure a callback at all and prevent looking for ignore
matches twice. To finalize the bucket refactor, a change to the
reconciler has been made to look for a `.sourceignore` file in
the root of the bucket to provide an additional way of configuring
(global) exclusions.
Signed-off-by: Hidde Beydals <hello@hidde.co>
To include a bug fix to the `ReconcilateAtChangedPredicate`
and renaming to `ReconcileRequestedPredicate`.
Signed-off-by: Hidde Beydals <hello@hidde.co>
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.0 to include `controller-runtime` changes.
- Loggers have been removed from the reconcilers and are 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
`ReconcilateAtChangedPredicate`, 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 `client.MatchingField` function was deprecated, and has been
replaced with `client.MatchingFields{}`.
- The `leader-election-role` was changed, as leader election now works
via the `coordination/v1` API.
Other notable changes:
- `util.ObjectKey` was added to easily construct a `client.ObjectKey` /
`types.NamespacedName` from a `metav1.Object`.
- The `SourceIndexKey` constant has been split out into
`{GitRepository,HelmRepository,Bucket}IndexKey` constants.
Signed-off-by: Hidde Beydals <hello@hidde.co>
We had a hardcoded assumption that the SSH user for a Git repository is
always "git". This is however not true in all scenarios, for example
when one is making use of Gerrit for team code collaboration, as users
there have their own username for (SSH) Git operations.
This commit changes the logic of the auth strategy helpers to:
1. Select the auth strategy based on the protocol of the parsed URL,
instead of a simple rely on a correct prefix.
2. Use the user information from the parsed URL to configure the user
for the public key authentication strategy, with a fallback to `git`
if none is defined.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Use SetResourceCondition as a generic method to set conditions for CRs,
implmeneting the ObjectWithStatusConditions interface used as input
type.
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
Updates to use metav1.Condition type and removes references for
deprecated corev1.Condition* constants and uses the new k8s api/meta
helpers in place of the old pkg/apis/meta types.
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
This commit ensures that resources will only return early if they are
already in a `Ready==True` state. If not, but the status object somehow
still reports that it has an artifact, the reconciliation will continue
to ensure and/or guarantee state, and to prevent a deadlock from
happening.
During high custom resource count / low interval tests, I was greated
with a `cannot patch resource "events"` message. This happened due to
event compaction, where it will perform a patch instead of a create.
By giving the role the permission to do so this should no longer pose
a problem.
When a delete of a resource is requested a `deletionTimestamp` is set
on the resource by the requester, this also results in a generation
change of the resource.
If the resource is under reconciliation while this timestamp is set, and
had not produced an artifact earlier on, this becomes a problem as the
artifact metadata is used to determine what should be garbage collected
on a deletion, resulting in stray files for resources that are no longer
present.
To resolve this for now, we always create a new artifact object for the
resource when `all==true` on the GC method call, and no longer rely on
the presence of the artifact object on the resource itself.
This includes a change to how the revision for HelmRepository sources is
recorded, as this will now equal to the generated timestamp from the index
in RFC3339Nano format.
Due to required domain changes for the helm-controller so that it
can co-exist in a cluster with the Helm Operator, other Toolkit
components are moving to a *.toolklit.fluxcd.io domain too.
- return reconciliation error so that controller runtime metrics record failures
- change structure logging labels to match the controller runtime format
- log the reconciliation duration for all kinds
- normalise log messages and labels across all controllers
This commit changes the file excludes for tarballs generated for
Git repository artifacts from a fixed set of strings to include
exclusion files files. It currently takes `.sourceignore` and
in the root of the given directory into account.
In addition to this the Git VCS related files that are ignored have
been extended to not only include the .git/ directory, but also the
.gitignore, .gitmodules and .gitattributes files. Mimicking part of
the --exclude-vcs flag not available on all tar versions.
This commit adds a timeout field to the GitRepositorySpec to be used
during the git clone operation when reconciling the resource.
When no interval is defined the default timeout returned by the getter
is 20 seconds.
The timeout can not be added yet to the Helm related sources as it
is currently not possible to inject anything custom into the HTTP
client from the Helm HTTP getter except for the authentication
options built in. A submit has been submitted to make this possible
and is waiting for review.
This commit includes some context changes to the other reconcilers
to tidy them up and make them depend on a single background context.
It also includes some added docblocks that crossed my path.
Hidde Beydals
Sunny
Paulo Gomes
Stefan Prodan
Aurel Canciu
Kingdon Barrett
Philip Laine
Somtochi Onyekwere
Philip Laine
Michael Bridgen
Erik Hollensbe
Hidde Beydals