Skip to content

Commit

Permalink
Fix excessive io around volume cleanup, move cleanup logic to startup…
Browse files Browse the repository at this point in the history
… rather than run each volume deprovision

Signed-off-by: Cameron McAvoy <[email protected]>
  • Loading branch information
cnmcavoy committed Aug 19, 2024
1 parent a5b4907 commit 506dd87
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 7 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.4.1] - 2024-08-19
### Fixed
- Fixed volume cleanup to only run at startup, reducing io when many mounts are used

## [0.4.0] - 2024-08-07
### Fixed
- Fixed locking behavior with volumes, terminating pods could fail to be cleaned up due to a race in the locks.
Expand Down
12 changes: 7 additions & 5 deletions pkg/ccm/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@ import (
"k8s.io/mount-utils"
)

func doCleanup(configMap, volumeID, targetPath string) {
func doCleanup() {
start := time.Now()
if err := cleanupDataDir(); err != nil {
logger.Error(err, fmt.Sprintf("failed to cleanup on node unpublish volume for volume id %q and target path %q", volumeID, targetPath))
unpublishErr.WithLabelValues(configMap, "failed to clean up volume data").Inc()
logger.Error(err, "failed to cleanup")
}
if err := cleanupMetadataDir(); err != nil {
logger.Error(err, fmt.Sprintf("failed to cleanup metadata on node unpublish volume for volume id %q and target path %q", volumeID, targetPath))
unpublishErr.WithLabelValues(configMap, "failed to clean up volume metadata").Inc()
logger.Error(err, "failed to cleanup metadata")
}
cleanupTime.WithLabelValues().Observe(time.Since(start).Seconds())
}
Expand All @@ -32,6 +30,10 @@ func cleanupDataDir() error {
dataDir := path.Join(storageDir, "data")
dirEntries, err := os.ReadDir(dataDir)
if err != nil {
if os.IsNotExist(err) {
logger.Info("[cleanup] no data dir, skipping cleanup")
return nil
}
return fmt.Errorf("failed to list dir entries for %q: %w", dataDir, err)
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/ccm/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"sync"

"github.com/container-storage-interface/spec/lib/go/csi"

"google.golang.org/grpc"

"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -100,6 +99,8 @@ func (d *driver) Run() error {
return resp, err
}

doCleanup()

d.srv = grpc.NewServer(grpc.UnaryInterceptor(errHandler))
csi.RegisterIdentityServer(d.srv, d)
csi.RegisterNodeServer(d.srv, d)
Expand Down
1 change: 0 additions & 1 deletion pkg/ccm/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ func (d *driver) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublish
unpublishErr.WithLabelValues(configMap, "volume was already unmounted").Inc()
}
logger.V(2).Info(fmt.Sprintf("node unpublish volume succeeded for volume id %q target path %q", req.VolumeId, req.TargetPath))
doCleanup(configMap, req.VolumeId, req.TargetPath)
unpublish.WithLabelValues(configMap).Inc()
unpublishTime.WithLabelValues(configMap).Observe(time.Since(start).Seconds())
return &csi.NodeUnpublishVolumeResponse{}, nil
Expand Down

0 comments on commit 506dd87

Please sign in to comment.