Commit Graph

271 Commits

Author SHA1 Message Date
Sunny ba7cbd31f1 bucket: Add more reconciler design improvements
- 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>
2022-02-23 12:35:30 +01:00
Sunny 848534a8f1 bucket: Ignore patch error not found on delete
Ignore "not found" error while patching when the delete timestamp is
set.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny f472cadab4 Add bucket controller tests for reconcileGCPSource
- Introduce mock GCP Server to test the gcp bucket client against mocked
  gcp server results.
- Add tests for reconcileGCPSource().
- Patch GCPClient.BucketExists() to return no error when the bucket
  doesn't exists. This keeps the GCP client compatible with the minio
  client.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny 25779241ec Add more reconcileMinioSource test cases
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny c79a55baa8 BucketReconciler: Add reconcileArtifact tests
Add `BucketReconciler.reconcileArtifact` tests based on
`GitRepositoryReconciler.reconcileArtifact` test cases.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Hidde Beydals 84301e6370 Consolidate condition types into `FetchFailed`
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>
2022-02-23 12:35:30 +01:00
Hidde Beydals 89ba8374b6 Rewrite `BucketReconciler` to new standards
This commit rewrites the `BucketReconciler` 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.
- Refactor of reconciler logic, including more efficient detection of
  changes to bucket objects by making use of the etag data available,
  and downloading of object files in parallel with a limited number of
  workers (4).
- 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>
2022-02-23 12:35:30 +01:00
Sunny 52f4a2a800 bucket: Replace GetInterval() with GetRequeueAfter()
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny c4fa79c85e gitrepo: Fix reconcileInclude() includes
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>
2022-02-23 12:35:30 +01:00
Sunny 2acb721c35 gitrepo: Use internal/util for creating temp dir
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny 5767291b58 gitrepo: Add more reconciler design improvements
- 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>
2022-02-23 12:35:30 +01:00
Sunny d9a947c909 gitrepo: Ignore patch error not found on delete
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>
2022-02-23 12:35:30 +01:00
Sunny 25ae83ad7d gitrepo: Fix SourceVerifiedCondition condition type
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>
2022-02-23 12:35:30 +01:00
Hidde Beydals 88dc2e6ed6 chore: ensure Git server dir is removed after test
Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-23 12:35:30 +01:00
Hidde Beydals dbaf21b974 Wrap err with context instead of logging twice
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>
2022-02-23 12:35:30 +01:00
Hidde Beydals a437ed69bd Consolidate condition types into `FetchFailed`
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>
2022-02-23 12:35:30 +01:00
Sunny 3d5698856a gitrepo: test reconcileArtifact condtns & symlink
Adds test cases for reconcileArtifact to check if old status
conditions are removed after new artifact is created.
Adds a test case to verify that the latest artifact symlink points to
the created artifact.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny 59b3e5da5d gitrepo: Add tests for old conditions update
This tests the status conditions update in the gitrepository reconciler.
Given a mix of old status conditions, on a successful reconciliation,
the status condition is set to Ready=True.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny 7e71185594 gitrepo: reconcileInclude test assertion fixes
Use the created artifact server test storage in reconcileInclude
test's GitRepositoryReconciler and cleanup the created storage.

Fix the test assertions to check the copied artifact directories in
the correct path. Also, update the tests to expect artifacts in the
include `toPath` to exist.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Hidde Beydals c2e6875284 Tweak logged messages
- 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>
2022-02-23 12:35:30 +01:00
Hidde Beydals 1e326e8f1c source: `GetRequeueAfter` in place of `GetInterval`
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>
2022-02-23 12:35:30 +01:00
Hidde Beydals 67bd2cb81c Replace %q in messages with '%s'
Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-23 12:35:30 +01:00
Sunny 18180776e7 controllers: Add more tests for reconcileArtifact
Fixes error returned from target path validation check and adds more
test cases for TestGitRepositoryReconciler_reconcileArtifact.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-23 12:35:30 +01:00
Sunny b814070bc2 Fixes to gitrepo reconciler tests
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>
2022-02-23 12:35:30 +01:00
Hidde Beydals 31d2e6d65c Rewrite `GitRepositoryReconciler` to new standards
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>
2022-02-23 12:35:30 +01:00
Sunny dc9d8b7f66 Add gomega matcher for artifact
Signed-off-by: Sunny <darkowlzz@protonmail.com>

Co-authored-by: Hidde Beydals <hello@hidde.co>
2022-02-23 12:35:30 +01:00
Hidde Beydals e190059cc7 Introduce `artifactSet` to replace `hasArtifactUpdated`
NOTE: Remove `hasArtifactUpdated` in the future once it's no longer
used.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-23 12:35:30 +01:00
Hidde Beydals 349739b7e4 Implement new runtime interfaces, prepare testenv
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>
2022-02-23 12:35:30 +01:00
Hidde Beydals 7c3c14997e Switch to v1beta2 API package
Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-23 12:35:25 +01:00
Sunny 649d33ca37
pkg/git: Include commit message and URL in error
go-git: Include the commit message in the returned commit object.
libgit2: Set the URL in the checkout error.

