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

update post-render to retry updating service account on error #269

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

YrrepNoj
Copy link
Contributor

@YrrepNoj YrrepNoj commented Feb 4, 2022

We experienced a race condition when updating the ServiceAccount for a newly created namespace.
Sometimes KIND distro would error because it was still updating the SA at the same time we were.

Without this change we were sometimes seeing the following error on KIND clusters:
DEBUG unable to update the default service account for the demo namespace: Operation cannot be fulfilled on serviceaccounts "default": the object has been modified; please apply your changes to the latest version and try again

I've tested this several times (more than 5..) locally (macOS and Linux) and have not run into the ImagePullBackoff because of an un-updated ServiceAccount.

@YrrepNoj YrrepNoj requested a review from jeff-mccoy February 4, 2022 22:12
@YrrepNoj
Copy link
Contributor Author

YrrepNoj commented Feb 4, 2022

I'm not a fan of naive retry loops but I couldn't find any way to check if KIND was done updating the SA before we try to add our keys and label.

Any thoughts? @RothAndrew @jeff-mccoy ?

@RothAndrew
Copy link
Contributor

It's definitely a bandaid fix, but I don't have any objections at this point in time.

@jeff-mccoy
Copy link
Contributor

I'll take a look in a couple hours if that's okay--I might have an idea.

@jeff-mccoy
Copy link
Contributor

@YrrepNoj I am unable to reproduce the original issue so having a hard time validating this. When you get a chance could you try this instead? This is how the K8s E2E tests do it and I think is a bit cleaner than a retry loop. Thanks!

@YrrepNoj
Copy link
Contributor Author

YrrepNoj commented Feb 6, 2022

@jeff-mccoy It doesn't seem to be happening nearly as often but I got the error message after about 10-15 attempts
DEBUG unable to update the default service account for the demo namespace: Operation cannot be fulfilled on serviceaccounts "default": the object has been modified; please apply your changes to the latest version and try again WARNING Unable to update the default service account for the demo namespace

My assumption is that KIND will create a blank/minimalistic ServiceAccount and then create a background job to update it with some metadata or something. Since this package is a lot more lightweight than any of the other packages I guess it just happens to run into this issue.

I got this result on a Ubuntu 18.04 machine with an Intel CPU. Zarf binary was built with version tag v0.14.0-99-g58c7321 and zarf package inspect on the data-injection package shows it was built from the same version.

@jeff-mccoy
Copy link
Contributor

Sorry which package is doing this?

@YrrepNoj
Copy link
Contributor Author

YrrepNoj commented Feb 6, 2022

data-injection

@RothAndrew RothAndrew force-pushed the multi-distro-support branch from 614b4d3 to 4c802d3 Compare February 7, 2022 17:03
@jeff-mccoy jeff-mccoy force-pushed the multi-distro-support branch 2 times, most recently from 614b4d3 to 53101f9 Compare February 8, 2022 00:07
We experienced a race condition when updating the ServiceAccount for a newly created namespace.
Sometimes KIND distro would  error because it was still updating the SA at the same time we were.
@YrrepNoj YrrepNoj force-pushed the retry-updating-service-account branch from 58c7321 to f51f413 Compare February 8, 2022 00:53
@YrrepNoj
Copy link
Contributor Author

YrrepNoj commented Feb 8, 2022

@jeff-mccoy all cleaned up ;)

@YrrepNoj YrrepNoj merged commit c95de62 into multi-distro-support Feb 8, 2022
@YrrepNoj YrrepNoj deleted the retry-updating-service-account branch February 8, 2022 01:49
jeff-mccoy pushed a commit that referenced this pull request Feb 8, 2022
We experienced a race condition when updating the ServiceAccount for a newly created namespace.
Sometimes KIND distro would  error because it was still updating the SA at the same time we were.
jeff-mccoy added a commit that referenced this pull request Feb 8, 2022
jeff-mccoy pushed a commit that referenced this pull request Feb 8, 2022
We experienced a race condition when updating the ServiceAccount for a newly created namespace.
Sometimes KIND distro would  error because it was still updating the SA at the same time we were.

Signed-off-by: Jeff McCoy <code@jeffm.us>
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
We experienced a race condition when updating the ServiceAccount for a newly created namespace.
Sometimes KIND distro would  error because it was still updating the SA at the same time we were.

Signed-off-by: Jeff McCoy <code@jeffm.us>
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

Successfully merging this pull request may close these issues.

3 participants