If implemented this merges
TestHelmChartReconciler_reconcileFromHelmRepository and
TestHelmChartReconciler_Reconcile
Signed-off-by: Soule BA <soule@weave.works>
1. moved chartRepo.Unload() from reconcileSource() to the defer func in reconcileArtifact to allow caching index in memory
2. added step to init memory cache in reconcileArtifact()
3. added step to save helmrepo index into memory cache in reconcileArtifact()
Signed-off-by: York Chen <ychen@d2iq.com>
Decommission libgit2 unmanaged transport and remove the related feature
gate, making managed transport the default.
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
If implemented, the helmrepository type will be used to decide whether a
reconciliation can continue in the absence of source artifact, instead
of url.
Signed-off-by: Soule BA <soule@weave.works>
If implemented, the source controller will be able to resolve charts
dependencies from OCI repositories.
The remote builder has been refactored as part of this work.
Signed-off-by: Soule BA <soule@weave.works>
- Update the comments around artifact retention fields in Storage.
- Update the comments around reconcileStorage regarding artifact
retention and garbage collection.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
GitRepository introduced correlation ID to improve
transport level logging. This change aligns the other
reconcilers to the same approach.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Debugging connection issues can be extremely difficult, even more so at scale or when
concurrent connections are required to trigger specific issues.
Changes:
- Add a correlation identifier for each reconciliation, which allows for greater traceability when
going through all the reconciliation operations - including at transport level.
- Add transportType to segregate HTTP and SSH transport logging.
- SSH operations are now enriched with addr containing server address, and HTTP url.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Refactors libgit2 checkout tests to test managed and unmanaged
transport by making sure the tests requiring unmanaged transport are run
before, any tests that require managed transport (since disabling
managed transport isn't possible). This is done via arranging the tests
carefully in alphabetically sorted names, i.e. the tests with unmanaged
transport go in `checkout_test.go`, which forces golang to run the tests
in that file before any other tests.
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
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>
Soule BA
York Chen
Sanskar Jaiswal
Paulo Gomes
Sunny
Stefan Prodan
Max Jonas Werner
Max Jonas Werner
Eng Zer Jun
Alexander Block