Add new method Commit.ShortMessage() for returning short commit
message.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-14 21:34:27 +05:30
Paulo Gomes cd6d33c101
Increase gingko timeout to 60s
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
2022-02-08 17:49:51 +00:00
Paulo Gomes fa00ec8fc7
Migrate from deprecated ginkgo async testing
https://github.com/onsi/ginkgo/blob/ver2/docs/MIGRATING_TO_V2.md\#removed-async-testing

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
2022-02-07 15:08:03 +00:00
Kevin McDermott c397ff902b Ensure that directories are included.
This avoids skipping the directories when generating the archive
tarball.

This makes it easier to scan directory trees.

Signed-off-by: Kevin McDermott <kevin@weave.works>

Ensure that directories are included.

This avoids skipping the directories when generating the archive
tarball.

This makes it easier to scan directory trees.

Signed-off-by: Kevin McDermott <kevin@weave.works>
2022-01-20 13:58:44 +00:00
Stefan Prodan 5be33770e8
Use patch instead of update when adding finalizers
This is needed to prevent source-controller from managing all the fields under `.spec`.

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2022-01-13 18:18:32 +02:00
Sanskar Jaiswal 2b8ede12cc fix makefile envtest and controller-gen usage
Refactor logic to install helper tools into one function in the
Makefile. Add support for envtest to help install tools like kubectl,
etcd which helps users run tests more conveniently.

Signed-off-by: Sanskar Jaiswal <sanskar.jaiswal@weave.works>
2022-01-13 16:48:04 +05:30
Tom Huang 5bb428349e
proper file close operation based on feedback
Signed-off-by: Tom Huang <tom.huang@weave.works>
2022-01-11 15:50:25 -05:00
Tom Huang 8868d3938a
Update file close operation to not use defer and add test case for CopyFromPath
Signed-off-by: Tom Huang <tom.huang@weave.works>
2022-01-11 13:23:17 -05:00
Stefan Prodan eacabe23a1
Log the error when tmp cleanup fails
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2022-01-07 10:37:06 +02:00
Aurel Canciu 22d0880e4d
Update flux pkg components
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
2021-12-20 14:47:44 +01:00
Hidde Beydals 5ddeb0934f controllers: use short SHA in chart SemVer meta
As the full version can be used as a label value, the full SHA from the
reference takes up too much space from the 63 characters available in
total.

To mitigate against this, we now take a "short" version of the first 12
characters, which was still unique for the Linux kernel in 2019 with
875.000 commits:
http://git-scm.com/book/en/v2/Git-Tools-Revision-Selection#Short-SHA-1

This should be sufficient to safely detect all changes within the
context of operations.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-12-02 22:28:18 +01:00
Hidde Beydals c793cd59da controllers: record suspension for HelmRepository
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-23 09:51:44 +01:00
Hidde Beydals 905602bdfe controllers: return err on auth dir create failure
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-23 09:51:44 +01:00
Hidde Beydals dbbef5add8 controllers: use `time.Since`
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-23 09:51:44 +01:00
Hidde Beydals 6a8b5889f1 controllers: absolute local path for cached chart
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-22 22:10:31 +01:00
Sunny 144766d03c
controllers: Fix helmchart values file merge test
Test case "Setting valid valuesFile attribute" and the tests around it
aren't isolated and most of the time pass because of the results from
the previous tests being re-read as the test expectation match the
previous test results. Failures are very rare to reproduce, even in
the CI they aren't seen but it failed very frequently on my computer,
especially this specific case because unlike the other cases, there is
just one file to be merged, which invalidates the chart result from
the previous cases.
In order to ensure the test wait for the chart to be updated by its
action and not by any other previous updates, status condition message
seems to be the most reliable way, as it also contains the paths of the
files that were merged.
With this change, I could no longer reproduce the failure on my
computer.
Reordering the tests makes this issue more clear.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2021-11-22 15:46:02 +05:30
Hidde Beydals 472eb12f43 controllers: set generation as version metadata
By providing the Generation of the object that is getting reconciled
as version metadata to the builder if any custom values files are
defined, the Artifact revision changes if the specification does,
ensuring consumers of the Artifact are able to react to changes in
values (and perform a release).

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-19 17:04:00 +01:00
Hidde Beydals a1e9302b7d internal/helm: "value files" -> "values files"
Previous usage while consistent, was incorrect, and inconsitent with
the field in the API spec.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-19 17:04:00 +01:00
Hidde Beydals 2b8134ce20 internal/helm: introduce typed BuildError
This commit introduces a typed `BuildError` to be returned by
`Builder.Build` in case of a failure.

The `Reason` field in combination with `BuildErrorReason` can be used
to signal (or determine) the reason of a returned error within the
context of the build process.

At present this is used to determine the correct Condition Reason, but
in a future iteration this can be used to determine the negative
polarity condition that should be set to indicate a precise failure to
the user.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-19 17:04:00 +01:00
Hidde Beydals 32e19ebcd0 controllers: more tidying of wiring
Dealing with some loose ends around making observations, and code
style.

The loaded byes of a chart are used as a revision to ensure e.g.
periodic builds with unstable ordering of items do not trigger a false
positive.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-19 17:04:00 +01:00
Hidde Beydals 7d0f79f41b internal/helm: divide into subpackages
With all the logic that used to reside in the `controllers` package
factored into this package, it became cluttered. This commit tries to
bring a bit more structure in place.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-11-19 17:04:00 +01:00