Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

automated and ignore annotations clash #1748

Closed
whitlaaa opened this issue Feb 19, 2019 · 5 comments
Closed

automated and ignore annotations clash #1748

whitlaaa opened this issue Feb 19, 2019 · 5 comments

Comments

@whitlaaa
Copy link

whitlaaa commented Feb 19, 2019

Having a resource annotated as both automated and ignored appears to be treated as both and seems to break automation of all cluster resources.

In our case, as long as we had some HelmRelease manifests annotated with both, none of the automated resources in our cluster were getting their images updated. Note that we were seeing the latest images available (via fluxctl list-images), they were just never used.

Upon removing one of the two, everything started flowing as normal.

One of our HelmReleases looked like this:

---
apiVersion: flux.weave.works/v1beta1
kind: HelmRelease
metadata:
  name: my-webapi
  namespace: default
  labels:
    chart: webapi
  annotations:
    flux.weave.works/automated: "true"
    flux.weave.works/tag.chart-image: glob:release-*
    flux.weave.works/ignore: true
...

With both of these annotations in place, we'd have log entries that seem to indicate it being both ignored and automated:

ts=2019-02-18T22:02:34.277718708Z caller=sync.go:66 component=sync-loop resource=default:helmrelease/my-webapi ignore=apply
ts=2019-02-18T22:07:27.170619954Z caller=images.go:33 component=sync-loop error="checking services for new images: helmreleases.flux.weave.works \"my-webapi\" not found"

After doing some digging, I think this occurs due to an issue in pollForNewImages.

It looks like GetUnlockedAutomatedResources doesn't take the policy.Ignore into account, so the image check fails when comparing with a non-existent flux controller on line 33. Since that error then causes a return, it short circuits image updates for everything else.

@hiddeco
Copy link
Member

hiddeco commented Feb 19, 2019

Thanks a lot for your high quality bug report and proposed solution. 🥇

I validated the bug (valid) and your proposed solution (correct) and turned it into #1749. I am aiming to get a patch release out as soon as possible after this has been merged into master.

@2opremio
Copy link
Contributor

and turned it into #1479

#1749 , for the sake of correctness. Numbers like to move :)

@whitlaaa
Copy link
Author

whitlaaa commented Feb 19, 2019

Awesome! To be honest, if this was still open by the weekend I was going to use it to cut my teeth on Go testing a bit more. 😁

Thanks for verifying so quickly!

@hiddeco
Copy link
Member

hiddeco commented Feb 19, 2019

To be honest, if this was still open by the weekend I was going to use it to cut my teeth on Go testing a bit more.

Our help-wanted label is always looking for contributors 😄

@hiddeco
Copy link
Member

hiddeco commented Feb 19, 2019

@whitlaaa until we have settled and prepped for release quay.io/weaveworks/flux:master-c9efad89 is your friend.

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

No branches or pull requests

3 participants