Remove stale condition from HelmRepo during garbage collection when a
type switch to OCI HelmRepo occurs. This ensures the OCI HelmRepo does
not have any conditions from the previous type.
Co-authored-by: Soule BA <soule@weave.works>
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Refactor the OCI HelmRepo reconciler to remove extra custom status
conditions and manage Ready, Reconciling and Stalled conditions within
the reconciler, in-line.
The internal/reconcile/summarize package uses the patch helper
conditions summary before patching which results in overwriting the
Ready condition with Reconciling condition as it's a negative polarity
condition.
For OCI HelmRepository, since it's a simple reconciler, it'd be better
to not introduce extra conditions and utilize the three base conditions
to represent the status. In order to have the same consistent status
conditions, a new summarization is written within the reconciler based
on the context. It takes into consideration a lot of the details from
the internal/reconcile/summarize package and handles certain scenarios
in context specific ways. All the result and error abstractions are
removed since they are only needed when using internal/reconcile
package.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Some scenarios could lead a goroutine to be running indefinetely within managed ssh.
Previously between the two git operations, the reconciliation
could take twice the timeout set for the Flux object.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Injects transport and auth options at the transport level directly to
bypass the inbuilt credentials callback because of it's several
shortcomings. Moves some of the pre-existing logic from the reconciler
to the checkout implementation.
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
gitCheckout() should return typed errors only. This helps prevent
error type assertions by the caller to determine how to handle the
error. gitCheckout() also sets the appropriate conditions associated
with the error, if any.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Some test cases rely on checksum to match in order to pass.
Those checksums were calculated based on file headers which
contain their file modes.
In Ubuntu, the umask is set to 002 by default, resulting in
the tests files having different permissions then when the
same files are cloned on another Linux machine with umask
set to 022.
This change ensures that the files are always set (to 0644 and
the directories to 0755) before running the aforementioned tests.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
In TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter, when
the type of HelmRepo is updated and immediately checked for the object
to be ready, if the check happens before the client cache is updated, it
results in observing the object to be ready in the previous generation.
This results in status check failure:
```
[Check-FAIL]: [Ready condition must be False when the ObservedGeneration is less than the object Generation, Ready condition must be False when any of the status condition's ObservedGeneration is less than the object Generation: [Ready ArtifactInStorage]]
```
Explicitly look for the object with the next generation to prevent such
failure.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This new package holds all Helm OCI registry-specific code now so we
have a single location to look for such code which makes it easier to
find yourself around.
Signed-off-by: Max Jonas Werner <mail@makk.es>
When setup of one of the two controller reconciling HelmRepositories
fails, it's now possible to judge from the log which setup call failed
by regarding the "type" log field.
Signed-off-by: Max Jonas Werner <mail@makk.es>
`loginOptionFromSecret` now derives username/password from a docker
config stored in Secrets of type "kubernetes.io/dockerconfigjson".
Signed-off-by: Max Jonas Werner <mail@makk.es>
When the Secret referenced in an OCI HelmRepository doesn't contain a
username and password, the controller doesn't panic, anymore.
Signed-off-by: Max Jonas Werner <mail@makk.es>
Introduce contentConfigChecksum in the GitRepository.Status to track the
configurations that affect the content of the artifact. It is used to
detect a change in the configuration that requires rebuilding the whole
artifact. This helps skip the reconciliation early when we find out that
the remote repository has not changed.
Moves fetching the included repositories in reconcileSource() to collect
enough information in reconcileSource() to be able to decide if the full
reconciliation can be skipped. This results in reconcileInclude() to
just copy artifact to the source build directory.
Introduce a gitCheckout() method to perform construction of all the git
checkout options and perform the checkout operation. This helps to
easily perform checkout multiple times when we need it in
reconcileSource(). When we check with the remote repository if there's
an update, and find out that there's no update, we check if any other
configurations that affect the source content has changed, like
includes, ignore rules, etc. If there's a change, we need to perform a
full checkout of the remote repository in order to fetch the complete
source. The git checkout no-op optimization is enabled in this method
based on the presence of an artifact in the storage.
The failure notification handler is modifed to handle the recovery of a
no-op reconcile failure and create a notification message accordingly
with the partial commit.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Introduce a new field in the GitRepositoryReconciler to set the enabled
features. This makes it test friendly compared to using global flags for
setting and checking flags in the tests.
Enable default feature gates in all the GitRepo reconciler tests.
Add test cases for reconcileSource() to test the behavior of optimized
git clone when the Repo is ready and not ready. This ensures that the
full reconciliation is not skipped when GitRepo is not ready.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
For gradual migration to Generic error, update only the GitRepo
reconciler to use Generic error.
Replace the Waiting error for git no change scenario with a Generic
error with proper no-op, early return, error configurations. This
ensures that the no-op only results in log and K8s native events at
normal level.
Fixes a reconciliation issue when recovering from a failure state (with
previous success state and artifact in the storage) and optimized git
clone feature is on, which results in failure to persist as the git
optimization prevented full reconciliation due to already existing
artifact and removal of failure negative conditions on the object
status. In order to allow failure recovery, the git clone optimizations
are now only applied when the object is already in a ready state.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
* Add OCI Helm support
* users will be able to declare OCI HelmRepository by using the `.spec.type` field of the HelmRepository API. Contrary to the HTTP/S HelmRepository no index.yaml is reconciled from source, instead a simple url and credentials validation is performed.
* For backwards-compatibility, an empty `.spec.type` field leads to the HelmRepository being treated as a plain old HTTP Helm repository.
* users will be able to declare the new OCI HelmRepository type as source using the .Spec.SourceRef field of the HelmChart API. This will result in reconciling a chart from an OCI repository.
* Add registryTestServer in the test suite and OCI HelmRepository test case
* Add a new OCI chart repository type that manage tags and charts from an OCI registry.
* Adapat RemoteBuilder to accept both repository types
* discard output from OCI registry client; The client has no way to set a verbosity level and spamming the controller logs with "Login succeeded" every time the object is reconciled doesn't help much.
Signed-off-by: Soule BA <soule@weave.works>
Signed-off-by: Max Jonas Werner <mail@makk.es>
Co-authored-by: Soule BA <soule@weave.works>
Hashed known_hosts was previously only supported when using
go-git. Now both Git implementations benefit from this
features, and the code coverage across them can ensure no
future regression.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
OptimizedGitClones decreases resource utilization for GitRepository
reconciliations. It supports both go-git and libgit2 implementations
when cloning repositories using branches or tags.
This is an opt-out feature, which can be disabled by starting the
controller with the argument '--feature-gates=OptimizedGitClones=false'.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
No-op reconciliations are very inefficient, as they carry out
a full clone operation of the target repository even when
no changes have taken place.
This change will execute a remote-ls operation, and cancel
the clone operation if the remote tip commit is still the same
as the one observed on the last reconcilation. In such cases,
an git.NoChangesError is returned.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `os.MkdirTemp`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This change prevents Reconciling and ArtifactOutdated conditions to be
set on HelmRepo when the checksum of a cached repo index changes.
Adds some tests to ensure that when the repo index is cached, the
revision and checksum of the returned artifact are the same as on the
existing object status.
Also adds checks for the returned artifact and chartRepo from
reconcileSource, to ensure that chartRepo is populated and the checksum
of a new potential artifact is always empty, as it's populated when the
artifact is written in the storage.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Avoid validating (and thus loading) indexes if the checksum already exists in storage.
In other words, if the YAML is identical to the Artifact in storage, the reconciliation should
be a no-op, and therefore can short-circuit long/heavy operations.
Co-authored-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
I assume using "interval" for timeouts was an accident and "timeout" was
actually meant to be used. This also fixes flakiness of tests.
Signed-off-by: Alexander Block <ablock84@gmail.com>
This fixes the immediate issue of the nil pointer dereference but we
still haven't isolated the actual cause of the size being nil to begin
with. This is ongoing work and as soon as we have boiled that down to
the simplest case we will provide a regression test for that case.
closes#680
Signed-off-by: Max Jonas Werner <mail@makk.es>
Co-authored-by: Hidde Beydals <hiddeco@users.noreply.github.com>
If implemented this will:
- enable the helmCharts dependency manager to use the helm in memry
cache to retrieve reconciled HelmRepositories indexes.
- record cache events.
Signed-off-by: Soule BA <soule@weave.works>
Azure SDK dependencies cannot be updated, as this requires us to move to
Go 1.18.
- cloud.google.com/go/storage to v1.22.0
- github.com/ProtonMail/go-crypto to v0.0.0-20220407094043-a94812496cf5
- github.com/darkowlzz/controller-check to v0.0.0-20220325122359-11f5827b7981
- github.com/elazarl/goproxy to v0.0.0-20220403042543-a53172b9392e
- github.com/fluxcd/pkg/gittestserver to v0.5.2
- github.com/go-logr/logr to v1.2.3
- github.com/minio/minio-go/v7 to v7.0.24
- github.com/onsi/gomega to v1.19.0
- golang.org/x/crypto to v0.0.0-20220411220226-7b82a4e95df4
- google.golang.org/api to v0.74.0
Signed-off-by: Hidde Beydals <hello@hidde.co>
As suggested by @pjbgf
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
Signed-off-by: Peter Gundel <mail@petergundel.de>
This better represent permissions as Linux handles such information in
octal format, meaning that the left-most 0 has an important meaning
and is not to be ignored as normally integers would.
See https://github.com/fluxcd/source-controller/issues/603
Signed-off-by: Peter Gundel <mail@petergundel.de>
Add two new flags to enable users to configure exponential
back-off for Flux objects. The default values are now
set to 750ms for minimum retry time, and 15min for max.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
This includes some rewiring of tests, and slight changes in how we work
with the local chart reference. `Path` is expected to be relative to
`WorkDir`, and both fields are now mandatory.
Signed-off-by: Hidde Beydals <hello@hidde.co>
notify() is used to emit events for new artifact and failure recovery
scenarios. It's implemented in all the reconcilers.
Previously, when there used to be a failure due to any reason, on a
subsequent successful reconciliation, no notification was sent to
indicate that the failure has been resolved.
With notify(), the old version of the object is compared with the new
version of the object to determine if all, if any, of the failures have
been resolved and a notification is sent. The notification message is
the same that's sent in usual successful source reconciliation message
about stored artifact.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
We try to avoid affecting the source reconciliation when there's a
garbage collection related failure.
The event logging was resulting in events and notifications related to
GC failure when the artifact directory isn't created in the first
reconciliation of an object.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Introduce two new flags to configure the ttl of an artifact and the max
no. of files to retain for an artifact. Modify the gc process to
consider the options and use timeouts to prevent the controller from
hanging.
This helps in situations when the SC has already garbage collected the
current artifact but the advertised artifact url is still the same,
which leads to the server returning a 404.
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
If implemented, will provide users with a way to cache index files.
This addresses issues where the index file is loaded and unmarshalled in
concurrent reconciliation resulting in a heavy memory footprint.
The caching strategy used is cache aside, and the cache is a k/v store
with expiration.
The cache number of entries and ttl for entries are configurable.
The cache is optional and is disabled by default
Signed-off-by: Soule BA <soule@weave.works>
Soule BA
Stefan Prodan
Sunny
Paulo Gomes
Sanskar Jaiswal
Max Jonas Werner
Max Jonas Werner
Eng Zer Jun
Alexander Block
Hidde Beydals
Peter Gundel
Peter Gundel