Skip to content

Commit

Permalink
feat(CSI-239): moveToTrash does not return error to upper layers
Browse files Browse the repository at this point in the history
  • Loading branch information
sergeyberezansky committed Sep 11, 2024
1 parent b0d812b commit eeb45e2
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 6 deletions.
10 changes: 6 additions & 4 deletions pkg/wekafs/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ func initInnerPathVolumeGc(mounter AnyMounter) *innerPathVolGc {
return &gc
}

func (gc *innerPathVolGc) triggerGcVolume(ctx context.Context, volume *Volume) {
func (gc *innerPathVolGc) triggerGcVolume(ctx context.Context, volume *Volume) error {
op := "triggerGcVolume"
ctx, span := otel.Tracer(TracerName).Start(ctx, op)
defer span.End()
ctx = log.With().Str("trace_id", span.SpanContext().TraceID().String()).Str("span_id", span.SpanContext().SpanID().String()).Str("op", op).Logger().WithContext(ctx)
logger := log.Ctx(ctx).With().Str("volume_id", volume.GetId()).Logger()
logger.Info().Msg("Triggering garbage collection of volume")
gc.moveVolumeToTrash(ctx, volume) // always do it synchronously
return gc.moveVolumeToTrash(ctx, volume)
}

func (gc *innerPathVolGc) moveVolumeToTrash(ctx context.Context, volume *Volume) {
func (gc *innerPathVolGc) moveVolumeToTrash(ctx context.Context, volume *Volume) error {
op := "moveVolumeToTrash"
ctx, span := otel.Tracer(TracerName).Start(ctx, op)
defer span.End()
Expand All @@ -54,7 +54,7 @@ func (gc *innerPathVolGc) moveVolumeToTrash(ctx context.Context, volume *Volume)
defer unmount()
if err != nil {
logger.Error().Err(err).Msg("Failed to mount filesystem for GC processing")
return
return err
}
volumeTrashLoc := filepath.Join(path, garbagePath)
if err := os.MkdirAll(volumeTrashLoc, DefaultVolumePermissions); err != nil {
Expand All @@ -68,6 +68,7 @@ func (gc *innerPathVolGc) moveVolumeToTrash(ctx context.Context, volume *Volume)
if err := os.Rename(fullPath, newPath); err != nil {
logger.Error().Err(err).Str("full_path", fullPath).
Str("volume_trash_location", volumeTrashLoc).Msg("Failed to move volume contents to volumeTrashLoc")
return err
}
// NOTE: there is a problem of directory leaks here. If the volume innerPath is deeper than /csi-volumes/vol-name,
// e.g. if using statically provisioned volume, we move only the deepest directory
Expand All @@ -77,6 +78,7 @@ func (gc *innerPathVolGc) moveVolumeToTrash(ctx context.Context, volume *Volume)
// 2024-07-29: apparently seems this is not a real problem since static volumes are not deleted this way
// and dynamic volumes are always created inside the /csi-volumes
logger.Debug().Str("full_path", fullPath).Str("volume_trash_location", volumeTrashLoc).Msg("Volume contents moved to trash")
return nil
}

func (gc *innerPathVolGc) purgeLeftovers(ctx context.Context, fs string, apiClient *apiclient.ApiClient) {
Expand Down
3 changes: 1 addition & 2 deletions pkg/wekafs/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,7 @@ func (v *Volume) updateCapacityXattr(ctx context.Context, enforceCapacity *bool,

func (v *Volume) Trash(ctx context.Context) error {
if v.requiresGc() {
v.server.getMounter().getGarbageCollector().triggerGcVolume(ctx, v)
return nil
return v.server.getMounter().getGarbageCollector().triggerGcVolume(ctx, v)
}
return v.Delete(ctx)
}
Expand Down

0 comments on commit eeb45e2

Please sign in to comment.