Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flux reconcile hr: double upgrade/redeployment #1882

Closed
1 task done
patrikbeno opened this issue Oct 1, 2021 · 9 comments
Closed
1 task done

flux reconcile hr: double upgrade/redeployment #1882

patrikbeno opened this issue Oct 1, 2021 · 9 comments

Comments

@patrikbeno
Copy link

patrikbeno commented Oct 1, 2021

Describe the bug

flux reconcile hr --with-source causes 2 helm upgrades.

Proof is in the helm history.

For simple deployments, this may be just an annoyance because the 2nd upgrade will be a no-op.
However, if you have some post-upgrade hooks configured, they will be executed twice, once for every upgrade.
This may or may not be an issue but it is certainly undesired.

Steps to reproduce

helm create qdh
cd qdh
git commit
git push

...

flux create source git --interval=1h  ... 
flux create helmrelease --interval=1h  ...

...

update chart version, commit, push

flux reconcile hr --with-source ...

helm history qdh

Expected behavior

single helm upgrade, of course

Screenshots and recordings

No response

OS / Distro

Windows 10 + Alpine Linux in K8s

Flux version

0.17.2

Flux check

$ flux check
► checking prerequisites
✔ kubectl 1.20.2 >=1.18.0-0
✔ Kubernetes 1.21.5 >=1.16.0-0
► checking controllers
✔ helm-controller: deployment ready
► ghcr.io/fluxcd/helm-controller:v0.11.2
✔ kustomize-controller: deployment ready
► ghcr.io/fluxcd/kustomize-controller:v0.14.1
✔ notification-controller: deployment ready
► ghcr.io/fluxcd/notification-controller:v0.16.0
✔ source-controller: deployment ready
► ghcr.io/fluxcd/source-controller:v0.15.4
✔ all checks passed

Git provider

No response

Container Registry provider

No response

Additional context

Workaround:

name=qdh
namespace=default

# reconcile git source instead of HelmRelease
flux reconcile source git gitops -n $namespace

# this causes HelmChart update
kubectl wait --for=condition=ready HelmChart $namespace-$name -n $namespace

# this casues HelmRelease reconcile
kubectl wait --for=condition=ready HelmRelease $name -n $namespace --timeout=5m

# now it's done & ok
flux get hr $name -n $namespace
helm  history $name -n $namespace --max=2

Code of Conduct

  • I agree to follow this project's Code of Conduct
@somtochiama
Copy link
Member

This happens because the source is annotated first(which triggers the first upgrade as the helm-controller watches it) and then the helm release is annotated again(second upgrade).
Not sure if annotating just the source object might be sufficient to fix this @hiddeco

@patrikbeno
Copy link
Author

patrikbeno commented Oct 1, 2021

If the GitRepository has interval=1m and HelmRelease has interval=1h, then chart update in GitRepository triggers HelmRelease reconciliation after 1m even if HelmRelease should be in quiet mode for next hour.

Feature/Bug? Dunno. Just unexpected.

Seems GitRepository updates should not trigger HelmRelease reconciliation as side effect.

HelmChart is updated from GitRepository but since HelmChart belongs to HelmRelease, its updates should not be driven by GitRepository events, but HelmRelease events instead.

@stefanprodan
Copy link
Member

then chart update in GitRepository triggers HelmRelease reconciliation

Yes all Flux reconcilers are triggered by source changes that's by design.

@patrikbeno
Copy link
Author

Yes all Flux reconcilers are triggered by source changes that's by design.

I get that, to a point.

HelmRelease has a HelmChart source which has a GitRepository source

interval parameter or explicit reconcile commands indicate that it's a pull model.

What you say indicates push model. If so, interval on a HelmRelease does not make much sense, since it's driven by its source anyway.

@stefanprodan
Copy link
Member

stefanprodan commented Oct 1, 2021

If so, interval on a HelmRelease does not make much sense, since it's driven by its source anyway.

The interval is there because Kubernetes events are not guaranteed, due to API server congestion and other network related issues is possible that the controller doesn't receive the update event. No matter if the event is received or not, Flux uses the interval to reconcile the cluster to the latest revision. Another function of interval is that Flux can correct drift in-cluster, such as a manual helm upgrade that is rollback at the specified interval.

@tbondarchuk
Copy link

Shouldn't second reconciliation (interval triggered) of HelmRelease not update anything since the one started by source update already made all necessary changes?

And might there by some issue with how helm-controller treats simultaneously running reconciliations, as in issues linked above?

@hiddeco
Copy link
Member

hiddeco commented Oct 8, 2021

A resource is never worked on by two processes at the same time as the queue won't allow this. What you are observing in that issue is likely a race condition that occurs between an apply and a condition related update.

These errors will no longer happen in an upcoming (to be announced) release, as we will be introducing a more intelligent patcher in the near future: https://pkg.go.dev/github.com/fluxcd/pkg/[email protected]/patch

@philipsabri
Copy link

@hiddeco

These errors will no longer happen in an upcoming (to be announced) release, as we will be introducing a more intelligent patcher in the near future: https://pkg.go.dev/github.com/fluxcd/pkg/[email protected]/patch

Any updates on this one?

@stefanprodan
Copy link
Member

Fixed in fluxcd/helm-controller#738

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants