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 seems to be overriding sidecar image tag #1908

Closed
crw opened this issue Mar 24, 2018 · 13 comments
Closed

Flux seems to be overriding sidecar image tag #1908

crw opened this issue Mar 24, 2018 · 13 comments
Labels
bug broken end user functionality; not working as the developers intended it component/flux-api stale Bulk closing old, stale issues

Comments

@crw
Copy link

crw commented Mar 24, 2018

Check out their YAML and this screen shot. In the words of the customer:

deploying a new version of one container, also changes the image on the sidecar? We clicked the "upgrade" button in the weave interface, and this is the diff which was generated

screenshot 2018-03-23 11 57 09

YAML: https://gist.github.com/wkm/c39b378c8b27b4aea50a03520c65f679

PLEASE DO NOT MOVE TO PUBLIC REPO: sharing private customer data.

@crw
Copy link
Author

crw commented Mar 24, 2018

Zendesk ticket #25 has been linked to this issue.

@squaremo
Copy link
Contributor

squaremo commented Mar 24, 2018

Is the yaml in question in a List resource (like this example: https://github.com/kubernetes/kubernetes/blob/master/hack/testdata/list.yaml) or a multidoc (like this: https://github.com/kubernetes/kubernetes/blob/master/hack/testdata/multi-resource-yaml.yaml)?

EDIT: didn't notice the link to the full YAML. It's not a list or multidoc. Will anonymise and try locally.

@squaremo
Copy link
Contributor

squaremo commented Mar 24, 2018

Adding the gist'ed YAML as a test case in weaveworks/flux/cluster/kubernetes/update_test.go, I can see it fail in exactly the same way shown above in master. After

git checkout 1.2.4 -- cluster/kubernetes/update.go

the test case does not fail. Narrowing it down a bit more, it turns out to be flux PR 1008 where it starts failing the test case. Not sure why, yet.

@squaremo
Copy link
Contributor

That was a false accusation! It looks like it worked before then because it doesn't report that test failing, but it does that because it stops at the first failure. Silly me (and silly tests).

In fact, it is the indentation of the containers that is fooling it. I can reindent one of the earlier test cases, and see it fail in both 1.2.4 and master.

@squaremo
Copy link
Contributor

Workaround: don't over-indent the container specs; they only need to be level with the containing key, like this:

containers:
- name: foo
  image: gcr.io/image1:v1
- name: bar
  image: gcr.io/image2:v1

But having extra indentation is valid YAML, so fluxd really ought to handle it.

@crw crw added the bug broken end user functionality; not working as the developers intended it label Mar 26, 2018
@crw
Copy link
Author

crw commented Mar 26, 2018

Comment made from Zendesk by Craig Wright on 2018-03-26 at 18:56:

Request #26 "Re: Hi from Weave Cloud!" was closed and merged into this request. Last comment in request #26:

Hi David,

Yes, thanks for asking! We have confirmed it as an issue with Flux, but there is a workaround. Essentially, we are misinterpreting the indentation in your YAML file. So, although it is not invalid to do so, the way your YAML is indented is causing the problem. Workaround: indent the container specs so that image is on the same indentation level as the containing key, like this:

containers:

  • name: foo

image: gcr.io/image1:v1

  • name: bar

image: gcr.io/image2:v1

If you can, please try changing your indent level and see if that helps. I will keep you posted with regards to getting that bug fixed.

(Note, we are moving to Zendesk for managing these types of support inquiries, so I added [email protected] to this thread. In the future, please include that email address, it will help the team follow the conversation. Thanks!)

Thanks!

Craig Wright

Customer Success, Weaveworks

https://www.weave.works/

@crw
Copy link
Author

crw commented Mar 26, 2018

Comment made from Zendesk by Ben Grohsgal on 2018-03-26 at 20:12:

Hi Craig,

I'm Ben, another engineer here who works with David.
Looking at your recommendation / example along with out yaml it seems like we're already doing what you recommend:


Are we misunderstanding? Is this a bug this workaround if not sufficient to resolve?

Thanks,

Ben

@crw
Copy link
Author

crw commented Mar 26, 2018

Comment made from Zendesk by Craig Wright on 2018-03-26 at 20:21:

Hi Ben,

Check out this part of the file:
containers:
- name: citizen-gcr-notifier
image: gcr.io/citizen-gcr/notifier:37f4b214ace2f3bc7dda8c828259b7e578c2390e
So the line items can actually be indented in-line with "containers" as in the example:
containers: 
- name: foo
image: gcr.io/image1:v1
- name: bar
image: gcr.io/image2:v1
Does that make sense? As mentioned, your style of indentation is correct YAML, the bug is on our end... but if you can make the change above, it should work. 

Let me know if this works for you. 

Thanks!

@crw
Copy link
Author

crw commented Mar 26, 2018

Comment made from Zendesk by Ben Grohsgal on 2018-03-26 at 20:22:

Ah sorry, misread - will give it a shot and see if it works.

@crw
Copy link
Author

crw commented Mar 27, 2018

Comment made from Zendesk by Ben Grohsgal on 2018-03-27 at 15:07:

Works! Thanks for the help

On Mon, Mar 26, 2018 at 4:22 PM, Ben Grohsgal <[email protected]> wrote:
Ah sorry, misread - will give it a shot and see if it works.

@crw
Copy link
Author

crw commented Mar 27, 2018

Comment made from Zendesk by Craig Wright on 2018-03-27 at 17:11:

Awesome to hear it, thanks for the confirmation!

@crw
Copy link
Author

crw commented Mar 27, 2018

Comment made from Zendesk by David on 2018-03-27 at 23:49:

Hello Craig,

The flux agent is still getting a 429 "Quota Exceeded" error, here we have changed some flux settings, still not luck.
Is there anything we can do here?

containers:
- name: flux-agent
image: quay.io/weaveworks/flux:1.2.5
args:
- --token=$(WEAVE_CLOUD_TOKEN)
- --connect=wss://cloud.weave.works./api/flux
- --memcached-hostname=weave-flux-memcached.weave.svc.cluster.local

- --git-path=orchestration
- --git-branch=master
- --git-label=flux-summer-rock-55
- --registry-rps=20
- --registry-poll-interval=10m
- --registry-burst=10

@crw
Copy link
Author

crw commented Mar 28, 2018

Comment made from Zendesk by Craig Wright on 2018-03-28 at 00:36:

Hi David,

This is kind of silly but can you send that same email to [email protected], again? It got attached to the last ticket, and I want to track this as a separate issue. This will help me help the dev team be less confused about which issues we are tracking. 

Thanks!

@ozamosi ozamosi added the stale Bulk closing old, stale issues label Nov 4, 2021
@ozamosi ozamosi closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug broken end user functionality; not working as the developers intended it component/flux-api stale Bulk closing old, stale issues
Projects
None yet
Development

No branches or pull requests

4 participants