New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3FS not deleting buckets due to a lack of cleanup of path #892

Closed
egnoriega opened this Issue Jun 15, 2017 · 15 comments

Comments

Projects
None yet
4 participants
@egnoriega

egnoriega commented Jun 15, 2017

Summary

S3FS creates the key /data in a volume/S3 bucket based on the configuration variable libstorage.integration.volume.operations.mount.rootPath. It does not clean that path up once done so that even an empty Docker volume cannot be deleted. (The variable is also not (configurable via the plugin, so it cannot be changed to another path such as / or ignored).

Version

$ docker plugin ls
ID                  NAME                DESCRIPTION               ENABLED
b8fca06b2231        rexray/s3fs:0.9.0   REX-Ray for Amazon S3FS   true

Expected Behavior

Docker volume deletion should remove the associated bucket

Actual Behavior

Docker volume deletion returns and error and does not remove the associated bucket. Note, if the bucket/volume is never used, the base path is not created, and the volume can be deleted by docker volume rm without error.

Steps To Reproduce

$ docker volume create --driver b8f dockvol-example-com9997
$ docker run -it -v dockvol-example-com9997:/myvol centos bash
$ docker volume rm dockvol-example-com9997
Error response from daemon: unable to remove volume: remove dockvol-example-com9997: VolumeDriver.Remove: {"Error":"error deleting s3 bucket"}
$

Logs

From docker.log:

time="2017-06-15T19:36:15Z" level=info msg="time=\"2017-06-15T19:36:15Z\" level=debug msg=\"api call error json\" apiErr=\"{\\\"message\\\":\\\"error deleting s3 bucket\\\",\\\"status\\\":500,\\\"error\\\":{\\\"inner\\\":\\\"BucketNotEmpty: The bucket you tried to delete is not empty\\\\n\\\\tstatus code: 409, request id: 20FC053728DF4139\\\",\\\"volumeID\\\":\\\"dockvol-example-com9997\\\"}}\" host=\"unix:///var/run/libstorage/833594181.sock\" route=volumeRemove server=fossil-raver-cr time=1497555375851 tls=false txCR=1497555375 txID=3c83e772-6f43-4123-5a9c-48a424719785 " plugin=b8fca06b2231b4a53971353dfd718a312ccb49a7334a8b732489c87bebb177ab 

@egnoriega egnoriega changed the title from S3FS not deleting buckets due to a lack of cleanup of to S3FS not deleting buckets due to a lack of cleanup of path Jun 15, 2017

@clintkitson

This comment has been minimized.

Show comment
Hide comment
@clintkitson

clintkitson Jun 15, 2017

Member

Interesting @egnoriega. It sounds like making this a configurable option at the plugin layer to use / would help. But even then you would have to launch a container and then delete the contents.

I believe that the "force" option in remove would actually take care of this which could also be made an integration option.
https://github.com/codedellemc/libstorage/blob/master/drivers/storage/s3fs/storage/s3fs_storage.go#L310

Member

clintkitson commented Jun 15, 2017

Interesting @egnoriega. It sounds like making this a configurable option at the plugin layer to use / would help. But even then you would have to launch a container and then delete the contents.

I believe that the "force" option in remove would actually take care of this which could also be made an integration option.
https://github.com/codedellemc/libstorage/blob/master/drivers/storage/s3fs/storage/s3fs_storage.go#L310

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

@clintkitson is correct, this is why I implemented the force flag for this driver. The fact is that it's a shared filesystem, and as such we shouldn't delete the volume unless forced to do so.

/cc @cduchesne - we discussed the force flag at the time and reasoning behind not deleting by default unless forced

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

@clintkitson is correct, this is why I implemented the force flag for this driver. The fact is that it's a shared filesystem, and as such we shouldn't delete the volume unless forced to do so.

/cc @cduchesne - we discussed the force flag at the time and reasoning behind not deleting by default unless forced

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

Also, Docker's volume driver API doesn't allow for the specification of options for a remove operation, so you cannot specify force via the Docker CLI. It also doesn't appear we enable force by default via a configuration property either.

@egnoriega - for now you'd have to do rexray volume rm --force NAME to remove it.

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

Also, Docker's volume driver API doesn't allow for the specification of options for a remove operation, so you cannot specify force via the Docker CLI. It also doesn't appear we enable force by default via a configuration property either.

@egnoriega - for now you'd have to do rexray volume rm --force NAME to remove it.

@egnoriega

This comment has been minimized.

Show comment
Hide comment
@egnoriega

egnoriega Jun 15, 2017

I agree that deleting buckets can be a dangerous option, since they carry their own meta-data and drive process. docker volume rm -f NAME does appear to work as an option. I like the need to explicitly indicate a dangerous operation.

It is odd that an unused bucket can be deleted without force. It's just the non-empty nature of the bucket that prevents it. Since the driver can't see the contents of the bucket outside of /data it creates some level of confusion, as well as the possibility of ending up with lots of buckets which are "empty" and can't be deleted. I'd suggest "with force" deletes the contents of libstorage.integration.volume.operations.mount.rootPath and then deleting the bucket w/o force might be the expected behavior, since it deletes the rexray space but not an in use bucket. I'm still not sure about all the nice meta/acl and process information, and in particular the possible versioned objects, but they might prevent the bucket being deleted on their own. I'd need to run some more tests, and am open to doing so.

egnoriega commented Jun 15, 2017

I agree that deleting buckets can be a dangerous option, since they carry their own meta-data and drive process. docker volume rm -f NAME does appear to work as an option. I like the need to explicitly indicate a dangerous operation.

It is odd that an unused bucket can be deleted without force. It's just the non-empty nature of the bucket that prevents it. Since the driver can't see the contents of the bucket outside of /data it creates some level of confusion, as well as the possibility of ending up with lots of buckets which are "empty" and can't be deleted. I'd suggest "with force" deletes the contents of libstorage.integration.volume.operations.mount.rootPath and then deleting the bucket w/o force might be the expected behavior, since it deletes the rexray space but not an in use bucket. I'm still not sure about all the nice meta/acl and process information, and in particular the possible versioned objects, but they might prevent the bucket being deleted on their own. I'd need to run some more tests, and am open to doing so.

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

Wait, docker volume rm -f works? As in it deletes the S3FS bucket or you're saying that there is a -f flag for docker volume rm? Because the actual VolumeDriver Remove request doesn't carry any options with it:

image

Did you mean rexray volume rm -f NAME?

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

Wait, docker volume rm -f works? As in it deletes the S3FS bucket or you're saying that there is a -f flag for docker volume rm? Because the actual VolumeDriver Remove request doesn't carry any options with it:

image

Did you mean rexray volume rm -f NAME?

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

You're suggesting that for the S3FS driver the force flag should recursively delete the contents beneath the libStorage data directory (or whatever the root path is configured as) and then attempt a non-force delete to see if that will be sufficient?

I just want to be clear that I am just not comfortable ever performing a destructive operation on a shared file system that occurs as a result of some workflow where a user didn't specify force delete.

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

You're suggesting that for the S3FS driver the force flag should recursively delete the contents beneath the libStorage data directory (or whatever the root path is configured as) and then attempt a non-force delete to see if that will be sufficient?

I just want to be clear that I am just not comfortable ever performing a destructive operation on a shared file system that occurs as a result of some workflow where a user didn't specify force delete.

@egnoriega

This comment has been minimized.

Show comment
Hide comment
@egnoriega

egnoriega Jun 15, 2017

Nope docker rm -f is a valid flag. I do retract the "it works" for "it suppresses the error message" since it in-fact does not delete anything.

egnoriega commented Jun 15, 2017

Nope docker rm -f is a valid flag. I do retract the "it works" for "it suppresses the error message" since it in-fact does not delete anything.

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

Understood. I don't dispute that there is a valid flag; I just knew that even if there was one we were not receiving any type of information it indicated via the API request for the remove operation.

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

Understood. I don't dispute that there is a valid flag; I just knew that even if there was one we were not receiving any type of information it indicated via the API request for the remove operation.

@egnoriega

This comment has been minimized.

Show comment
Hide comment
@egnoriega

egnoriega Jun 15, 2017

I'm with you in terms of data loss, however the expected behavior under docker is that you can delete non-empty volumes, and these are shared structures (although within a given set of containers on one node/cluster, so smaller scope).

I'm more worried about the non-alignment between the bucket and the volume abstractions, since a volume is just storage whereas a bucket carries more stuff along.

egnoriega commented Jun 15, 2017

I'm with you in terms of data loss, however the expected behavior under docker is that you can delete non-empty volumes, and these are shared structures (although within a given set of containers on one node/cluster, so smaller scope).

I'm more worried about the non-alignment between the bucket and the volume abstractions, since a volume is just storage whereas a bucket carries more stuff along.

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

I'm more worried about the non-alignment between the bucket and the volume abstractions, since a volume is just storage whereas a bucket carries more stuff along.

So what you're suggesting is that a Volume WRT S3FS should be a VFS path inside a bucket and the bucket itself either be required to exist or a Create request creates the bucket if does not yet exist?

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

I'm more worried about the non-alignment between the bucket and the volume abstractions, since a volume is just storage whereas a bucket carries more stuff along.

So what you're suggesting is that a Volume WRT S3FS should be a VFS path inside a bucket and the bucket itself either be required to exist or a Create request creates the bucket if does not yet exist?

@egnoriega

This comment has been minimized.

Show comment
Hide comment
@egnoriega

egnoriega Jun 15, 2017

That would create something closer to what docker volumes are, which is a managed storage pool. I'd also like to be able to specify that path on create since it allows for sharing of data not just between containers but with other processes.

Creation of the bucket doesn't need to be part of a volume create, although it is handy. It does come with the issue of S3 name collision, so it's a bit different from what most volume create operations need to deal with.

egnoriega commented Jun 15, 2017

That would create something closer to what docker volumes are, which is a managed storage pool. I'd also like to be able to specify that path on create since it allows for sharing of data not just between containers but with other processes.

Creation of the bucket doesn't need to be part of a volume create, although it is handy. It does come with the issue of S3 name collision, so it's a bit different from what most volume create operations need to deal with.

@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

So, for what the S3FS driver is today treats buckets as volumes. I see what you're asking, and it's definitely valid. However I think if you view S3FS just as an object data store where the driver treats the bucket as the atomic storage container, then things are okay. The trouble begins when you start to want to access the bucket outside the workflow of Docker/REX-Ray. There you will definitely encounter issues.

I think we could enhance the S3FS driver to accommodate your ask -- to create a much richer S3FS experience. I just don't think know what the priority to do that might be.

You're of course welcome to submit a PR :)

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

So, for what the S3FS driver is today treats buckets as volumes. I see what you're asking, and it's definitely valid. However I think if you view S3FS just as an object data store where the driver treats the bucket as the atomic storage container, then things are okay. The trouble begins when you start to want to access the bucket outside the workflow of Docker/REX-Ray. There you will definitely encounter issues.

I think we could enhance the S3FS driver to accommodate your ask -- to create a much richer S3FS experience. I just don't think know what the priority to do that might be.

You're of course welcome to submit a PR :)

@egnoriega

This comment has been minimized.

Show comment
Hide comment
@egnoriega

egnoriega Jun 15, 2017

I will look into a PR. I'm not sure it answers the confusing deletion issue, but it's mostly a documentation issue and does land on the safe side of things.

egnoriega commented Jun 15, 2017

I will look into a PR. I'm not sure it answers the confusing deletion issue, but it's mostly a documentation issue and does land on the safe side of things.

@egnoriega egnoriega closed this Jun 15, 2017

akutz added a commit to akutz/libstorage that referenced this issue Jun 15, 2017

Enable Forced Vol Removal via Ig Drvr
This patch enables forced volume removal via a configuration property
`libstorage.integration.volume.operations.remove.force` for the
integration driver. This will enable Docker modules in REX-Ray to be
configured to force remove backend storage volumes for platforms that
require such behavior, such as S3FS.

This patch is related to rexray/rexray#892.
@akutz

This comment has been minimized.

Show comment
Hide comment
@akutz

akutz Jun 15, 2017

Member

Hi @egnoriega,

Please see thecodeteam/libstorage#577. The below config example illustrates how to utilize this feature for a specific Docker module in REX-Ray:

libstorage:
  service: s3fs
rexray:
  modules:
# the default-docker module is implicitly defined, but it's included
# here and commented out just to show that it isn't affected by the 
# second module which is configured to do forced volume removal
#    default-docker:
#      type: docker
#      desc: The default docker module.
#      host: unix:///run/docker/plugins/rexray.sock
    vrmf:
      type: docker
      desc: A docker module that force removes volumes.
      host: unix:///run/docker/plugins/rexray-vrmf.sock
      libstorage:
        integration:
          volume:
            operations:
              remove:
                force: true

The above configuration depends upon PR thecodeteam/libstorage#577.

Member

akutz commented Jun 15, 2017

Hi @egnoriega,

Please see thecodeteam/libstorage#577. The below config example illustrates how to utilize this feature for a specific Docker module in REX-Ray:

libstorage:
  service: s3fs
rexray:
  modules:
# the default-docker module is implicitly defined, but it's included
# here and commented out just to show that it isn't affected by the 
# second module which is configured to do forced volume removal
#    default-docker:
#      type: docker
#      desc: The default docker module.
#      host: unix:///run/docker/plugins/rexray.sock
    vrmf:
      type: docker
      desc: A docker module that force removes volumes.
      host: unix:///run/docker/plugins/rexray-vrmf.sock
      libstorage:
        integration:
          volume:
            operations:
              remove:
                force: true

The above configuration depends upon PR thecodeteam/libstorage#577.

@shafffooo

This comment has been minimized.

Show comment
Hide comment
@shafffooo

shafffooo Jun 20, 2018

Hi @akutz,

I am getting the same error when deleting a volume created using the rexray docker plugin for s3 storage. I am very new to rexray so I do not understand the solution you provided above. Where can I find this configuration file? Also is it possible to provide a command line option at plugin install time to make force removal of bucket as default when user issues docker volume -f to make it seamless for users?

shafffooo commented Jun 20, 2018

Hi @akutz,

I am getting the same error when deleting a volume created using the rexray docker plugin for s3 storage. I am very new to rexray so I do not understand the solution you provided above. Where can I find this configuration file? Also is it possible to provide a command line option at plugin install time to make force removal of bucket as default when user issues docker volume -f to make it seamless for users?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment