Skip to content
This repository has been archived by the owner on Mar 6, 2023. It is now read-only.

Separate downloading and unpacking prometheus archive #102

Merged
merged 3 commits into from
Apr 20, 2018
Merged

Conversation

paulfantom
Copy link
Member

@paulfantom paulfantom commented Apr 17, 2018

Resolves #101

Possibly accidentally solves ans issue with too many downloads of prometheus archive to localhost.

@taiansu
Copy link

taiansu commented Apr 18, 2018

Thanks for the blasting fast fixing. I tried the fix, and with two minor modifications, it works correctly.

The changes are:

  1. Replace line#55 's delegate_to: localhost with remote_src: yes
  2. Add a remote_src: yes for copy module, for example, between line#61 and line#62.

@SuperQ
Copy link
Collaborator

SuperQ commented Apr 18, 2018

Thinking about this, can we download these archives to something like /opt/prometheus/.archive/ rather than /tmp? I'm not sure if the ansible get_url is safe against existing symlinks/hardlinks in /tmp.

@paulfantom
Copy link
Member Author

@taiansu your modifications would change how we want this role to work. If we add remote_src in the places you say it will cause ansible to download archives to target hosts and this is highly inefficient in large infrastructures (also sometimes won't work if target hosts doesn't have internet access).
Our approach is to download archives from internet to deployer machine (one which runs ansible) and propaagte from this machine to other nodes.

@paulfantom
Copy link
Member Author

paulfantom commented Apr 18, 2018

@SuperQ we could probably use tmp_dest feature of get_url module to solve this issue, but I need to look closer at it.

@paulfantom
Copy link
Member Author

@taiansu can you tell what is not working in current version? Some error messages could be useful.

@SuperQ
Copy link
Collaborator

SuperQ commented Apr 18, 2018

One other nice thing, it would be good to cache this so it doesn't download the file every time. In theory, we could extract the checksum from the sums published on github.

@paulfantom
Copy link
Member Author

@SuperQ I agree that it would be nice to validate checksums. This option is even provided in get_url module. However first we would need to download checksums. I think it would be better to split it into another issue (#104).
As for caching, this won't be possible with tmp_dest parameter and I don't think that storing it in /opt is a good idea. Currently we provide some caching functionality by storing files in /tmp directory, which is cleaned only after deployer machine is rebooted.

@paulfantom paulfantom changed the title Separate downloading and unarchiving prometheus archive Separate downloading and unpacking prometheus archive Apr 18, 2018
@taiansu
Copy link

taiansu commented Apr 18, 2018

Yes I understand that will change the behaviour. If I apply the changes as is, the first error I got is like:

fatal: [test -> localhost]: FAILED! => {"changed": false, "msg": "Failed to get information on remote file (/tmp): sudo: a password is required\n"}

And adding become: yes to the unarchive module still got the same error. Then this leads me to the thread ansible/ansible#6131 (comment) which describes the presumption of unarchive module.

@paulfantom
Copy link
Member Author

@taiansu this should be fixed now, could you test it?

@paulfantom
Copy link
Member Author

@SuperQ I don't think we can use other directories on deployer machine apart from /tmp due to permission issues.

@rdemachkovych
Copy link
Member

For me, it's required install libselinux-python and work fine

@taiansu
Copy link

taiansu commented Apr 18, 2018

I tried the fix, but I got the simillar error back to the first point:

fatal: [test -> localhost]: FAILED! => {"changed": false, "msg": "Failed to find handler for \"/Users/tai/.ansible/tmp/ansible-tmp-1524074379.55-78415089495321/source\". Make sure the required command to extract the file is installed. Command \"/usr/bin/tar\" detected as tar type bsd. GNU tar required. Command \"/usr/bin/unzip\" could not handle archive."}

So I'm wondering the error may not be relate to the separate the download and unarchve, but rather we just can't unarchive a linux tar in the host while it's a Mac.

Anyway thanks for all the help.

@paulfantom
Copy link
Member Author

@taiansu yes it seems that there is a problem with tar on Mac:

Make sure the required command to extract the file is installed. Command "/usr/bin/tar" detected as tar type bsd. GNU tar required. Command "/usr/bin/unzip" could not handle archive.

Could you check if after installing gnu-tar this works?

brew install gnu-tar

@taiansu
Copy link

taiansu commented Apr 19, 2018

It works. Thanks so much!

@paulfantom
Copy link
Member Author

Great!
I've added this to readme in requirements section.

@rdemachkovych
Copy link
Member

Should we also update the Requiremets in the README?

Copy link
Member

@rdemachkovych rdemachkovych left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paulfantom paulfantom merged commit 9204adb into master Apr 20, 2018
@paulfantom paulfantom deleted the issue101 branch April 20, 2018 10:02
slomo pushed a commit to slomo/ansible-prometheus that referenced this pull request Dec 12, 2018
[patch] Separate downloading and unpacking prometheus archive
@lock
Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants