Skip to content

Commit

Permalink
Ensure tombstones created before kubexit started are read
Browse files Browse the repository at this point in the history
This commit changes the `Watch` function inside the `tombstone` package to
also emit an fake 'Created' event besides the real `fsnotify` events. This
initial event is send out immediatly when `Watch` is called and the watcher
has been setup.

This change allows kubexit to detect tombstones written before kubexit was
started. This prevents possible race conditions as described by karlkfi#8.

This change on its own introduces a new bug where the tombstone is written
as part of an initial event, but the child process will still start because
`child.Start()` is being called after the watcher has been setup. To overcome
this issue, the shutdown state of the child is tracked in a new flag, which is
set if `ShutdownNow()` or `ShutdownWithTimeout()` is executed.
  • Loading branch information
RemcodM committed Aug 10, 2020
1 parent 4e1c0c9 commit 088968a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/supervisor/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Supervisor struct {
cmd *exec.Cmd
sigCh chan os.Signal
startStopLock sync.Mutex
shutdown bool
shutdownTimer *time.Timer
}

Expand All @@ -32,13 +33,18 @@ func New(name string, args ...string) *Supervisor {
cmd.Env = os.Environ()
return &Supervisor{
cmd: cmd,
shutdown: false,
}
}

func (s *Supervisor) Start() error {
s.startStopLock.Lock()
defer s.startStopLock.Unlock()

if s.shutdown {
return errors.New("not starting child process: shutdown already started")
}

log.Printf("Starting: %s\n", s)
if err := s.cmd.Start(); err != nil {
return fmt.Errorf("failed to start child process: %v", err)
Expand Down Expand Up @@ -90,6 +96,8 @@ func (s *Supervisor) ShutdownNow() error {
s.startStopLock.Lock()
defer s.startStopLock.Unlock()

s.shutdown = true

if !s.isRunning() {
log.Println("Skipping ShutdownNow: child process not running")
return nil
Expand All @@ -109,6 +117,8 @@ func (s *Supervisor) ShutdownWithTimeout(timeout time.Duration) error {
s.startStopLock.Lock()
defer s.startStopLock.Unlock()

s.shutdown = true

if !s.isRunning() {
log.Println("Skipping ShutdownWithTimeout: child process not running")
return nil
Expand Down
14 changes: 14 additions & 0 deletions pkg/tombstone/tombstone.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,19 @@ func Watch(ctx context.Context, graveyard string, eventHandler EventHandler) err
if err != nil {
return fmt.Errorf("failed to add watcher: %v", err)
}

files, err := ioutil.ReadDir(graveyard)
if err != nil {
return fmt.Errorf("failed to read graveyard dir: %v", err)
}

for _, f := range files {
event := fsnotify.Event{
Name: filepath.Join(graveyard, f.Name()),
Op: fsnotify.Create,
}
eventHandler(event)
}

return nil
}

0 comments on commit 088968a

Please sign in to comment.