-
Notifications
You must be signed in to change notification settings - Fork 90
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 NGF fails to recover if conf files are unexpectedly removed #1132
Fix NGF fails to recover if conf files are unexpectedly removed #1132
Conversation
a00bdb7
to
a2c6b27
Compare
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.
looks good! one change request
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.
🚀
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.
👍
ae37218
to
06cdadf
Compare
Proposed changes
Problem: When the
http.conf
file did not exist, any updates to NGF would cause NGF to error as it could not "replace" that file as it didn't exist.Solution: Added a check to see if the error returned from trying to remove a conf file was a
IsNotExist
, if so, continue and act as if the file has been deleted.Testing: Manually tested that NGF was able to recover after manually deleting the
http.conf
file and deploying an example. Also checked forconfig-version.conf
and saw the same fixed results.Note: Part of AC was determining how the
http.conf
file could be removed as a part of our normal workflow and I worked with @kate-osborn to run through some of the most realistic scenarios and they all felt very unlikely to happen and we decided to log an error message at the debug level for when this occurs and not to pursue looking into it anymore. Perhaps when looking through log messages if one sees the log message, they can revisit this situation with more information.Closes #1110
Checklist
Before creating a PR, run through this checklist and mark each as complete.