From 26471d57235a14f76487673330ef7b39f609a8ac Mon Sep 17 00:00:00 2001 From: Paul Morie Date: Sat, 27 Feb 2016 16:09:06 -0500 Subject: [PATCH] Remove sentinel file from atomic writer --- pkg/volume/util/atomic_writer.go | 54 +++---------- pkg/volume/util/atomic_writer_test.go | 112 +------------------------- 2 files changed, 13 insertions(+), 153 deletions(-) diff --git a/pkg/volume/util/atomic_writer.go b/pkg/volume/util/atomic_writer.go index 7734cdc0432..24c84822412 100644 --- a/pkg/volume/util/atomic_writer.go +++ b/pkg/volume/util/atomic_writer.go @@ -37,11 +37,7 @@ const ( ) // AtomicWriter handles atomically projecting content for a set of files into -// a target directory. AtomicWriter maintains a sentinel file named -// "..sentinel" in the target directory which is updated after new data is -// projected into the target directory, allowing consumers of the data to -// listen for updates by monitoring the sentinel file with inotify or -// fanotify. +// a target directory. // // Note: // @@ -55,6 +51,10 @@ const ( // data directory symlink are created in the writer's target dir.  This scheme // allows the files to be atomically updated by changing the target of the // data directory symlink. +// +// Consumers of the target directory can monitor the ..data symlink using +// inotify or fanotify to receive events when the content in the volume is +// updated. type AtomicWriter struct { targetDir string logContext string @@ -72,9 +72,8 @@ func NewAtomicWriter(targetDir, logContext string) (*AtomicWriter, error) { } const ( - sentinelFileName = "..sentinel" - dataDirName = "..data" - newDataDirName = "..data_tmp" + dataDirName = "..data" + newDataDirName = "..data_tmp" ) // Write does an atomic projection of the given payload into the writer's target @@ -112,10 +111,8 @@ const ( // 8.  A symlink to the new timestamped directory ..data_tmp is created that will // become the new data directory // 9.  The new data directory symlink is renamed to the data directory; rename is atomic -// 10. The sentinel file modification and access times are updated (file is created if it does not -// already exist) -// 11. Old paths are removed from the user-visible portion of the target directory -// 12.  The previous timestamped directory is removed, if it exists +// 10. Old paths are removed from the user-visible portion of the target directory +// 11.  The previous timestamped directory is removed, if it exists func (w *AtomicWriter) Write(payload map[string][]byte) error { // (1) cleanPayload, err := validatePayload(payload) @@ -189,18 +186,12 @@ func (w *AtomicWriter) Write(payload map[string][]byte) error { } // (10) - if err = w.touchSentinelFile(); err != nil { - glog.Errorf("%s: error touching sentinel file: %v", w.logContext, err) - return err - } - - // (11) if err = w.removeUserVisiblePaths(pathsToRemove); err != nil { glog.Errorf("%s: error removing old visible symlinks: %v", w.logContext, err) return err } - // (12) + // (11) if len(oldTsDir) > 0 { if err = os.RemoveAll(path.Join(w.targetDir, oldTsDir)); err != nil { glog.Errorf("%s: error removing old data directory %s: %v", w.logContext, oldTsDir, err) @@ -434,28 +425,3 @@ func (w *AtomicWriter) removeUserVisiblePaths(paths sets.String) error { return nil } - -// touchSentinelFile touches the sentinel file or creates it if it doesn't exist. -func (w *AtomicWriter) touchSentinelFile() error { - sentinelFilePath := path.Join(w.targetDir, sentinelFileName) - _, err := os.Stat(sentinelFilePath) - if err != nil && os.IsNotExist(err) { - file, err := os.Create(sentinelFilePath) - if err != nil { - glog.Errorf("%s: unexpected error creating sentinel file %s: %v", w.logContext, sentinelFilePath, err) - return err - } - file.Close() - } else if err != nil { - return err - } - - ts := time.Now() - err = os.Chtimes(sentinelFilePath, ts, ts) - if err != nil { - glog.Errorf("%s: error updating sentinel file mod time: %v", w.logContext, err) - return err - } - - return nil -} diff --git a/pkg/volume/util/atomic_writer_test.go b/pkg/volume/util/atomic_writer_test.go index 2fba4d7bb39..ab741ebe5ab 100644 --- a/pkg/volume/util/atomic_writer_test.go +++ b/pkg/volume/util/atomic_writer_test.go @@ -27,7 +27,6 @@ import ( "reflect" "strings" "testing" - "time" "k8s.io/kubernetes/pkg/util/sets" utiltesting "k8s.io/kubernetes/pkg/util/testing" @@ -371,7 +370,6 @@ IAAAAAAAsDyZDwU=` } checkVolumeContents(targetDir, tc.name, tc.payload, t) - checkSentinelFile(targetDir, t) } } @@ -547,8 +545,6 @@ func TestUpdate(t *testing.T) { continue } - oldTs := checkSentinelFile(targetDir, t) - err = writer.Write(tc.next) if err != nil { if tc.shouldWrite { @@ -561,19 +557,13 @@ func TestUpdate(t *testing.T) { } checkVolumeContents(targetDir, tc.name, tc.next, t) - - ts := checkSentinelFile(targetDir, t) - if !ts.After(oldTs) { - t.Errorf("Unexpected timestamp on sentinel file; expected %v to be after %v", ts, oldTs) - } } } func TestMultipleUpdates(t *testing.T) { cases := []struct { - name string - payloads []map[string][]byte - clearSentinel bool + name string + payloads []map[string][]byte }{ { name: "update 1", @@ -625,7 +615,6 @@ func TestMultipleUpdates(t *testing.T) { "bar": []byte("bar4"), }, }, - clearSentinel: true, }, { name: "subdirectories 2", @@ -710,84 +699,12 @@ func TestMultipleUpdates(t *testing.T) { continue } - var oldTs *time.Time = nil writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"} - for ii, payload := range tc.payloads { + for _, payload := range tc.payloads { writer.Write(payload) checkVolumeContents(targetDir, tc.name, payload, t) - ts := checkSentinelFile(targetDir, t) - - if oldTs != nil && !ts.After(*oldTs) { - t.Errorf("%v[%v] unexpected timestamp on sentinel file; expected %v to be after %v", tc.name, ii, ts, oldTs) - } - oldTs = &ts - - if tc.clearSentinel { - clearSentinelFile(targetDir, t) - } - } - } -} - -func TestSentinelFileModTimeIncreasing(t *testing.T) { - cases := []struct { - name string - iterations int - clearSentinelFile bool - }{ - { - name: "5 iters", - iterations: 5, - }, - { - name: "50 iters", - iterations: 50, - }, - { - name: "1000 iters", - iterations: 1000, - }, - { - name: "1000 clear sentinel", - iterations: 1000, - clearSentinelFile: true, - }, - { - name: "10000 clear sentinel", - iterations: 10000, - clearSentinelFile: true, - }, - } - - for _, tc := range cases { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) - continue - } - - var oldTs *time.Time = nil - writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"} - - for i := 0; i < tc.iterations; i++ { - err = writer.touchSentinelFile() - if err != nil { - t.Errorf("%v: unexpected error touching sentinel file: %v", tc.name, err) - continue - } - - ts := checkSentinelFile(targetDir, t) - if oldTs != nil && !ts.After(*oldTs) { - t.Errorf("%v: unexpected timestamp on sentinel file; expected %v to be after %v", tc.name, ts, oldTs) - continue - } - oldTs = &ts - - if tc.clearSentinelFile { - clearSentinelFile(targetDir, t) - } } } } @@ -829,26 +746,3 @@ func checkVolumeContents(targetDir, tcName string, payload map[string][]byte, t t.Errorf("%v: payload and observed payload do not match.", tcName) } } - -func checkSentinelFile(targetDir string, t *testing.T) time.Time { - sentinelFilePath := filepath.Join(targetDir, sentinelFileName) - info, err := os.Stat(sentinelFilePath) - if err != nil { - t.Errorf("Couldn't stat sentinel file for dir %v: %v", targetDir, err) - return time.Now() - } - - return info.ModTime() -} - -func clearSentinelFile(targetDir string, t *testing.T) { - sentinelFilePath := filepath.Join(targetDir, sentinelFileName) - _, err := os.Stat(sentinelFilePath) - if err != nil { - t.Errorf("Couldn't stat sentinel file for dir %v: %v", targetDir, err) - } - err = os.Remove(sentinelFilePath) - if err != nil { - t.Errorf("Error removing sentinel file: %v", err) - } -}