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

Fix initContainer failure during pod restart #337

Merged
merged 1 commit into from
Mar 23, 2023
Merged

Fix initContainer failure during pod restart #337

merged 1 commit into from
Mar 23, 2023

Conversation

pl4nty
Copy link
Contributor

@pl4nty pl4nty commented Dec 17, 2022

Proposed changes

The gateway initContainer fails when creating folders on the config volume, if they already exist:

mkdir: can't create directory '/etc/nginx/conf.d': File exists
mkdir: can't create directory '/etc/nginx/secrets': File exists

This failure prevents the gateway from starting. This PR adds a mkdir flag to ignore the error and skip folder creation if the folders already exist.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@pl4nty pl4nty requested a review from a team as a code owner December 17, 2022 07:56
@pl4nty
Copy link
Contributor Author

pl4nty commented Dec 17, 2022

This fix resolved the error in my environment, but I'm not sure which tests are required to validate it. I'm also unsure how files were created on the EmptyDir before init. Possibly a successful init previously, then a restart on the same node.

@kate-osborn
Copy link
Contributor

@pl4nty thanks for the PR!

Instead of adding -p flag to the mkdir commands, can you clear out or remove the directories before creating them? I'd like to avoid having out-of-date or invalid configs in these directories after a restart.

@pl4nty
Copy link
Contributor Author

pl4nty commented Dec 23, 2022

Thanks @kate-osborn, how's this? Just removes the stale config if present. I've tested successfully on my dev cluster

@kate-osborn
Copy link
Contributor

Thanks @kate-osborn, how's this? Just removes the stale config if present. I've tested successfully on my dev cluster

LGTM!

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pl4nty

There is a small problem in the script -- please see the review comment with the fix suggestion.

Could you also rebase your branch against the latest main?

deploy/manifests/nginx-gateway.yaml Outdated Show resolved Hide resolved
@kate-osborn kate-osborn self-requested a review January 5, 2023 19:01
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Jan 9, 2023
@kate-osborn
Copy link
Contributor

@pl4nty the commit history on this PR has several commits from the main branch which is making the diff incorrect. Can you do an interactive rebase on main and squash all the commits into a single commit? That should fix the diff. Thanks!

@github-actions github-actions bot removed the dependencies Pull requests that update a dependency file label Feb 6, 2023
@pl4nty
Copy link
Contributor Author

pl4nty commented Feb 6, 2023

@kate-osborn apologies, I was away for a while. Thanks for your patience!

@kate-osborn kate-osborn merged commit 99320a5 into nginxinc:main Mar 23, 2023
@pleshakov pleshakov changed the title Ignore error during gateway init if folders already exist in volume Fix initContainer failure during pod restart Apr 21, 2023
@pleshakov pleshakov added the bug Something isn't working label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants