feat: Ensure repo update is only run once (#2378)
* feat: Ensure repo update is only run once Perform a single Hang tight while we grab the latest from your chart repositories... ...Successfully got an update from the "glm-bitnami" chart repository ...Unable to get an update from the "fluent" chart repository (https://fluent.github.io/helm-charts): Get "https://fluent.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51893->185.199.108.153:443: read: connection reset by peer ...Unable to get an update from the "grafana" chart repository (https://grafana.github.io/helm-charts): Get "https://grafana.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51897->185.199.109.153:443: read: connection reset by peer ...Unable to get an update from the "ingress-nginx" chart repository (https://kubernetes.github.io/ingress-nginx): Get "https://kubernetes.github.io/ingress-nginx/index.yaml": read tcp 192.168.0.104:51894->185.199.110.153:443: read: connection reset by peer ...Unable to get an update from the "chartmuseum" chart repository (https://chartmuseum.github.io/charts): Get "https://chartmuseum.github.io/charts/index.yaml": read tcp 192.168.0.104:51896->185.199.110.153:443: read: connection reset by peer ...Successfully got an update from the "glm-chartmuseum" chart repository ...Successfully got an update from the "apollo" chart repository ...Successfully got an update from the "kyverno" chart repository ...Unable to get an update from the "mysql-operator" chart repository (https://mysql.github.io/mysql-operator/): Get "https://mysql.github.io/mysql-operator/index.yaml": read tcp 192.168.0.104:51903->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "metallb" chart repository (https://metallb.github.io/metallb): Get "https://metallb.github.io/metallb/index.yaml": read tcp 192.168.0.104:51904->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "dragonfly" chart repository (https://dragonflyoss.github.io/helm-charts/): Get "https://dragonflyoss.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51905->185.199.108.153:443: read: connection reset by peer ...Unable to get an update from the "openfga" chart repository (https://openfga.github.io/helm-charts): Get "https://openfga.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51907->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "cnpg" chart repository (https://cloudnative-pg.github.io/charts): Get "https://cloudnative-pg.github.io/charts/index.yaml": read tcp 192.168.0.104:51910->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "metrics-server" chart repository (https://kubernetes-sigs.github.io/metrics-server/): Get "https://kubernetes-sigs.github.io/metrics-server/index.yaml": read tcp 192.168.0.104:51913->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "ot-helm" chart repository (https://ot-container-kit.github.io/helm-charts/): Get "https://ot-container-kit.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51914->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "coredns" chart repository (https://coredns.github.io/helm): Get "https://coredns.github.io/helm/index.yaml": read tcp 192.168.0.104:51917->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "redis-operator" chart repository (https://ot-container-kit.github.io/helm-charts/): Get "https://ot-container-kit.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51912->185.199.111.153:443: read: connection reset by peer ...Unable to get an update from the "andrcuns" chart repository (https://andrcuns.github.io/charts): Get "https://andrcuns.github.io/charts/index.yaml": read tcp 192.168.0.104:51915->185.199.111.153:443: read: connection reset by peer ...Successfully got an update from the "gitlab-jh" chart repository ...Successfully got an update from the "hashicorp" chart repository ...Successfully got an update from the "incubator" chart repository ...Successfully got an update from the "jenkins" chart repository ...Successfully got an update from the "nvidia" chart repository ...Successfully got an update from the "elastic" chart repository ...Successfully got an update from the "projectcalico" chart repository ...Unable to get an update from the "juicefs" chart repository (https://juicedata.github.io/charts/): Get "https://juicedata.github.io/charts/index.yaml": read tcp 192.168.0.104:51919->185.199.111.153:443: read: connection reset by peer ...Successfully got an update from the "bitnami" chart repository Update Complete. ⎈Happy Helming!⎈ before running any commands, allowing us to safely pass --skip-refresh to avoid redundant repo updates for each chart with external dependencies. This reduces the number of repository refresh operations from O(n) to O(1) where n is the number of charts with remote dependencies. Co-authored-by: Javex <github@javex.eu> Signed-off-by: yxxhero <aiopsclub@163.com> * fix: ensure repo update only runs when repositories are configured This fixes CI issues where tests fail with 'no repositories found' error. The PR #2378 adds a single helm.UpdateRepo() call before running helm dep build commands. However, when no repositories are configured, this call fails. The fix adds a check for len(st.Repositories) > 0 before calling UpdateRepo(). Additionally, updated snapshot files to reflect the new output ordering where repo update happens before building dependencies. Signed-off-by: yxxhero <aiopsclub@163.com> * feat: Update test snapshots for single repo update The code changes in PR #2378 ensure that helm repo update is only run once before building dependencies. This requires updating test snapshots to include the 'Updating repo' output that now appears before 'Building dependency' messages. Updated snapshots: - chart_need/output.yaml - chart_need_enable_live_output/output.yaml - release_template_inheritance/output.yaml - environments_releases_without_same_yaml_part/output.yaml - environment_missing_in_subhelmfile/output.yaml - pr_560/output.yaml - environments_values_gotmpl_with_environment_name/output.yaml - postrenderer/output.yaml (fixed YAML structure) - oci_need/output.yaml Signed-off-by: yxxhero <aiopsclub@163.com> * fix: Correctly update test snapshots based on repository configuration Only update snapshots for tests that have repositories defined: - chart_need/output.yaml (has repositories - shows 'Updating repo') - chart_need_enable_live_output/output.yaml (has repositories - shows 'Updating repo') - release_template_inheritance/output.yaml (has repositories - shows 'Updating repo') Tests without repositories should NOT show 'Updating repo': - environments_releases_without_same_yaml_part/output.yaml - environments_values_gotmpl_with_environment_name/output.yaml - pr_560/output.yaml - environment_missing_in_subhelmfile/output.yaml - postrenderer/output.yaml (uses OCI dependencies) - oci_need/output.yaml (uses OCI dependencies) This matches the conditional logic in the code that only runs helm.UpdateRepo() when len(st.Repositories) > 0. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: correct snapshot test expectations for repo update optimization - Re-add trailing newlines to environment_missing_in_subhelmfile output - Restore correct chart paths (/... instead of ../../...) - Restore postrenderer output with cm2 ConfigMap and correct field order - Fixes CI test failures introduced by incorrect snapshot updates Signed-off-by: yxxhero <aiopsclub@163.com> * fix: update integration test expected lint output for repo update Include 'Updating repo' messages in expected lint output files to match the new behavior where helm repo update is run once before building dependencies. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: remove extra blank line from lint output files Integration test output files had an extra blank line that was not present in the expected output, causing test failures. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: update lint output for single repo update With the repo update optimization, lint runs only once with 'Updating repo' messages instead of running twice. Update expected output to match new single-run behavior. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: filter out repo update messages in lint test Update test runner to filter out repo update messages that are now generated by the single helm.UpdateRepo() call, keeping the expected lint output consistent with the original behavior. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: filter repo update messages from diff test Filter out repo update messages in diff test output to match new behavior where helm.UpdateRepo() is called once. Signed-off-by: yxxhero <aiopsclub@163.com> * Fix missing closing parenthesis in grep command Signed-off-by: yxxhero <aiopsclub@163.com> * fix: prevent --args flags from being passed to helm repo commands When helmfile template --args is used, the extra flags were being passed to helm repo update and helm repo add commands, which don't support all flags that helm template/install support. This caused failures when flags like --dry-run were passed via --args. The fix saves the extra flags before executing helm repo commands, clears them, and restores them afterwards to ensure repo commands run without unsupported flags. Fixes CI issue in PR #2378 where test issue-1749 fails with "Error: unknown flag: --dry-run" during helm repo update. Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com> Co-authored-by: Javex <github@javex.eu>
This commit is contained in:
yxxhero
GitHub
parent
11def2e7e8
commit
9964a2eacb
|
|
@ -204,6 +204,12 @@ func (helm *execer) AddRepo(name, repository, cafile, certfile, keyfile, usernam
|
|||
return fmt.Errorf("empty field name")
|
||||
}
|
||||
|
||||
savedExtra := helm.extra
|
||||
helm.extra = []string{}
|
||||
defer func() {
|
||||
helm.extra = savedExtra
|
||||
}()
|
||||
|
||||
switch managed {
|
||||
case "acr":
|
||||
helm.logger.Infof("Adding repo %v (acr)", name)
|
||||
|
|
@ -252,6 +258,11 @@ func (helm *execer) AddRepo(name, repository, cafile, certfile, keyfile, usernam
|
|||
|
||||
func (helm *execer) UpdateRepo() error {
|
||||
helm.logger.Info("Updating repo")
|
||||
savedExtra := helm.extra
|
||||
helm.extra = []string{}
|
||||
defer func() {
|
||||
helm.extra = savedExtra
|
||||
}()
|
||||
out, err := helm.exec([]string{"repo", "update"}, map[string]string{}, nil)
|
||||
helm.info(out)
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -1828,7 +1828,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
|
|||
}
|
||||
|
||||
if len(builds) > 0 {
|
||||
if err := st.runHelmDepBuilds(helm, concurrency, builds); err != nil {
|
||||
if err := st.runHelmDepBuilds(helm, concurrency, builds, opts); err != nil {
|
||||
return nil, []error{err}
|
||||
}
|
||||
}
|
||||
|
|
@ -1837,7 +1837,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
|
|||
}
|
||||
|
||||
// nolint: unparam
|
||||
func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, builds []*chartPrepareResult) error {
|
||||
func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, builds []*chartPrepareResult, opts ChartPrepareOptions) error {
|
||||
// NOTES:
|
||||
// 1. `helm dep build` fails when it was run concurrency on the same chart.
|
||||
// To avoid that, we run `helm dep build` only once per each local chart.
|
||||
|
|
@ -1848,7 +1848,21 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int,
|
|||
//
|
||||
// See https://github.com/roboll/helmfile/issues/1521
|
||||
|
||||
// Perform an update of repos once before running `helm dep build` so that we
|
||||
// can safely pass --skip-refresh to the command to avoid doing a repo update
|
||||
// for every iteration of the loop where charts have external dependencies.
|
||||
// Only do this if there are repositories configured.
|
||||
if len(builds) > 0 && !opts.SkipRefresh && len(st.Repositories) > 0 {
|
||||
if err := helm.UpdateRepo(); err != nil {
|
||||
return fmt.Errorf("updating repo: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
for _, r := range builds {
|
||||
// Never update the local repository cache here, since we've already
|
||||
// updated it before entering the loop
|
||||
r.skipRefresh = true
|
||||
|
||||
buildDepsFlags := getBuildDepsFlags(r)
|
||||
if err := helm.BuildDeps(r.releaseName, r.chartPath, buildDepsFlags...); err != nil {
|
||||
if r.chartFetchedByGoGetter {
|
||||
|
|
|
|||
|
|
@ -1,10 +1,12 @@
|
|||
Adding repo myrepo http://localhost:18080/
|
||||
"myrepo" has been added to your repositories
|
||||
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Updating repo
|
||||
Hang tight while we grab the latest from your chart repositories...
|
||||
...Successfully got an update from the "myrepo" chart repository
|
||||
Update Complete. ⎈Happy Helming!⎈
|
||||
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Saving 1 charts
|
||||
Downloading raw from repo http://localhost:18080/
|
||||
Deleting outdated charts
|
||||
|
|
|
|||
|
|
@ -2,10 +2,12 @@ Live output is enabled
|
|||
Adding repo myrepo http://localhost:18081/
|
||||
"myrepo" has been added to your repositories
|
||||
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Updating repo
|
||||
Hang tight while we grab the latest from your chart repositories...
|
||||
...Successfully got an update from the "myrepo" chart repository
|
||||
Update Complete. ⎈Happy Helming!⎈
|
||||
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Saving 1 charts
|
||||
Downloading raw from repo http://localhost:18081/
|
||||
Deleting outdated charts
|
||||
|
|
|
|||
|
|
@ -1,6 +1,11 @@
|
|||
Adding repo myrepo http://localhost:18084/
|
||||
"myrepo" has been added to your repositories
|
||||
|
||||
Updating repo
|
||||
Hang tight while we grab the latest from your chart repositories...
|
||||
...Successfully got an update from the "myrepo" chart repository
|
||||
Update Complete. ⎈Happy Helming!⎈
|
||||
|
||||
Building dependency release=foo1, chart=../../charts/raw-0.1.0
|
||||
Building dependency release=foo2, chart=../../charts/raw-0.1.0
|
||||
Templating release=foo1, chart=../../charts/raw-0.1.0
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ for i in $(seq 10); do
|
|||
info "Comparing lint/chart-needs #$i"
|
||||
# Remove azuredisk-csi-driver repo to ensure consistent output (repo addition message)
|
||||
${helm} repo remove azuredisk-csi-driver &>/dev/null || true
|
||||
${helmfile} -f ${chart_need_case_input_dir}/${config_file} lint --include-needs | grep -v Linting | grep -v "has been removed" | grep -Ev "(Warning:.*is not a valid SemVerV2|\[WARNING\].*is not a valid SemVerV2|failed to load plugins)" > ${chart_needs_lint_reverse} || fail "\"helmfile lint\" shouldn't fail"
|
||||
${helmfile} -f ${chart_need_case_input_dir}/${config_file} lint --include-needs | grep -v Linting | grep -v "has been removed" | grep -Ev "(Updating repo|Hang tight while we grab|Update Complete|Successfully got an update from|Warning:.*is not a valid SemVerV2|\[WARNING\].*is not a valid SemVerV2|failed to load plugins)" > ${chart_needs_lint_reverse} || fail "\"helmfile lint\" shouldn't fail"
|
||||
diff -u ${lint_out_file} ${chart_needs_lint_reverse} || fail "\"helmfile lint\" should be consistent"
|
||||
done
|
||||
|
||||
|
|
@ -43,7 +43,7 @@ for i in $(seq 10); do
|
|||
info "Comparing diff/chart-needs #$i"
|
||||
# Remove azuredisk-csi-driver repo to ensure consistent output (repo addition message)
|
||||
${helm} repo remove azuredisk-csi-driver &>/dev/null || true
|
||||
${helmfile} -f ${chart_need_case_input_dir}/${config_file} diff --include-needs | grep -Ev "Comparing release=azuredisk-csi-storageclass, chart=.*/chartify.*/azuredisk-csi-storageclass" > ${chart_needs_diff_reverse}.tmp || fail "\"helmfile diff\" shouldn't fail"
|
||||
${helmfile} -f ${chart_need_case_input_dir}/${config_file} diff --include-needs | grep -Ev "(Updating repo|Hang tight while we grab|Update Complete|Successfully got an update from|Comparing release=azuredisk-csi-storageclass, chart=.*/chartify.*/azuredisk-csi-storageclass)" > ${chart_needs_diff_reverse}.tmp || fail "\"helmfile diff\" shouldn't fail"
|
||||
cat ${chart_needs_diff_reverse}.tmp | sed -E '/\*{20}/,/\*{20}/d' > ${chart_needs_diff_reverse}
|
||||
|
||||
# With --enable-live-output, there's a race condition that can cause non-deterministic ordering
|
||||
|
|
|
|||
Loading…
Reference in New Issue