Making S3 get overwrite behavior match get_url

Hi,

I am using S3 to store my build artifacts and I would like to copy them to hosts using the s3 module using something like the following task:

  • s3: mode=get s3 bucket=myartifacts object=/website.zip dest=/srv/www/website.zip

The tasks accepts an ‘overwrite’ parameter that determines whether to overwrite the local file. Unfortunately, the behavior is different than what I would expect. If overwrite=yes, the local file will always be overwritten – even if it is identical. (Note that this means that the tasks always returns changed status.) Of overwrite=no, then the task will fail if an existing file is not identical.

At minimum I would like the module not to overwrite a local file with an identical one. The change would entail exiting early when the md5sums match.

But really, I would like to see the module behavior match the ‘force’ parameter from ‘get_url’: if ‘no’ leave any existing file even if different, if ‘yes’ overwrite if different. However, ‘force’ is already an alias for ‘overwrite’ so this means introducing backward incompatibility.

Am I correct in thinking current behavior is wrong? If so, what is the correct course of action: only fix overwriting identical files or change to ‘get_url’ semantics?

Regards,
Joost

If overwrite is no, we should make sure the task does not fail if the file already exists, that’s for sure.

If overwrite is yes, I DO agree it should overwrite the file, that is what was requested.

Yeah, I have the same complaint in my notes concerning the s3 module, it doesn’t quite match the copy module’s force argument which is what I expected.

From the copy module:

the default is yes, which will replace the remote file when contents are different than the source.
If no, the file will only be transferred if the destination does not exist. (added in Ansible 1.1)

Specifically, force=yes only replaces the remote file when contents are different than source. This differs from s3_module’s overwrite=yes which always replaces the file even if the contents are the same.

I agree that “overwrite” means the file should be overwritten no matter what, but there should be an option to only write the file when the contents are changed (and that isn’t overwrite=no because that implies never overwrite even if contents differ).

I guess there are three options being fit into a boolean:

  • always replace the file
  • replace the file only if contents have changed
  • never replace the file

s3_module is missing the middle option I believe.

–michael

Sounds like the solution here might be to introduce a trinary as a possible value, attempt to booleanify it using the module_utils code and if it doesn’t booleanify, but is set to “smart”, it would do that middle option.

This could keep everything working while adding the new bits.

"smart" sounds like a good compromise. I noticed some possibilities
for refactoring as well. Okay if I take a stab at it?

Regards,
Joost

Definitely, please do!

Joost,

Let me know if you are working on this, otherwise I can probably squeeze some time in to fix it. I would also like to see an option to set the file permissions of a local file when it is downloaded (but that may be beyond scope).

–michael

Hi Micheal,

Yeah, I'm working on it. Should have a PR this evening. Thanks for the
offer, if something comes between I'll take you up on it!

Regards,
Joost

Hi,

I created a pull request for the changes I made:
https://github.com/ansible/ansible/pull/7797

@MichaelS: if you want to add file permissions we should probably take
it one step at a time. I hope the refactorings will make it easier to
implement your changes.

Regards,
Joost

Joost,

Yeah that functionality should be separately added, so I’ll work on it when I get the chance. I just used the code in your PR for a few deploys and the smart more worked as I was expecting. Thanks!

–michael