mirror of
https://github.com/juanfont/headscale.git
synced 2026-09-09 18:21:32 +09:00
dns: cancel extra-records retry on shutdown and close watcher on setup error
After Remove/Rename, the extra-records filewatcher retried with context.Background and the default 15-minute backoff budget, so Close could not stop Run. Cancel that retry when closeCh closes. If the file is still missing after the budget, watch the parent directory so a later recreate is seen. Close the fsnotify watcher on NewExtraRecordsManager error paths after NewWatcher succeeds. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
This commit is contained in:
committed by
Kristoffer Dalby
parent
c26b2fd0a4
commit
a48a42baf4
@@ -45,6 +45,7 @@ keys remain all-access.
|
||||
- Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324)
|
||||
- Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341)
|
||||
- Headscale now requires Go 1.27 to build
|
||||
- Fix extra-records filewatcher hanging on shutdown after the watched file is deleted, and leaking the watcher when setup fails
|
||||
|
||||
## 0.29.3 (2026-07-29)
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
|
||||
"github.com/cenkalti/backoff/v5"
|
||||
@@ -37,17 +38,24 @@ func NewExtraRecordsManager(path string) (*ExtraRecordsMan, error) {
|
||||
return nil, fmt.Errorf("creating watcher: %w", err)
|
||||
}
|
||||
|
||||
closeWatcher := func() {
|
||||
_ = watcher.Close()
|
||||
}
|
||||
|
||||
fi, err := os.Stat(path)
|
||||
if err != nil {
|
||||
closeWatcher()
|
||||
return nil, fmt.Errorf("getting file info: %w", err)
|
||||
}
|
||||
|
||||
if fi.IsDir() {
|
||||
closeWatcher()
|
||||
return nil, fmt.Errorf("%w: %s", ErrPathIsDirectory, path)
|
||||
}
|
||||
|
||||
records, hash, err := readExtraRecordsFromPath(path)
|
||||
if err != nil {
|
||||
closeWatcher()
|
||||
return nil, fmt.Errorf("reading extra records from path: %w", err)
|
||||
}
|
||||
|
||||
@@ -62,6 +70,8 @@ func NewExtraRecordsManager(path string) (*ExtraRecordsMan, error) {
|
||||
|
||||
err = watcher.Add(path)
|
||||
if err != nil {
|
||||
closeWatcher()
|
||||
|
||||
return nil, fmt.Errorf("adding path to watcher: %w", err)
|
||||
}
|
||||
|
||||
@@ -101,15 +111,21 @@ func (e *ExtraRecordsMan) Run() {
|
||||
// If a file is removed or renamed, fsnotify will lose track of it
|
||||
// and not watch it. We will therefore attempt to re-add it with a backoff.
|
||||
case fsnotify.Remove, fsnotify.Rename:
|
||||
_, err := backoff.Retry(context.Background(), func() (struct{}, error) {
|
||||
if _, err := os.Stat(e.path); err != nil { //nolint:noinlineerr
|
||||
return struct{}{}, err
|
||||
err := e.waitUntilPathExists()
|
||||
if err != nil {
|
||||
select {
|
||||
case <-e.closeCh:
|
||||
return
|
||||
default:
|
||||
}
|
||||
|
||||
return struct{}{}, nil
|
||||
}, backoff.WithBackOff(backoff.NewExponentialBackOff()))
|
||||
if err != nil {
|
||||
log.Error().Caller().Err(err).Msgf("extra records filewatcher retrying to find file after delete")
|
||||
|
||||
addErr := e.watcher.Add(filepath.Dir(e.path))
|
||||
if addErr != nil {
|
||||
log.Error().Caller().Err(addErr).Msgf("extra records filewatcher watching parent after delete failed")
|
||||
}
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -139,6 +155,29 @@ func (e *ExtraRecordsMan) Close() {
|
||||
close(e.closeCh)
|
||||
}
|
||||
|
||||
func (e *ExtraRecordsMan) waitUntilPathExists() error {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
go func() {
|
||||
select {
|
||||
case <-e.closeCh:
|
||||
cancel()
|
||||
case <-ctx.Done():
|
||||
}
|
||||
}()
|
||||
|
||||
_, err := backoff.Retry(ctx, func() (struct{}, error) {
|
||||
if _, err := os.Stat(e.path); err != nil { //nolint:noinlineerr
|
||||
return struct{}{}, err
|
||||
}
|
||||
|
||||
return struct{}{}, nil
|
||||
}, backoff.WithBackOff(backoff.NewExponentialBackOff()))
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
func (e *ExtraRecordsMan) UpdateCh() <-chan []tailcfg.DNSRecord {
|
||||
return e.updateCh
|
||||
}
|
||||
|
||||
@@ -43,3 +43,32 @@ func TestUpdateRecordsDoesNotBlockShutdown(t *testing.T) {
|
||||
t.Fatal("updateRecords parked on a blocking send and did not return after Close")
|
||||
}
|
||||
}
|
||||
|
||||
// TestWaitUntilPathExistsReturnsOnClose ensures Close unblocks the
|
||||
// Remove/Rename retry. A missing extra-records file must not pin Run
|
||||
// for the 15-minute backoff budget after shutdown.
|
||||
func TestWaitUntilPathExistsReturnsOnClose(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "extra.json")
|
||||
require.NoError(t, os.WriteFile(path,
|
||||
[]byte(`[{"name":"a.example.com","type":"A","value":"100.64.0.1"}]`), 0o600))
|
||||
|
||||
er, err := NewExtraRecordsManager(path)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.NoError(t, os.Remove(path))
|
||||
|
||||
done := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
done <- er.waitUntilPathExists()
|
||||
}()
|
||||
|
||||
er.Close()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
require.Error(t, err)
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("waitUntilPathExists did not return after Close while the extra-records file was missing")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user