Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

include datadog_skip_install #431

Closed
wants to merge 2 commits into from
Closed

include datadog_skip_install #431

wants to merge 2 commits into from

Conversation

rockaut
Copy link
Contributor

@rockaut rockaut commented Apr 11, 2022

fixes #430

  • Include a default datadog_skip_install variable set to false/no as default
  • Don't override datadog_skip install in set-parse-version.yml but check if its set or use default false

@rockaut rockaut requested a review from a team as a code owner April 11, 2022 12:31
@bkabrda
Copy link
Contributor

bkabrda commented Apr 12, 2022

Hi 👋 thanks for sending the PR! While I do agree with the idea of making this configurable, I think that if user provides an explicit value, it should override all further assignments to the variable.

I think the way this should work is that we assign e.g. an empty string to the variable in defaults/main.yml. Then in the tasks/set-parse-version.yml file:

  • If the value is empty string, do all the assignments that we do now.
  • If the value is not empty string, take it and skip all the assignments that we do now.

Does this make sense?

@rockaut
Copy link
Contributor Author

rockaut commented Apr 12, 2022

You mean if the user passes the variable with extra_var/cli? This takes already precedence over the variable in inventory (https://docs.ansible.com/ansible/latest/reference_appendices/general_precedence.html).

That said, you are right that currently something is not working. I today learned, that set_fact for variables is not "called" as whole if the variable is passed with extra_vars. This creates problems on boolean variables in newer Ansible versions as cli extra_vars are string type. I always thought one could then call |bool in set_fact but this doesn't work as said.

So there could be two solutions:

  1. set the variable datadog_skip_install but then actually create a _datadog_skip_install converted one for example in set-parse-version.yml...
  2. ... or just use datadog_skip_install|bool instead if the variable is used.

As the datadog_skip_install isn't used that often I would go with variant 2.

Beside the declaration of datadog_skip_install in defaults/main.yml has the nice side effect that one less task call is needed as set_fact in set-parse-version.yml is actually not needed anymore.

@rockaut
Copy link
Contributor Author

rockaut commented Apr 12, 2022

Done...

Without the variable set in inventory or cli, the agent will be installed:

datadog-agent on  main [✘!?] took 7s
❯ ansible-playbook site.yml -i inventories/hosts.yml --limit host01p

PLAY [linux] ***************************************************************************************************************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] *****************************************************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [set_fact] ************************************************************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [include_role : datadog.datadog] **************************************************************************************************************************************************************************************************************************************************************************************

TASK [datadog.datadog : Include Gather Ansible Facts task on Ansible >= 2.10] **********************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/facts-ansible10.yml for host01p

TASK [datadog.datadog : Gather Ansible Facts] ******************************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : Include Gather Ansible Facts task on Ansible < 2.10] ***********************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Check if OS is supported] **************************************************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/os-check.yml for host01p

TASK [datadog.datadog : Fail if OS is not supported] ***********************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Resolve datadog_tracked_checks later to defend against variable presidence issues arising from dynamically included null datadog_checks] ***********************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/sanitize-checks.yml for host01p

TASK [datadog.datadog : Defend against defined but null datadog_checks variable] *******************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : Resolve datadog_tracked_checks] ********************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : Check that datadog_checks is a mapping] ************************************************************************************************************************************************************************************************************************************************************
ok: [host01p] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [datadog.datadog : Set Facts for Datadog Agent Major Version] *********************************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/set-parse-version.yml for host01p

TASK [datadog.datadog : Convert datadog_agent_major_version to string] *****************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : include_tasks] *************************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Set Agent default major version] *******************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Debian Install Tasks] ******************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : RedHat Install Tasks] ******************************************************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/pkg-redhat.yml for host01p

...

PLAY RECAP *****************************************************************************************************************************************************************************************************************************************************************************************************************
host01p                   : ok=12   changed=0    unreachable=0    failed=0    skipped=5    rescued=0    ignored=0

with overriding the variable in cli the install step will be skipped:

datadog-agent on  main [✘!?] took 7s
❯ ansible-playbook site.yml -i inventories/hosts.yml --limit host01p -e datadog_skip_install=yes

