Skip to content

Commit 7e442d5

Browse files
rbtrrbtr
authored
backport: fix: add contexts to clean up fsnotify goroutines (#2450)
Signed-off-by: Evan Baker <[email protected]>
1 parent 59c3187 commit 7e442d5

File tree

4 files changed

+56
-50
lines changed

4 files changed

+56
-50
lines changed

‎cns/fsnotify/fsnotify.go‎

Lines changed: 32 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package fsnotify
33
import (
44
"context"
55
"io"
6+
"io/fs"
67
"os"
78
"sync"
89
"time"
@@ -11,6 +12,7 @@ import (
1112
"github.com/fsnotify/fsnotify"
1213
"github.com/pkg/errors"
1314
"go.uber.org/zap"
15+
"golang.org/x/sync/errgroup"
1416
)
1517

1618
type releaseIPsClient interface {
@@ -27,17 +29,18 @@ type watcher struct {
2729
}
2830

2931
// Create the AsyncDelete watcher.
30-
func New(cli releaseIPsClient, path string, logger *zap.Logger) *watcher { //nolint
32+
func New(cli releaseIPsClient, path string, logger *zap.Logger) (*watcher, error) { //nolint
3133
// Add directory where intended deletes are kept
32-
if err := os.Mkdir(path, 0o755); err != nil { //nolint
34+
if err := os.Mkdir(path, 0o755); err != nil && !errors.Is(err, fs.ErrExist) { //nolint
3335
logger.Error("error making directory", zap.String("path", path), zap.Error(err))
36+
return nil, errors.Wrapf(err, "failed to create dir %s", path)
3437
}
3538
return &watcher{
3639
cli: cli,
3740
path: path,
3841
log: logger,
3942
pendingDelete: make(map[string]struct{}),
40-
}
43+
}, nil
4144
}
4245

4346
// releaseAll locks and iterates the pendingDeletes map and calls CNS to
@@ -111,10 +114,29 @@ func (w *watcher) watchFS(ctx context.Context) error {
111114
}
112115
defer watcher.Close()
113116

117+
// Start watching the directory, so that we don't miss any events.
114118
err = watcher.Add(w.path)
115119
if err != nil {
116120
w.log.Error("failed to add path to fsnotify watcher", zap.String("path", w.path), zap.Error(err))
121+
return errors.Wrap(err, "failed to add path to fsnotify watcher")
122+
}
123+
// List the directory and creates synthetic events for any existing items.
124+
w.log.Info("listing directory", zap.String("path", w.path))
125+
dirContents, err := os.ReadDir(w.path)
126+
if err != nil {
127+
w.log.Error("error reading deleteID directory", zap.String("path", w.path), zap.Error(err))
128+
return errors.Wrapf(err, "failed to read %s", w.path)
129+
}
130+
if len(dirContents) == 0 {
131+
w.log.Info("no missed deletes found")
117132
}
133+
w.lock.Lock()
134+
for _, file := range dirContents {
135+
w.log.Info("adding missed delete from file", zap.String("name", file.Name()))
136+
w.pendingDelete[file.Name()] = struct{}{}
137+
}
138+
w.lock.Unlock()
139+
118140
// Start listening for events.
119141
w.log.Info("listening for events from fsnotify watcher")
120142
for {
@@ -139,51 +161,18 @@ func (w *watcher) watchFS(ctx context.Context) error {
139161
}
140162
}
141163

142-
// readFS lists the directory and enqueues any missed deletes that are already
143-
// present on-disk.
144-
func (w *watcher) readFS() error {
145-
w.log.Info("listing directory", zap.String("path", w.path))
146-
dirContents, err := os.ReadDir(w.path)
147-
if err != nil {
148-
w.log.Error("error reading deleteID directory", zap.String("path", w.path), zap.Error(err))
149-
return errors.Wrapf(err, "failed to read %s", w.path)
150-
}
151-
if len(dirContents) == 0 {
152-
w.log.Info("no missed deletes found")
153-
return nil
154-
}
155-
w.lock.Lock()
156-
for _, file := range dirContents {
157-
w.log.Info("adding missed delete from file", zap.String("name", file.Name()))
158-
w.pendingDelete[file.Name()] = struct{}{}
159-
}
160-
w.lock.Unlock()
161-
return nil
162-
}
163-
164-
// WatchFS starts the filesystem watcher to handle async Pod deletes.
164+
// Start starts the filesystem watcher to handle async Pod deletes.
165165
// Blocks until the context is closed; returns underlying fsnotify errors
166166
// if something goes fatally wrong.
167167
func (w *watcher) Start(ctx context.Context) error {
168-
errs := make(chan error)
168+
g, groupCtx := errgroup.WithContext(ctx)
169169
// Start watching for enqueued missed deletes so that we process them as soon as they arrive.
170-
go func(errs chan<- error) {
171-
errs <- w.watchPendingDelete(ctx)
172-
}(errs)
173-
170+
g.Go(func() error { return w.watchPendingDelete(groupCtx) })
174171
// Start watching for changes to the filesystem so that we don't miss any async deletes.
175-
go func(errs chan<- error) {
176-
errs <- w.watchFS(ctx)
177-
}(errs)
178-
179-
// Read the directory to enqueue any missed deletes that are already present on-disk.
180-
if err := w.readFS(); err != nil {
181-
return err
182-
}
183-
184-
// block until one of the goroutines returns an error
185-
err := <-errs
186-
return err
172+
g.Go(func() error { return w.watchFS(groupCtx) })
173+
// the first error from the errgroup will trigger context cancellation for other goroutines in the group.
174+
// this will block until all goroutines complete and return the first error.
175+
return g.Wait() //nolint:wrapcheck // ignore
187176
}
188177

189178
// AddFile creates new file using the containerID as name

‎cns/service/main.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import (
5353
localtls "github.com/Azure/azure-container-networking/server/tls"
5454
"github.com/Azure/azure-container-networking/store"
5555
"github.com/Azure/azure-container-networking/telemetry"
56-
"github.com/avast/retry-go/v3"
56+
"github.com/avast/retry-go/v4"
5757
"github.com/pkg/errors"
5858
"go.uber.org/zap"
5959
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -804,15 +804,19 @@ func main() {
804804
z.Error("failed to create cnsclient", zap.Error(err))
805805
}
806806
go func() {
807-
for {
807+
_ = retry.Do(func() error {
808808
z.Info("starting fsnotify watcher to process missed Pod deletes")
809-
w := fsnotify.New(cnsclient, cnsconfig.AsyncPodDeletePath, z)
809+
w, err := fsnotify.New(cnsclient, cnsconfig.AsyncPodDeletePath, z)
810+
if err != nil {
811+
z.Error("failed to create fsnotify watcher", zap.Error(err))
812+
return errors.Wrap(err, "failed to create fsnotify watcher, will retry")
813+
}
810814
if err := w.Start(rootCtx); err != nil {
811815
z.Error("failed to start fsnotify watcher, will retry", zap.Error(err))
812-
time.Sleep(time.Minute)
813-
continue
816+
return errors.Wrap(err, "failed to start fsnotify watcher, will retry")
814817
}
815-
}
818+
return nil
819+
}, retry.DelayType(retry.BackOffDelay), retry.Attempts(0), retry.Context(rootCtx)) // infinite cancellable exponential backoff retrier
816820
}()
817821
}
818822

‎go.mod‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/Azure/azure-container-networking
22

3-
go 1.20
3+
go 1.21
44

55
require (
66
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.1.3
@@ -34,6 +34,7 @@ require (
3434
github.com/stretchr/testify v1.8.2
3535
go.uber.org/zap v1.24.0
3636
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1
37+
golang.org/x/sync v0.1.0
3738
golang.org/x/sys v0.13.0
3839
google.golang.org/grpc v1.52.0
3940
google.golang.org/protobuf v1.28.1

‎go.sum‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,15 @@ github.com/alexflint/go-filemutex v0.0.0-20171022225611-72bdc8eae2ae/go.mod h1:C
8686
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
8787
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
8888
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
89+
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
8990
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
9091
github.com/avast/retry-go/v3 v3.1.1 h1:49Scxf4v8PmiQ/nY0aY3p0hDueqSmc7++cBbtiDGu2g=
9192
github.com/avast/retry-go/v3 v3.1.1/go.mod h1:6cXRK369RpzFL3UQGqIUp9Q7GDrams+KsYWrfNA1/nQ=
9293
github.com/avast/retry-go/v4 v4.3.4 h1:pHLkL7jvCvP317I8Ge+Km2Yhntv3SdkJm7uekkqbKhM=
9394
github.com/avast/retry-go/v4 v4.3.4/go.mod h1:rv+Nla6Vk3/ilU0H51VHddWHiwimzX66yZ0JT6T+UvE=
9495
github.com/aws/aws-sdk-go v1.15.11/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0=
9596
github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
97+
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
9698
github.com/beorn7/perks v0.0.0-20160804104726-4c0e84591b9a/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
9799
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
98100
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
@@ -246,6 +248,7 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm
246248
github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
247249
github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E=
248250
github.com/dnaeon/go-vcr v1.1.0 h1:ReYa/UBrRyQdant9B4fNHGoCNKw6qh6P0fsdGmZpR7c=
251+
github.com/dnaeon/go-vcr v1.1.0/go.mod h1:M7tiix8f0r6mKKJ3Yq/kqU1OYf3MnfmBWVbPx/yU9ko=
249252
github.com/docker/cli v0.0.0-20191017083524-a8ff7f821017/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8=
250253
github.com/docker/distribution v0.0.0-20190905152932-14b96e55d84c/go.mod h1:0+TTO4EOBfRPhZXAeF1Vu+W3hHZ8eLp8PgKVZlcvtFY=
251254
github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
@@ -291,6 +294,7 @@ github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5Kwzbycv
291294
github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
292295
github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k=
293296
github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE=
297+
github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps=
294298
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
295299
github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
296300
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
@@ -317,6 +321,7 @@ github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbV
317321
github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0=
318322
github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
319323
github.com/go-logr/zapr v1.2.3 h1:a9vnzlIBPQBBkeaR9IuMUfmVOrQlkoC4YfPoFkX3T7A=
324+
github.com/go-logr/zapr v1.2.3/go.mod h1:eIauM6P8qSvTw5o2ez6UEAfGjQKrxQTl5EoK+Qa2oG4=
320325
github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+35s3my2LFTysnkMfxsJBAMHj/DoqoB9knIWoYG/Vk0=
321326
github.com/go-openapi/jsonpointer v0.19.2/go.mod h1:3akKfEdA7DF1sugOqz1dVQHBcuDBPKZGEoHC/NkiQRg=
322327
github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg=
@@ -355,6 +360,7 @@ github.com/golang-jwt/jwt v3.2.1+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzq
355360
github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY=
356361
github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
357362
github.com/golang-jwt/jwt/v4 v4.2.0 h1:besgBTC8w8HjP6NzQdxwKH9Z5oQMZ24ThTrHp3cZ8eU=
363+
github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
358364
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
359365
github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
360366
github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
@@ -513,6 +519,7 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN
513519
github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
514520
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
515521
github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
522+
github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
516523
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
517524
github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA=
518525
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
@@ -596,6 +603,7 @@ github.com/onsi/ginkgo v1.12.0 h1:Iw5WCbBcaAAd0fpRb1c9r5YCylv4XDoCSigm1zLevwU=
596603
github.com/onsi/ginkgo v1.12.0/go.mod h1:oUhWkIvk5aDxtKvDDuw8gItl8pKl42LzjC9KZE0HfGg=
597604
github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
598605
github.com/onsi/ginkgo/v2 v2.6.0 h1:9t9b9vRUbFq3C4qKFCGkVuq/fIHji802N1nrtkh1mNc=
606+
github.com/onsi/ginkgo/v2 v2.6.0/go.mod h1:63DOGlLAH8+REH8jUGdL3YpCpu7JODesutUjdENfUAc=
599607
github.com/onsi/gomega v1.10.0 h1:Gwkk+PTu/nfOwNMtUB/mRUv0X7ewW5dO4AERT1ThVKo=
600608
github.com/onsi/gomega v1.10.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA=
601609
github.com/opencontainers/go-digest v0.0.0-20170106003457-a6d0ee40d420/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
@@ -683,6 +691,7 @@ github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6So
683691
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
684692
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
685693
github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
694+
github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
686695
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
687696
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
688697
github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4=
@@ -811,6 +820,7 @@ go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
811820
go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
812821
go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
813822
go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk=
823+
go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo=
814824
go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
815825
go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8=
816826
go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak=
@@ -943,6 +953,8 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ
943953
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
944954
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
945955
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
956+
golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
957+
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
946958
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
947959
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
948960
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

0 commit comments

Comments
 (0)