-
Notifications
You must be signed in to change notification settings - Fork 96
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/increase nginx timeout #777
Fix/increase nginx timeout #777
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increase the pid timeout (i.e. the time we wait for nginx to start) to 10s. This is sufficient in my local environment.
I wonder if the timeout also depends on the time it takes for Docker to download NGINX image. if that is the case, it might take more than 10s in different environments and perhaps pre-loading NGINX image can also help?
perhaps the big problem here is that NKG once it fails to reload, doesn't try to recover from it and reload again? (unless the config is re-generated for some reason)
if NKG supported retrying, it would be able to wait until NKG started and eventually fix the statuses. I think being able to fix that will be important not only for conformance tests but for production (since the timeout 10s might not be enough)
at the same time, approving this PR, since it fixes the issue for the conformance tests and the other changes also make sense 👍
9e5a7d3
to
b9d3a96
Compare
I think that's a smart idea. We should do that in our conformance test pipeline job @ciarams87 @lucacome
Yep, we need to handle this. Do we have a ticket for this yet? If not, I can create one. |
we have a related #664 but nothing about retrying reloads |
Updated the ticket to include retries |
Proposed changes
Problems:
Solutions:
Testing: Ran the conformance tests locally. Confirmed that they will consistently pass setup and run to completion.
Checklist