It looks like the use of chartutil.ProcessDependencies in the HelmChart
Controller was not correct, this method seems to be used in Helm only
during install/upgrade. The intention was to load the dependencies but
this seems to not be needed as it's already done through the loaders
(loader.Load).
The use of this method caused a regression where Chart.yaml files would
be overwritten and registered subcharts that had aliases would be
renamed using the alias name. While this is an expected behaviour of
chartutil.ProcessDependencies it is not what the controller should do
to the chart during (re)packaging.
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
Non-packaged charts that don't have their dependencies present in
charts/ will now have these dependencies built using the
DependencyManager. The idea behind it is to replicate the logic
implemeneted in Helm's downloader.Manager with the support for already
existing HelmRepository resources and their chart retrieval capabilities.
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
To facilitate an inexpensive lookup when collecting credentials and
index artifacts while working with chart dependencies.
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
The feature allows the source-controller to load packaged helm charts
for HelmChart resource artifacts from GitRepository and Bucket sources
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.
As the storage base directory is determined during runtime, and
artifacts may live longer than that if they are e.g. stored in a
persistent volume but the mount path configuration changes.
Given that:
* The produced artifact as advertisted in the path should always
be a regular file (including the exclusion of symlinks).
* The produced artifact should be readable, so any type of error
should count as "does not exist".
We should use `os.Lstat` to not follow symlinks; return `false`
on any error we run in to, or return if the file mode information
reports a regular file.
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.
filepath.Walk can return a `nil` for the stat value, when it does, the
directory is invalid and the error will be set. This causes a
panic+crash if the directory does not currently exist when
RemoveAllButCurrent is called.
The following patch makes the behavior an error instead.
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
- 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.
Introduces new helpers and config structs to all reconcilers to
set the max concurrent reconciles number.
Introduces a new flag `--concurrent` to configure the number of
concurrent reconciles per reconciler, defaults to `2`.
- Add the HelmChart types and controller
- Semver expressions are found by utilizing Helm repository index
helpers. As Helm makes use of `masterminds/semver`, the support
for i.e. ranges less mature than the `GitRepository` implementation.
- Recorded semver is as defined in the metadata of the chart. The
used name for the artifact does however include the checksum of the
chart archive, as chart maintainers may not always properly apply
semver.
- Switches to `sigs.k8s.io/yaml` for YAML operations as this among
other things is able to properly unmarshal embedded structures.
- Directly requeues on transient errors instead of using the defined
interval as a back-off strategy is applied on repeated failures.
Philip Laine
Aurel Canciu
Hidde Beydals
Stefan Prodan
Michael Bridgen
Erik Hollensbe
Hidde Beydals