Skip to content

Commit

Permalink
OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)
Browse files Browse the repository at this point in the history
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
  • Loading branch information
jwendell authored and jewertow committed Aug 24, 2022
1 parent f639909 commit 1ea2cce
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cni/pkg/install/cniconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func getCNIConfigFilepath(ctx context.Context, cfg pluginConfig) (string, error)
return filepath.Join(cfg.mountedCNINetDir, filename), nil
}

watcher, fileModified, errChan, err := util.CreateFileWatcher(cfg.mountedCNINetDir)
watcher, fileModified, errChan, err := util.CreateFileWatcher("", []string{cfg.mountedCNINetDir})
if err != nil {
return "", err
}
Expand Down
2 changes: 1 addition & 1 deletion cni/pkg/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func readServiceAccountToken() (string, error) {
func sleepCheckInstall(ctx context.Context, cfg *config.InstallConfig, cniConfigFilepath string, isReady *atomic.Value) error {
// Create file watcher before checking for installation
// so that no file modifications are missed while and after checking
watcher, fileModified, errChan, err := util.CreateFileWatcher(append(cfg.CNIBinTargetDirs, cfg.MountedCNINetDir)...)
watcher, fileModified, errChan, err := util.CreateFileWatcher(cfg.CNIBinariesPrefix, append(cfg.CNIBinTargetDirs, cfg.MountedCNINetDir))
if err != nil {
return err
}
Expand Down
21 changes: 15 additions & 6 deletions cni/pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,30 @@ import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/fsnotify/fsnotify"

"istio.io/istio/pkg/file"
)

type watchPrefix struct {
watcher *fsnotify.Watcher
prefix string
}

// Creates a file watcher that watches for any changes to the directory
func CreateFileWatcher(dirs ...string) (watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error, err error) {
// If `prefix` is non-empty, it will only notify of changes for files whose filename starts with `prefix`
func CreateFileWatcher(prefix string, dirs []string) (watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error, err error) {
watcher, err = fsnotify.NewWatcher()
if err != nil {
return
}

fileModified, errChan = make(chan bool), make(chan error)
go watchFiles(watcher, fileModified, errChan)
wp := &watchPrefix{watcher: watcher, prefix: prefix}
go watchFiles(wp, fileModified, errChan)

for _, dir := range dirs {
if file.IsDirWriteable(dir) != nil {
Expand All @@ -50,17 +59,17 @@ func CreateFileWatcher(dirs ...string) (watcher *fsnotify.Watcher, fileModified
return
}

func watchFiles(watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error) {
func watchFiles(wp *watchPrefix, fileModified chan bool, errChan chan error) {
for {
select {
case event, ok := <-watcher.Events:
case event, ok := <-wp.watcher.Events:
if !ok {
return
}
if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 {
if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 && strings.HasPrefix(filepath.Base(event.Name), wp.prefix) {
fileModified <- true
}
case err, ok := <-watcher.Errors:
case err, ok := <-wp.watcher.Errors:
if !ok {
return
}
Expand Down

0 comments on commit 1ea2cce

Please sign in to comment.