Skip to content

Commit

Permalink
fix: don't delete an existing conflist (#2115) (#2189)
Browse files Browse the repository at this point in the history
  • Loading branch information
thatmattlong committed Aug 29, 2023
1 parent c01c2e7 commit 161a5f8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
37 changes: 30 additions & 7 deletions fs/atomic.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package fs

import (
"io"
"io/fs"
"os"
"path"

Expand All @@ -15,19 +16,39 @@ type AtomicWriter struct {

var _ io.WriteCloser = &AtomicWriter{}

// NewAtomicWriter returns an io.WriteCloser that will write contents to a temp file and move that temp file to the destination filename. If the destination
// filename already exists, this constructor will copy the file to <filename>-old, truncating that file if it already exists.
func NewAtomicWriter(filename string) (*AtomicWriter, error) {
// if a file already exists, copy it to <filname>-old
exists := true
if _, err := os.Stat(filename); err != nil {
if os.IsNotExist(err) {
existingFile, err := os.Open(filename)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
exists = false
} else {
return nil, errors.Wrap(err, "unable to stat existing file")
return nil, errors.Wrap(err, "error opening existing file")
}
}

if exists {
if err := os.Rename(filename, filename+"-old"); err != nil {
return nil, errors.Wrap(err, "unable to move existing file from destination")
// os.Create truncates existing files so we'll keep overwriting the <filename>-old and not filling up the disc if the
// process calls this over and over again on the same filename (e.g. if CNS uses this for conflist generation and keeps crashing and re-writing)
oldFilename := filename + "-old"
oldFile, createErr := os.Create(oldFilename)
if createErr != nil {
if closeErr := existingFile.Close(); closeErr != nil {
return nil, errors.Wrapf(createErr, "error closing file: [%v] occurred when handling file creation error", closeErr.Error())
}
return nil, errors.Wrapf(createErr, "error creating file %s", oldFilename)
}

// copy the existing file to <filename>-old
if _, err := io.Copy(oldFile, existingFile); err != nil { //nolint:govet // shadowing err is fine here since its encapsulated in the if block
return nil, errors.Wrapf(err, "error copying existing file %s to destination %s", existingFile.Name(), oldFile.Name())
}

if err := existingFile.Close(); err != nil { //nolint:govet // shadowing err is fine here since its encapsulated in the if block
return nil, errors.Wrapf(err, "error closing file %s", existingFile.Name())
}
}

Expand All @@ -39,18 +60,20 @@ func NewAtomicWriter(filename string) (*AtomicWriter, error) {
return &AtomicWriter{filename: filename, tempFile: tempFile}, nil
}

// Close closes the temp file handle and moves the temp file to the final destination
func (a *AtomicWriter) Close() error {
if err := a.tempFile.Close(); err != nil {
return errors.Wrap(err, "unable to close temp file")
return errors.Wrapf(err, "unable to close temp file %s", a.tempFile.Name())
}

if err := os.Rename(a.tempFile.Name(), a.filename); err != nil {
return errors.Wrap(err, "unable to move temp file to destination")
return errors.Wrapf(err, "unable to move temp file %s to destination %s", a.tempFile.Name(), a.filename)
}

return nil
}

// Write writes the buffer to the temp file. You must call Close() to complete the move from temp file to dest file
func (a *AtomicWriter) Write(p []byte) (int, error) {
bs, err := a.tempFile.Write(p)
return bs, errors.Wrap(err, "unable to write to temp file")
Expand Down
4 changes: 3 additions & 1 deletion fs/atmoic_test.go → fs/atomic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ func TestAtomicWriterFileExists(t *testing.T) {
w, err := fs.NewAtomicWriter(file)
require.NoError(t, err, "error creating atomic writer")

// atomic writer should replace existing file with -old suffix
// atomic writer should copy existing file with -old suffix
_, err = os.Stat(file)
require.NoError(t, err, "error stating existing file")
_, err = os.Stat(file + "-old")
require.NoError(t, err, "error stating old file")

Expand Down

0 comments on commit 161a5f8

Please sign in to comment.