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

pkg-debian: fix repo idempotence #250

Closed
wants to merge 1 commit into from
Closed

pkg-debian: fix repo idempotence #250

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions tasks/pkg-debian.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@

- name: Ensure Datadog repository is up-to-date
apt_repository:
filename: ansible_datadog_agent
filename: "ansible_datadog_{{ item.key }}"
repo: "{{ item.value }}"
state: "{% if item.key == datadog_agent_major_version|int and datadog_apt_repo | length == 0 %}present{% else %}absent{% endif %}"
update_cache: yes
Expand All @@ -53,7 +53,7 @@

- name: (Custom) Ensure Datadog repository is up-to-date
apt_repository:
filename: ansible_datadog_agent
filename: ansible_datadog_custom
Copy link
Contributor

@KSerrania KSerrania Feb 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

We tried doing that before, but this method has a problem in the following situation:

  • Let's say you do a first run with the following configuration:
datadog_agent_major_version: 7
datadog_apt_repo: "deb https://my.custom.repo stable main"

Then, after this run, you have:

$ find /etc/apt/sources.list.d/ -type f -print -exec cat {} \;
/etc/apt/sources.list.d/ansible_datadog_custom.list
deb https://my.custom.repo stable main
  • Then, on the following run, you decide to use an official repo and use the following configuration:
datadog_agent_major_version: 7

Because of this, the(Custom) task will be skipped, so the custom repository won't be added, but it won't be removed either, and you'll end up with:

$ find /etc/apt/sources.list.d/ -type f -print -exec cat {} \;
/etc/apt/sources.list.d/ansible_datadog_7.list
deb https://apt.datadoghq.com/ stable 7
/etc/apt/sources.list.d/ansible_datadog_custom.list
deb https://my.custom.repo stable main

and in this situation, the role may have an unexpected behavior as you can't know in advance from which source the installed package will come from.

Another problematic situation is the following:

  • First run with datadog_apt_repo: "deb https://my.custom.repo stable main"
  • Second run with datadog_apt_repo: "deb https://my.custom.repo2 stable main"
    Then you end up with:
$ find /etc/apt/sources.list.d/ -type f -print -exec cat {} \;
/etc/apt/sources.list.d/ansible_datadog_custom.list
deb https://my.custom.repo stable main
deb https://my.custom.repo2 stable main

In this case, you don't know from which custom repository will the package come from.

I think the following fix would work for both scenarios while keeping the role idempotency:

  • keep your current changes
  • remove lines 26-29 (they're useless now)
  • add the following tasks before this one:
- name: (Custom) Initialize custom repo file deletion flag to False
  set_fact:
    datadog_remove_custom_repo_file: "False"

- name: (Custom) Check if custom repository file exists
  stat:
    path: /etc/apt/sources.list.d/ansible_datadog_custom.list
  register: datadog_custom_repo_file

- name: (Custom) Flag custom repository file for deletion if different from current repository config
  set_fact:
    datadog_remove_custom_repo_file: "{{ datadog_repo_file_contents != datadog_apt_repo }}"
  vars:
    datadog_repo_file_contents: "{{ lookup('file', '/etc/apt/sources.list.d/ansible_datadog_custom.list') }}"
  when: datadog_custom_repo_file.stat.exists

- name: (Custom) Remove Datadog custom repository file when not set or updated
  file:
    path: /etc/apt/sources.list.d/ansible_datadog_custom.list
    state: absent
  vars:
    repo_file_contents: "{{ lookup('file', '/etc/apt/sources.list.d/ansible_datadog_custom.list') }}"
  when: (not (datadog_apt_repo | length > 0) or datadog_remove_custom_repo_file)  and (not ansible_check_mode)

That way, we make sure the previous custom repository is removed when:

  • datadog_apt_repo is not set (that way, we don't keep a previous custom repo activated when we want the official repos)
  • datadog_apt_repo has been modified (that way, we only use the custom repository currently specified in the role config).

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KSerrania thanks looking at my PR and for the thoughtful review! I hadn't considered the case that you outlined and agree that your proposed solution seems like the most straightforward way to address things 🚀

Copy link
Contributor

Choose a reason for hiding this comment

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

@jharley I'll cherry-pick the changes you made here and add my own then, if you don't mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KSerrania Not at all! Thank you 🙏

repo: "{{ datadog_apt_repo }}"
state: present
update_cache: yes
Expand Down