Skip to content

Commit

Permalink
Fix NGF fails to recover if conf files are unexpectedly removed (#1132)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bjee19 authored Oct 13, 2023
1 parent 084ddcd commit 5324908
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
12 changes: 10 additions & 2 deletions internal/mode/static/nginx/file/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,18 @@ func NewManagerImpl(logger logr.Logger, osFileManager OSFileManager) *ManagerImp
func (m *ManagerImpl) ReplaceFiles(files []File) error {
for _, path := range m.lastWrittenPaths {
if err := m.osFileManager.Remove(path); err != nil {
if os.IsNotExist(err) {
m.logger.Info(
"File not found when attempting to delete",
"path", path,
"error", err,
)
continue
}
return fmt.Errorf("failed to delete file %q: %w", path, err)
}

m.logger.Info("deleted file", "path", path)
m.logger.Info("Deleted file", "path", path)
}

// In some cases, NGINX reads files in runtime, like a JWK. If you remove such file, NGINX will fail
Expand All @@ -106,7 +114,7 @@ func (m *ManagerImpl) ReplaceFiles(files []File) error {
}

m.lastWrittenPaths = append(m.lastWrittenPaths, file.Path)
m.logger.Info("wrote file", "path", file.Path)
m.logger.Info("Wrote file", "path", file.Path)
}

return nil
Expand Down
20 changes: 20 additions & 0 deletions internal/mode/static/nginx/file/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,26 @@ var _ = Describe("EventHandler", func() {
})
})

When("file does not exist", func() {
It("should not error", func() {
fakeOSMgr := &filefakes.FakeOSFileManager{}
mgr := file.NewManagerImpl(zap.New(), fakeOSMgr)

files := []file.File{
{
Type: file.TypeRegular,
Path: "regular-1.conf",
Content: []byte("regular-1"),
},
}

Expect(mgr.ReplaceFiles(files)).ToNot(HaveOccurred())

fakeOSMgr.RemoveReturns(os.ErrNotExist)
Expect(mgr.ReplaceFiles(files)).ToNot(HaveOccurred())
})
})

When("file type is not supported", func() {
It("should panic", func() {
mgr := file.NewManagerImpl(zap.New(), nil)
Expand Down

0 comments on commit 5324908

Please sign in to comment.