debops / ansible-pki

Bootstrap and manage internal PKI, Certificate Authorities and OpenSSL/GnuTLS certificates
GNU General Public License v3.0
65 stars 29 forks source link

Update apt cache when older than 1 day #120

Closed mathieumd closed 6 years ago

mathieumd commented 6 years ago

Maybe the cache_valid_time would better be configurable, though?

drybjed commented 6 years ago

There is a fact in the debops.core which was meant to be used with this parameter to control the valid time centrally, however... In Debian and I think Ubuntu, there's an APT job configured in cron to update apt-cache daily. The unattended-upgrades command updates the APT cache periodically as well. Therefore during normal operation, the cache shouldn't be older than a day or so.

Do you get cache misses often with this role? In the common playbook, debops.pki role is executed before debops.apt which usually updates the cache if it's not up to date.

What I'm getting at is, I think that the issue is more with your environment than one role, since this parameter could be added to any and all apt module calls, but I want to avoid that. So, can you tell me more about why you want to add this parameter first?

mathieumd commented 6 years ago

My case is indeed not critical: I used debops.pki role on machines freshly deployed from images where APT caches were quite obsolete. Actually, as you said, the local unattended-upgrade would have update these caches sooner or later. But still, I believe it could be a good practice to have roles self-contained, so that they could be working flawlessly with the less possible dependencies. This one being an easy fix, I think it's worth it. Thanks anyway for your wonderful roles; I can't stop being amazed by your work!

drybjed commented 6 years ago

Wery well then. In that case I would change the value and use something like this:

- name: Install stuff
  apt:
    name: '{{ item }}'
    cache_valid_time: '{{ ansible_local.core.cache_valid_time
                          if (ansible_local|d() and ansible_local.core|d() and
                              ansible_local.core.cache_valid_time|d())
                          else "86400" }}'

Doing it this way in this and other roles in the future will let users control the interval in one place (debops.core role) instead of setting up variables for each role separately in the inventory. Keep in mind that DebOps does not have a concept of global variables which could be overridden by the user through Ansible inventory; for now Ansible local facts is the best equivalent we have.

mathieumd commented 6 years ago

Just like you do for other roles. Looks perfect to me! 😃

drybjed commented 6 years ago

@mathieumd So, do you plan to update this PR according to my suggestion?

mathieumd commented 6 years ago

Here we are.