diff --git a/pkg/kubelet/kubeletconfig/util/files/files.go b/pkg/kubelet/kubeletconfig/util/files/files.go index 62d96fa5ad..a732bbcce8 100644 --- a/pkg/kubelet/kubeletconfig/util/files/files.go +++ b/pkg/kubelet/kubeletconfig/util/files/files.go @@ -63,34 +63,51 @@ func EnsureFile(fs utilfs.Filesystem, path string) error { return file.Close() } -// ReplaceFile replaces the contents of the file at `path` with `data` by writing to a tmp file in the same -// dir as `path` and renaming the tmp file over `path`. The file does not have to exist to use ReplaceFile. -func ReplaceFile(fs utilfs.Filesystem, path string, data []byte) error { +// WriteTmpFile creates a temporary file at `path`, writes `data` into it, and fsyncs the file +func WriteTmpFile(fs utilfs.Filesystem, path string, data []byte) (tmpPath string, retErr error) { dir := filepath.Dir(path) prefix := filepath.Base(path) // create the tmp file tmpFile, err := fs.TempFile(dir, prefix) if err != nil { - return err + return "", err } + defer func() { + // close the file, return the close error only if there haven't been any other errors + if err := tmpFile.Close(); retErr == nil { + retErr = err + } + // if there was an error writing, syncing, or closing, delete the temporary file and return the error + if retErr != nil { + if err := fs.Remove(tmpPath); err != nil { + retErr = fmt.Errorf("attempted to remove temporary file %q after error %v, but failed due to error: %v", path, retErr, err) + } + tmpPath = "" + } + }() // Name() will be an absolute path when using utilfs.DefaultFS, because ioutil.TempFile passes // an absolute path to os.Open, and we ensure similar behavior in utilfs.FakeFS for testing. - tmpPath := tmpFile.Name() + tmpPath = tmpFile.Name() // write data if _, err := tmpFile.Write(data); err != nil { - return err + return tmpPath, err } // sync file, to ensure it's written in case a hard reset happens - if err := tmpFile.Sync(); err != nil { - return err - } - if err := tmpFile.Close(); err != nil { - return err - } + return tmpPath, tmpFile.Sync() +} +// ReplaceFile replaces the contents of the file at `path` with `data` by writing to a tmp file in the same +// dir as `path` and renaming the tmp file over `path`. The file does not have to exist to use ReplaceFile. +// Note ReplaceFile calls fsync. +func ReplaceFile(fs utilfs.Filesystem, path string, data []byte) error { + // write data to a temporary file + tmpPath, err := WriteTmpFile(fs, path, data) + if err != nil { + return err + } // rename over existing file if err := fs.Rename(tmpPath, path); err != nil { return err diff --git a/pkg/util/filesystem/defaultfs.go b/pkg/util/filesystem/defaultfs.go index ea9d73f139..f87fe3d6df 100644 --- a/pkg/util/filesystem/defaultfs.go +++ b/pkg/util/filesystem/defaultfs.go @@ -62,6 +62,11 @@ func (DefaultFs) RemoveAll(path string) error { return os.RemoveAll(path) } +// Remove via os.RemoveAll +func (DefaultFs) Remove(name string) error { + return os.Remove(name) +} + // ReadFile via ioutil.ReadFile func (DefaultFs) ReadFile(filename string) ([]byte, error) { return ioutil.ReadFile(filename) diff --git a/pkg/util/filesystem/fakefs.go b/pkg/util/filesystem/fakefs.go index 28bbce8067..b103007a55 100644 --- a/pkg/util/filesystem/fakefs.go +++ b/pkg/util/filesystem/fakefs.go @@ -92,6 +92,11 @@ func (fs *fakeFs) RemoveAll(path string) error { return fs.a.RemoveAll(path) } +// Remove via afero.RemoveAll +func (fs *fakeFs) Remove(name string) error { + return fs.a.Remove(name) +} + // fakeFile implements File; for use with fakeFs type fakeFile struct { file afero.File diff --git a/pkg/util/filesystem/filesystem.go b/pkg/util/filesystem/filesystem.go index fcfef1d1f9..16e329b30f 100644 --- a/pkg/util/filesystem/filesystem.go +++ b/pkg/util/filesystem/filesystem.go @@ -31,6 +31,7 @@ type Filesystem interface { MkdirAll(path string, perm os.FileMode) error Chtimes(name string, atime time.Time, mtime time.Time) error RemoveAll(path string) error + Remove(name string) error // from "io/ioutil" ReadFile(filename string) ([]byte, error)