PLAY [linux] ***************************************************************************************************************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] *****************************************************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [set_fact] ************************************************************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [include_role : datadog.datadog] **************************************************************************************************************************************************************************************************************************************************************************************

TASK [datadog.datadog : Include Gather Ansible Facts task on Ansible >= 2.10] **********************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/facts-ansible10.yml for host01p

TASK [datadog.datadog : Gather Ansible Facts] ******************************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : Include Gather Ansible Facts task on Ansible < 2.10] ***********************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Check if OS is supported] **************************************************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/os-check.yml for host01p

TASK [datadog.datadog : Fail if OS is not supported] ***********************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Resolve datadog_tracked_checks later to defend against variable presidence issues arising from dynamically included null datadog_checks] ***********************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/sanitize-checks.yml for host01p

TASK [datadog.datadog : Defend against defined but null datadog_checks variable] *******************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : Resolve datadog_tracked_checks] ********************************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : Check that datadog_checks is a mapping] ************************************************************************************************************************************************************************************************************************************************************
ok: [host01p] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [datadog.datadog : Set Facts for Datadog Agent Major Version] *********************************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/set-parse-version.yml for host01p

TASK [datadog.datadog : Convert datadog_agent_major_version to string] *****************************************************************************************************************************************************************************************************************************************************
ok: [host01p]

TASK [datadog.datadog : include_tasks] *************************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Set Agent default major version] *******************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Debian Install Tasks] ******************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : RedHat Install Tasks] ******************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Suse Install Tasks] ********************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Windows Install Tasks] *****************************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Linux Configuration Tasks (Agent 5)] ***************************************************************************************************************************************************************************************************************************************************************
skipping: [host01p]

TASK [datadog.datadog : Linux Configuration Tasks] *************************************************************************************************************************************************************************************************************************************************************************
included: /mnt/c/Users/fism/code/work/datadog-agent/roles/datadog.datadog/tasks/agent-linux.yml for host01p

...

I've tested this agains RHEL Linux. Can someone test this with windows?

@bkabrda
Copy link
Contributor

bkabrda commented Apr 12, 2022

Hi 👋
What I meant in my previous comment is that if datadog_skip_install is provided explicitly by the user, we should skip all the other tasks that possibly set it - see https://github.com/DataDog/ansible-datadog/blob/main/tasks/parse-version.yml#L85-L93. I think the best way to do it is to set it to e.g. empty string in the defaults/main.yml file. Then in tasks/parse-version.yml:

  • If datadog_skip_install is empty string, we know that the user didn't provide explicit true/false, hence we let the tasks currently living in the tasks/parse-version.yml file do what they do now.
  • If datadog_skip_install is not an empty string, we know that the user provided an explicit value. So we should take that value and not modify datadog_skip_install by further tasks in the tasks/parse-version.yml file.

In terms of the |bool conversion, I'd much prefer doing this in one place - the tasks/parse-version.yml file - to centralize how we create that variable and just be sure we have a boolean everywhere else.

@rockaut
Copy link
Contributor Author

rockaut commented Apr 15, 2022

I think I see what you mean. I had a rough week and going into easter vacation, but will look into it next week.

@amenon90
Copy link

Any updates with this PR? We are looking for this exact implementation to be available for our use case.
We are creating VM image with a specific agent version and holding it
While the checks and configuration bit is done when the VM is provisioned and doesn't need to install datadog agent.

@alopezz
Copy link
Contributor

alopezz commented Jul 19, 2024

I'm going to close this for now as the PR author didn't address the last feedback from our side (and it's been more than 2 years). @amenon90 if you're interested feel free to try to put this into good shape, otherwise we may address #430 at some future time (feel also free to add a comment there so that we can keep record of this still being a relevant feature).

@alopezz alopezz closed this Jul 19, 2024
@rockaut
Copy link
Contributor Author

rockaut commented Aug 6, 2024 •
edited
Loading

Heja there all,

yeah I now stumbled upon this again - somehow that got overlooked totally by me. But now I would need that again too.
I will see if I can redo this from scratch ... again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

datadog_skip_install=true as host_var is ignored
4 participants
@rockaut @bkabrda @amenon90 @alopezz