Commit Graph

275 Commits

Author SHA1 Message Date
Sunny 28ec142cb7 helmchart: Replace GetInterval() with GetRequeueAfter()
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Sunny abc9f9b17e helmrepo: 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.
- Simplify logging and event recording
- Introduce controller-check/status checks to assert that the status
  conditions are valid at the end of the tests.
- Create variables for various condition groups: owned conditions, ready
  dependencies and ready dependencies negative.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Hidde Beydals 7f4f80bab4 Rewrite `HelmRepositoryReconciler` to new standards
This commit rewrites the `HelmRepositoryReconciler` 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.
- Refactoring of some Helm elements to make them easier to use within
  the new reconciler logic.
- 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-15 15:55:44 +05:30
Sunny a867303f54 helmrepo: Replace GetInterval() with GetRequeueAfter()
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Sunny 6808244b53 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-15 15:55:44 +05:30
Sunny 8b3caa28b3 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-15 15:55:44 +05:30
Sunny 4eccb045f1 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-15 15:55:44 +05:30
Sunny 8384ced601 Add more reconcileMinioSource test cases
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Sunny ec03a6ae2e BucketReconciler: Add reconcileArtifact tests
Add `BucketReconciler.reconcileArtifact` tests based on
`GitRepositoryReconciler.reconcileArtifact` test cases.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Hidde Beydals 11b2328012 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-15 15:55:44 +05:30
Hidde Beydals e26a0c3b6a 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-15 15:55:44 +05:30
Sunny 66a074778d bucket: Replace GetInterval() with GetRequeueAfter()
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Sunny 1e6f4d9a70 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-15 15:55:44 +05:30
Sunny 4502d15549 gitrepo: Use internal/util for creating temp dir
Signed-off-by: Sunny <darkowlzz@protonmail.com>
2022-02-15 15:55:44 +05:30
Sunny 8b43f6d7a7 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-15 15:55:38 +05:30
Sunny ea45903dd4 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-15 15:42:18 +05:30
Sunny 160432c022 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-15 15:42:18 +05:30
Hidde Beydals 4958ab3848 chore: ensure Git server dir is removed after test
Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-15 15:42:18 +05:30
Hidde Beydals 56b2a0cab7 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-15 15:42:18 +05:30
Hidde Beydals 68794ac22a 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-15 15:42:18 +05:30
Sunny febd79a4c5 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-15 15:42:18 +05:30
Sunny 12fff6f03f 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-15 15:42:18 +05:30
Sunny bbb3c9166c 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-15 15:42:18 +05:30
Hidde Beydals 7e8e66b130 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-15 15:42:18 +05:30
Hidde Beydals bb9f972e70 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-15 15:42:18 +05:30
Hidde Beydals a9d6cab286 Replace %q in messages with '%s'
Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-15 15:42:18 +05:30
Sunny d7a22b7289 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-15 15:42:18 +05:30
Sunny 0d53f08148 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-15 15:42:18 +05:30
Hidde Beydals 29caae3fca 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-15 15:42:14 +05:30
Sunny a0ea2481bb Add gomega matcher for artifact
Signed-off-by: Sunny <darkowlzz@protonmail.com>

Co-authored-by: Hidde Beydals <hello@hidde.co>
2022-02-15 15:40:57 +05:30
Hidde Beydals 6bf1385e4c 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-15 15:40:57 +05:30
Hidde Beydals 6ee2b95345 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-15 15:40:57 +05:30
Hidde Beydals 78a63acd55 Switch to v1beta2 API package
Signed-off-by: Hidde Beydals <hello@hidde.co>
2022-02-15 15:40:57 +05:30
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