cephfs: avoid hanging os.Stat() in NodeGetVolumeStats after restart

If a node-plugin pod restarts while the backend (e.g. MDS) is down,
NodeGetVolumeStats used to call os.Stat() on the target path
immediately after (re)starting the health checker, before the
checker had a chance to detect the outage. Since os.Stat() blocks
in the kernel on an unresponsive mount, this held the VolumeLock
forever and made every later call for that path fail with Aborted.

Return early with an "not yet available" condition instead of
calling os.Stat() when the checker was just (re)started, for
CephFS.

Signed-off-by: SanjalKatiyar <sanjaldhir@gmail.com>

Author: SanjalKatiyar

internal/cephfs/nodeserver.go

@@ -941,19 +941,38 @@ func (ns *cephfsNodeServer) NodeGetVolumeStats(
 	// health check first, return without stats if unhealthy
 	healthy, msg := ns.healthChecker.IsHealthy(volumeID, targetPath)
 
-	// If healthy and an error is returned, it means that the checker was not
-	// started. This could happen when the node-plugin was restarted and the
+	// If healthy and an error is returned, it either means that the checker
+	// was not started or it is running but has not completed its
+	// first health-check cycle yet.
+	// This could happen when the node-plugin was restarted and the
 	// volume is already staged and published.
 	if healthy && msg != nil {
 		// Start a StatChecker for the mounted targetPath, this prevents
 		// writing a file in the user-visible location. Ideally a (shared)
 		// FileChecker is started with the stagingTargetPath, but we can't
 		// get the stagingPath from the request easily.
 		// TODO: resolve the stagingPath like rbd.getStagingPath() does
+		// NOTE: rbd.getStagingPath() uses os.Stat() internally which
+		// if called synchronously, could block indefinitely.
+
+		// Start the background checker but return
+		// immediately instead of calling os.Stat() on this goroutine.
+		// If the mount is unresponsive, os.Stat() would block,
+		// holding the VolumeLock (acquired above) and preventing all future
+		// calls for this path from reaching isHealthy().
+		// The background checker will do the stat(), the next periodic
+		// call will pick up the result (or detect the timeout).
 		err = ns.healthChecker.StartChecker(req.GetVolumeId(), targetPath, hc.StatCheckerType)
 		if err != nil {
 			log.WarningLog(ctx, "failed to start healthchecker: %v", err)
 		}
+
+		return &csi.NodeGetVolumeStatsResponse{
+			VolumeCondition: &csi.VolumeCondition{
+				Abnormal: false,
+				Message:  "health checker started, status not yet available",
+			},
+		}, nil
 	}
 
 	// !healthy indicates a problem with the volume
@@ -966,7 +985,8 @@ func (ns *cephfsNodeServer) NodeGetVolumeStats(
 		}, nil
 	}
 
-	// warning: stat() may hang on an unhealthy volume
+	// warning: reaching here should mean that synchronous os.Stat()
+	// call is safe and will not indefinitely block/hang
 	stat, err := os.Stat(targetPath)
 	if err != nil {
 		if util.IsCorruptedMountError(err) {

internal/health-checker/checker.go

@@ -37,8 +37,8 @@ type checker struct {
 	// timeout contains the delay (interval + timeout)
 	timeout time.Duration
 
-	// mutex protects against concurrent access to healthy, err and
-	// lastUpdate
+	// mutex protects against concurrent access to healthy, err,
+	// lastUpdate and checked
 	mutex *sync.RWMutex
 
 	// current status
@@ -47,6 +47,12 @@ type checker struct {
 	err        error
 	lastUpdate time.Time
 
+	// checked is set to true once runChecker() has completed its first
+	// health-check cycle (successful or not). This matters for
+	// checker (re)start scenarios, e.g. following a node-plugin restart
+	// where the volume was already mounted.
+	checked bool
+
 	// commands is the channel to read commands from; when to stop.
 	commands chan command
 
@@ -60,6 +66,7 @@ func (c *checker) initDefaults() {
 	c.isRunning = false
 	c.err = nil
 	c.healthy = true
+	c.checked = false
 	c.lastUpdate = time.Now()
 	c.commands = make(chan command)
 
@@ -91,12 +98,25 @@ func (c *checker) isHealthy() (bool, error) {
 	// It is required to check, in case the write or read in the go routine
 	// is blocked.
 
-	delay := time.Since(c.lastUpdate)
-	if delay > (c.interval + c.timeout) {
+	c.mutex.RLock()
+	checked := c.checked
+	lastUpdate := c.lastUpdate
+	c.mutex.RUnlock()
+
+	delay := time.Since(lastUpdate)
+	switch {
+	case delay > (c.interval + c.timeout):
 		c.mutex.Lock()
 		c.healthy = false
 		c.err = fmt.Errorf("health-check has not responded for %f seconds", delay.Seconds())
 		c.mutex.Unlock()
+	case !checked:
+		// The first health-check cycle has not completed yet.
+		// Report this explicitly so that callers do
+		// not mistake this for a confirmed healthy volume and fall
+		// through to a fallback that could itself block (e.g. a
+		// synchronous os.Stat() on an unresponsive mount).
+		return true, fmt.Errorf("health-check has not completed its first check yet")
 	}
 
 	// read lock to get consistency between the return values

internal/health-checker/checker_test.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2026 ceph-csi authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package healthchecker
+
+import (
+	"testing"
+	"time"
+)
+
+func TestCheckerNotYetChecked(t *testing.T) {
+	t.Parallel()
+
+	c := &checker{}
+	c.initDefaults()
+
+	healthy, err := c.isHealthy()
+	if !healthy || err == nil {
+		t.Errorf("expected (true, error) for a checker that has not completed its first check, got (%t, %v)",
+			healthy, err)
+	}
+}
+
+func TestCheckerStuckFirstCheckEventuallyTimesOut(t *testing.T) {
+	t.Parallel()
+
+	c := &checker{}
+	c.initDefaults()
+	c.interval = time.Millisecond
+	c.timeout = time.Millisecond
+
+	c.mutex.Lock()
+	c.lastUpdate = time.Now().Add(-time.Hour)
+	c.mutex.Unlock()
+
+	healthy, err := c.isHealthy()
+	if healthy || err == nil {
+		t.Errorf("expected (false, error) for a checker whose first check is stuck and overdue, got (%t, %v)",
+			healthy, err)
+	}
+}
+
+func TestCheckerConfirmedHealthyStillExpires(t *testing.T) {
+	t.Parallel()
+
+	c := &checker{}
+	c.initDefaults()
+	c.interval = time.Millisecond
+	c.timeout = time.Millisecond
+
+	c.mutex.Lock()
+	c.checked = true
+	c.healthy = true
+	c.err = nil
+	c.lastUpdate = time.Now().Add(-time.Hour)
+	c.mutex.Unlock()
+
+	healthy, err := c.isHealthy()
+	if healthy || err == nil {
+		t.Errorf("expected (false, error) for a checker that stopped reporting, got (%t, %v)", healthy, err)
+	}
+}

internal/health-checker/filechecker.go

@@ -52,6 +52,7 @@ func newFileChecker(dir string) ConditionChecker {
 				err := fc.writeTimestamp(now)
 				if err != nil {
 					fc.mutex.Lock()
+					fc.checked = true
 					fc.healthy = false
 					fc.err = err
 					fc.mutex.Unlock()
@@ -62,6 +63,7 @@ func newFileChecker(dir string) ConditionChecker {
 				ts, err := fc.readTimestamp()
 				if err != nil {
 					fc.mutex.Lock()
+					fc.checked = true
 					fc.healthy = false
 					fc.err = err
 					fc.mutex.Unlock()
@@ -72,6 +74,7 @@ func newFileChecker(dir string) ConditionChecker {
 				// verify that the written timestamp is read back
 				if now.Compare(ts) != 0 {
 					fc.mutex.Lock()
+					fc.checked = true
 					fc.healthy = false
 					fc.err = errors.New("timestamp read from file does not match what was written")
 					fc.mutex.Unlock()
@@ -81,6 +84,7 @@ func newFileChecker(dir string) ConditionChecker {
 
 				// run health check, write a timestamp to a file, read it back
 				fc.mutex.Lock()
+				fc.checked = true
 				fc.healthy = true
 				fc.err = nil
 				fc.lastUpdate = ts

internal/health-checker/filechecker_test.go

@@ -30,7 +30,7 @@ func TestFileChecker(t *testing.T) {
 	if !ok {
 		t.Errorf("failed to convert fc to *fileChecker: %v", fc)
 	}
-	checker.interval = time.Second * 5
+	checker.interval = time.Second * 3
 
 	// start the checker
 	checker.start()
@@ -41,9 +41,18 @@ func TestFileChecker(t *testing.T) {
 		t.Error("checker failed to start")
 	}
 
+	// before the checker has completed its first check cycle
+	healthy, msg := checker.isHealthy()
+	if !healthy || msg == nil {
+		t.Errorf("expected (true, error) before the first tick, got (%t, %v)", healthy, msg)
+	}
+
+	// wait well past the first tick, so the first check has completed.
+	time.Sleep(checker.interval + time.Second)
+
 	for range 10 {
 		// check health, should be healthy
-		healthy, msg := checker.isHealthy()
+		healthy, msg = checker.isHealthy()
 		if !healthy || msg != nil {
 			t.Error("volume is unhealthy")
 		}

internal/health-checker/statchecker.go

@@ -47,20 +47,18 @@ func newStatChecker(dir string) ConditionChecker {
 
 				return
 			case now := <-ticker.C:
-				_, err := os.Stat(sc.dirname)
-				if err != nil {
-					sc.mutex.Lock()
-					sc.healthy = false
-					sc.err = err
-					sc.mutex.Unlock()
-
-					continue
-				}
+				_, statErr := os.Stat(sc.dirname)
 
 				sc.mutex.Lock()
-				sc.healthy = true
-				sc.err = nil
-				sc.lastUpdate = now
+				sc.checked = true
+				if statErr != nil {
+					sc.healthy = false
+					sc.err = statErr
+				} else {
+					sc.healthy = true
+					sc.err = nil
+					sc.lastUpdate = now
+				}
 				sc.mutex.Unlock()
 			}
 		}

internal/health-checker/statchecker_test.go

@@ -30,7 +30,7 @@ func TestStatChecker(t *testing.T) {
 	if !ok {
 		t.Errorf("failed to convert fc to *fileChecker: %v", sc)
 	}
-	checker.interval = time.Second * 5
+	checker.interval = time.Second * 3
 
 	// start the checker
 	checker.start()
@@ -41,9 +41,18 @@ func TestStatChecker(t *testing.T) {
 		t.Error("checker failed to start")
 	}
 
+	// before the checker has completed its first check cycle
+	healthy, msg := checker.isHealthy()
+	if !healthy || msg == nil {
+		t.Errorf("expected (true, error) before the first tick, got (%t, %v)", healthy, msg)
+	}
+
+	// wait well past the first tick, so the first check has completed.
+	time.Sleep(checker.interval + time.Second)
+
 	for i := range 10 {
 		// check health, should be healthy
-		healthy, msg := checker.isHealthy()
+		healthy, msg = checker.isHealthy()
 		if !healthy || msg != nil {
 			t.Errorf("volume is unhealthy after %d tries", i+1)
 		}