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

Add binary install directory #137

Merged
merged 1 commit into from
Jan 22, 2020

Conversation

guimaluf
Copy link
Contributor

Binaries are currently droped in /usr/local/bin

This commit add node_exporter_binary_install_dir variable where user can
change the binary destination to /opt/bin for example.

@github-actions github-actions bot added area/jinja Templates area/tasks Logic behind ansible role area/vars Ansible variables used in role labels Jan 13, 2020
Copy link
Member

@paulfantom paulfantom left a comment

Choose a reason for hiding this comment

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

It was a conscious decision to not provide such option in any of ansible role located in @cloudalchemy. /usr/local/bin is the correct place for those binaries and it strictly adheres to man hier(7).

What is the reasoning behind providing this as a feature?

@guimaluf
Copy link
Contributor Author

The main reason is that in the cluster I'm working with the /usr/local/bin is a mounted directory which I don't have permissions to change.

I'm aware that the correct hierarchy resides on /usr/local/bin that manner I keep the default as it is.
I could remove the task where the directory is created and let just as an option.

I don't mean to change the default behaviour. Just make it customizable.

Binaries are currently droped in /usr/local/bin

This commit add `node_exporter_binary_install_dir` variable where user can
change the binary destination to `/opt/bin` for example.
@guimaluf guimaluf force-pushed the add_binary_install_dir branch from e30b2aa to 07d7b4a Compare January 20, 2020 09:42
@guimaluf guimaluf requested a review from paulfantom January 22, 2020 15:03
@paulfantom paulfantom merged commit f6f13c7 into cloudalchemy:master Jan 22, 2020
@paulfantom
Copy link
Member

paulfantom commented Jan 22, 2020

After I merged it, I remembered one problem with this approach here and in previous designs. Always someone proposed to include a task responsible for creating a directory where node_exporter binary would be placed. This, in turn, had huge impact on managing directory permissions in default installations as we are by default making this role responsible for managing /usr/local/bin ownership (in

- name: create node_exporter binary install directory
file:
path: "{{ _node_exporter_binary_install_dir }}"
state: directory
owner: root
group: root
).

This task will be removed in future PR and if anyone wants to create a directory, then he/she needs to do it on a playbook level as managing system directories is out of scope for this role.

paulfantom added a commit that referenced this pull request Jan 22, 2020
By default node_exporter binary is placed in `/usr/local/bin` which causes removed task to manage this directory. Such "feature" is out of scope of this role and can cause problems in some deployment scenarios.\

This is a follow-up to #137
paulfantom added a commit that referenced this pull request Jan 24, 2020
By default node_exporter binary is placed in `/usr/local/bin` which causes removed task to manage this directory. Such "feature" is out of scope of this role and can cause problems in some deployment scenarios.\

This is a follow-up to #137
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/jinja Templates area/tasks Logic behind ansible role area/vars Ansible variables used in role
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants