feature: sync subcharts from different registries #172
Conversation
chrissgyulev
force-pushed
the
task/sync-chart-depedencies
branch
from
13c5584 to
3da9e2e
Compare
There was a problem hiding this comment.
Hi @chrissgyulev!
Thank you so much for contributing to this repo!
I have added a few comments, but I have a slightly different proposal to tackle this issue:
- Remove
target.replaceDependencyRepo - Replace
source.trustedSourceDepsbysource.ignoreTrustedRepos. This option will ignore certain "trusted" repositories from the sync process (so the resulting Helm Chart will use the same external repository). - Add a
target.syncTrustedRepos. This option will enforce syncing certain "trusted" repositories during the sync process (so the resulting Helm Chart will include references to the target repository, where the charts included in the trusted repos should be present after the process).
NOTES: source.ignoreTrustedRepos can be ignored if we don't want to just ignore some repositories and, instead, go for an AON (All-Or-None) approach.
The main reason to follow this approach is that an unexperienced reader will clearly identify which repositories will be ignored from source vs which repositories will be fully synced into the target. By having two different settings that interact internally (this PR) might be troublesome to understand and debug if the code gets more complex.
charts-syncer.yaml
Outdated
| @@ -4,6 +4,11 @@ | |||
|
|
|||
| # source includes relevant information about the source chart repository | |||
| source: | |||
| # Dependencies located in repos from this list will be considered as trusted, and also synced. | |||
There was a problem hiding this comment.
they will be synced only if we enable target.replaceDependencyRepo = true
charts-syncer.yaml
Outdated
| # repoName is used to modify the README of the chart. Default value: `myrepo` | ||
| # In case there is a need to mirror dependencies (from trustedSourceDeps list, see above) - this must be set to true | ||
| replaceDependencyRepo: true | ||
| # repoName is used to modify the README of the chart. Default value: `myrepo` |
docker-compose.yaml
Outdated
| @@ -0,0 +1,11 @@ | |||
| services: | |||
There was a problem hiding this comment.
even though I am glad you took the time to craft this, we don't want to support docker compose. Please drop this file.
|
OK, I'll make the adjustments taking into account your comments @jotadrilo . Do some testing and get back to you for another review. Thank you for your time to review my proposal. Regards |
It's really great that you started this PR. We are also in need of a solution for this. Did you @chrissgyulev by any chance find some time to adapt the PR? |
|
@MShekow Hi ! I'll try to find some spare time to finish it soon. My apologies. |
Hey @chrissgyulev any luck yet? We would really benefit from it. |
chrissgyulev
force-pushed
the
task/sync-chart-depedencies
branch
from
28896ff to
92e4d21
Compare
|
Hi again ! Could you take a look ? @MShekow could you help us, testing your case ? Some things to consider:
Thank you very much for your patience! Regards, |
|
@chrissgyulev Thanks for updating the PR. It works with our use case, as expected 👍 |
|
There is one problematic issue, though: synchronization now takes forever. I use a self-built Docker image (using your When running it with your version, I get this: As you can see, checking each image takes ca. 0.5 sec with the upstream image, and ca. 20 sec with your image :( UPDATE: from the looks of it, it seems that this is because your branch is outdated (13 commits behind master), and the speed up happens in one of these 13 commits. I guess that would be solved by rebasing. |
… for testing the new feature
chrissgyulev
force-pushed
the
task/sync-chart-depedencies
branch
from
92e4d21 to
009c35d
Compare
|
OK @MShekow ! I've rebased this branch with master. Can you try again, please ? If possible with exactly the same configuration. Thank you very much! |
I just checked it out. The performance is now good again, like in the upstream image. Thank you |
|
@jotadrilo / @tompizmor could you take a look ? Regards, |
|
OUTDATED, see comment below @chrissgyulev I found another problem: the sync generally seems to be broken. Example YAML: source:
repo:
kind: HELM
url: https://argoproj.github.io/argo-helm
ignoreTrustedRepos:
- kind: HELM # we do not want to synchronize the redis-ha subchart
url: https://dandydeveloper.github.io/charts
target:
repo:
kind: OCI
url: https://our-hosted-registry.com/some-namespace/synced-helm-charts
charts:
- argo-cd
Log output (with The problem occurs here: 
The I created a PR with an idea for how to fix it. See chrissgyulev#1 |
|
You can discard most of the above comment. I simply mistyped the URL for I also learnt that sometimes errors arise because of older tier1-charts referencing different sub-charts (than the most current one). So, for instance, for the Still, I think that the PR I created (chrissgyulev#1) may be useful, because it skips dependencies/sub-charts early. The use case is that we have limited access to the Internet. We need to get firewall clearances for every DNS/Host. Thus, we want to limit the number of rules to a minimum, and thus we would like to avoid trying to look up charts which we do not need anyway. |
|
ok @MShekow let me take a look. Thank you very much |
|
Is this feature going to be added back to the next release ? |
Adds a feature to sync sub-charts from non-source registries
Related Issues:
Activated by adding a list of repos via trustedSourceDeps:
Also adds replaceDependencyRepo
Take a look at examples/sync-deps for full example. This example addresses directly #147 (could be launched via newly added docker-compose.yaml)
Other minor improvements