overhaul of unarchive module

Hi all, I’m the original author of the unarchive module and I think I need to give it an overhaul. I wanted to mention it before I got started in case anyone had comments/ideas. I’ve been having the same conversation in several issue comments and I wanted to make one overall conversation about the general state of unarchive, hence this post.

There are a lot of people wanting more options added to the module. Options like “–strip-components” seem to be in great demand, while more obscure options, like “–numeric-owner” would be nice, but are less in demand.
Here’s what I’m thinking I should do:

  1. add options with a lot of requests (like --strip-components) as thier own options.
  2. add a generic extra_options option that would allow folks to use obscure tool specific options that we haven’t included yet
  3. overhaul how the unarchive module handles options to make it easier to add more options.

If no one cares, I’m just going to proceed with that plan. Otherwise, please let me know what you think.

Thanks!
-Dylan

Hi all, I'm the original author of the unarchive module and I think I need
to give it an overhaul. I wanted to mention it before I got started in case
anyone had comments/ideas. I've been having the same conversation in
several issue comments and I wanted to make one overall conversation about
the general state of unarchive, hence this post.

There are a lot of people wanting more options added to the module. Options
like "--strip-components" seem to be in great demand, while more obscure
options, like "--numeric-owner" would be nice, but are less in demand.
Here's what I'm thinking I should do:

1) add options with a lot of requests (like --strip-components) as thier own
options.

Sounds good.

2) add a generic extra_options option that would allow folks to use obscure
tool specific options that we haven't included yet

Not sure -- This makes it so that playbook tasks are tied to the
specific command used to unarchive. Is that an anti-design goal or
acceptable? @bcoca, jimi-c?

3) overhaul how the unarchive module handles options to make it easier to
add more options.

+1

-Toshio

For me #2 is the same as having the shell: module, it would be
preferable not to need it, but sometimes you have no choice.

We should just make sure docs clearly state that changed state is not
really verifiable that way nor that we can ensure they work across OSs
nor versions.

I’m getting discouraged. The features provided by the tar utility and the unzip utility are very different. I don’t think I’ve seen a single request for a feature that is supported by both. I need guidance from people with a wider ansible-philosophy view. IE I need to know what the right ansible way to do this is. Here’s what I’ve thought of so far.

  1. Make two separate modules for tar and unzip. Then utility specific features would make sense as module arguments.
  2. Add a generic “extra_options” option so whatever underlying utility is being used can be tweaked by users
  3. Stop using utilities to do the work and handle everything in python. Then we could implement any feature we wanted. Note: I don’t have time to implement this by myself.
  4. Only support features shared by both tar and unzip and for everything else, tell users to go use shell
  5. Add utility specific features as arguments even though that’s confusing

I prefer #2. Less work for me, enables features no one has asked for, hard to screw up.
#1 might be the “best” way to go. We have separate modules for yum and apt, so why only one module for tar and unzip?

The real value of the unarchive module, as I see it, is that it handles the downloading of the archive and the deleting of the archive. If you’re using unarchive to unpack something you’ve already got on your target box, you really might as well use shell. That being said, I wonder if there’s a way to write a module that does the copy & delete steps? Present a file-ish thing for use by other modules? Maybe make it a special url or something, so you could do a shell line like

shell: tar -xzf ansible://mything/myfile.tgz

and ansbile would know that meant download the file into your temp area and substitute the path to the temp area for ansible://mything/myfile.tgz in the shell line. This might sound trivial, but the copy and delete steps suck when you do

copy: src=myfile.tgz dest=/tmp/myfile.tgz
shell: chdir=/use/local/mything tar -xzf myfile.tgz
file: path=/tmp/myfile.tgz state=absent

Okay, I’m rambling…

Please let me know what you think we should do about the direction of the unarchive module

Thanks
-Dylan

Dylan,

here are my thoughts, though they might not add much clarity.

#2 might be the easiest but also would make it very hard to detect
changed= and would always need to execute when present (not saying no,
just establishing the price

#4) is 'current state', feature is not implemented and users already
need to go to shell, which has some more consequences than #2
#3) might be 'best' but also requires the writing of LOTS of code that
will still not function as people expect as you are duplicating
functionality that already exists in the tools

#1) might be only one that gives everyone what they want, but it still
requires lots of work and breeds complexity and possibly duplicate
code (or find good way for action plugins to share code).

#5) is doing #1 in a single module, fixing the duplication issues but
adding much confusion ....

i even have #6, use unarchive for common functions and have zip and
tar as their own modules (which unarchive can call), this would follow
the pattern established by the package module/action plugin.

I understand your frustration as there does not seem to be a 'good way
out' just a 'least bad choice'.

Waitaminute… a module can call other modules? I thought I couldn’t do that. I wanted to call the copy module instead of re-creating it when I started this thing. Or do action plugins cause a special case?

If I was going to do #1, I’d definitely make the action plugin part generic (if it isn’t already). That might make other modules easier too.

Agree on all points. Definitely don’t have time for #3 on my own.

check template action plugin, it calls copy and file, the new
'package' just calls whatever underlying package management module it
can find

Sorry, just finally got some time to work on this again. (If this is the wrong way to post updates, please let me know) I’m writing this in hopes of feedback for my plan.

I think I’m going to try making three unarchive modules: a tar specific, a zip specific and a generic. To make that work, it looks like I’m going to have to make three separate action plugins, because apparently action plugins have to have the same name as their module. There will be a lot of common code, so I should be able to put that into a lib somewhere.

For the client-side module, I think I have to make three there as well, though It seems like it should be possible to make the generic one just do file type discovery and then hand off to the specific client-side module, so not a lot of duplication.

If I understand correctly, using the generic module on a tar file would act something like this:

  • generic-unarchive-action-plugin copies the archive to the client if needed and calls the generic-unarchive-module on the client

  • generic-unarchive-module determines the file type as tar and tells the action plugin on the server

  • generic-unarchive-action-plugin calls the tar-specific-unarchive-module

  • tar-specific-unarchive-module unpacks the archive

Any feedback is appreciated.
-Dylan

I kind of like how unarchive handles anything with an archive at the moment, this was the basis for me not to do any kind of unarchiving in the uncompress module I created. Great job on the unarchive module by the way.

Hey folks. I submitted two PRs for the changes discussed in this thread and I haven’t heard any feedback. I’m not convinced I did this the best way, so any feedback would be really appreciated.

https://github.com/ansible/ansible/pull/13869https://github.com/ansible/ansible-modules-core/pull/2806

There are other PRs for unarchive that I can’t integrate until I know if people liked the direction I took on my changes.

Thanks
-Dylan

Hi again. I haven’t received any real feedback saying my patch to break unarchive into multiple modules is good or bad. I’m going to assume that means it’s bad. I’m going to check out the latest version, add extra-args and call it good.

Dylan,

Thanks for your transparency on this. So is https://github.com/ansible/ansible/pull/13869 the current PR that you’d like feedback on? I see that you closed https://github.com/ansible/ansible-modules-core/pull/2806, presumably pending another refactor.

Feel free to post updates to this thread—I don’t receive email updates to the ansible GH repo because of heavy traffic, but this list strikes a fine balance.

If I undestand your question correctly, the answer is Yes. https://github.com/ansible/ansible/pull/13869 is actually the same as https://github.com/ansible/ansible-modules-core/pull/2806. I blew up 2806 trying to do a rebase. 13869 just has some changes from mainline added in so it would merge.

These both contain the changes I wanted feedback on. I’ve had a few items of feedback, but nothing that says “This is a good idea” or “This is a bad idea” I don’t have a lot of spare time to work on this and I don’t want to spend more of it if the community doesn’t like my idea. I trust the community’s judgement much more than my own.

I expected folks to say “This is a lousy idea, stop working on it” or “This is a good idea, here are five things that suck about it that you can fix before we accept the PR.”

There are PRs for features on unarchive that can go in if we skip my refactor, which I assume is what we should do.