Reworking-varnish-role #2354
Reworking-varnish-role #2354
Conversation
|
There was a problem hiding this comment.
thanks, left some comments.
I wonder why we have to do this systemd file override. Is there a fix we could do in the upstream varnish role https://github.com/geerlingguy/ansible-role-varnish ?
Please document why we need our own systemd service and why the default setup is broken with unattended upgrades.
| - name: Template varnish systemd service file | ||
| ansible.builtin.template: | ||
| src: varnish.service.j2 | ||
| dest: /etc/systemd/system/varnish.service |
There was a problem hiding this comment.
there is also a systemd file in /lib/systemd/system/varnish.service , probably installed by the varnish package. Do we need to take care of that?
| notify: | ||
| - reload systemd # handlers in https://github.com/geerlingguy/ansible-role-varnish/blob/master/handlers/main.yml | ||
| - restart varnish | ||
| - block: |
There was a problem hiding this comment.
any reason why we need this block? please add a code comment.
| - reload systemd # handlers in https://github.com/geerlingguy/ansible-role-varnish/blob/master/handlers/main.yml | ||
| - restart varnish | ||
| - block: | ||
| - name: Copy Varnish default VCL. |
There was a problem hiding this comment.
indentation should be 2 spaces, or is there a reason here are 4?
| limit_memlock: 85983232 | ||
| limit_core: infinity | ||
| tasks_max: infinity | ||
| varnish_storage: "malloc,{{ ansible_facts.memtotal_mb // 10 }}M" |
There was a problem hiding this comment.
i think we should not duplicate this variable under varnish_config. It already exists as global varnish_storage from the varnish role?
| @@ -17,3 +17,10 @@ varnish_config: | |||
| upstream_proxies: [] | |||
| # Provide an alternative filename if you are providing a template. | |||
| template_filename: default.vcl | |||
| # Varnish systemd overrides to make varnish consistent even after upgrades | |||
| varnish_pid: /run/varnishd.pid | |||
| limit_nofile: 131072 | |||
There was a problem hiding this comment.
this also already exists as varnish_limit_nofile
geerlinguy also uses systemd overrides: We can either use the overrides setup I created, or use geerlinguy variables for system overrides |
|
OK, since this vanrish_config role depends on the geerlingguy varnish role we should use the upstream variables to avoid duplication and confusion. |
No description provided.