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.

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

Ensure that ansible-runner worker --worker-info Returns Unique IDs #854

Merged
merged 10 commits into from
Oct 5, 2021
Merged

Ensure that ansible-runner worker --worker-info Returns Unique IDs #854

merged 10 commits into from
Oct 5, 2021

Conversation

beeankha
Copy link
Contributor

@beeankha beeankha commented Sep 28, 2021

Fixes #850

In certain cases (e.g. when utilizing VMs spun up with IBM Cloud), the provided machine ID for each node will sometimes be identical; this code change would make it so that we purposefully generate a brand new UUID for each node and store it in a file for retrieval (the details of this file and its location are still TBD); this will ensure an actually unique UUID for each worker node.

@beeankha
Copy link
Contributor Author

recheck

ansible_runner/utils/capacity.py Outdated Show resolved Hide resolved
ansible_runner/utils/capacity.py Outdated Show resolved Hide resolved
ansible_runner/utils/capacity.py Show resolved Hide resolved
@samdoran
Copy link
Contributor

samdoran commented Sep 29, 2021

I just looked at the existing tests and there are no tests at all for get_uuid(). That means that changing this function is risky since we 1) have nothing that validates the current behavior, 2) have no way of knowing if the changes made here break that behavior, and 3) have no tests for the new code.

At a minimum, we need tests for the net new code. It would be really great to create tests for the existing function before changing it as well.

beeankha and others added 2 commits October 4, 2021 09:48
Change both functions to accept a path to the UUID file, defaulting to an
immutable Path object. This allows easier testing without mocking.

Change function names to refelct that one sets the UUID (_set_uuid()), always
overwriting any existing file while the other function, ensure_uuid(), will
only set the UUID if the UUID file does not exist.

Add unit tests.
@shanemcd shanemcd changed the title [WIP] Ensure that ansible-runner worker --worker-info Returns Unique IDs Ensure that ansible-runner worker --worker-info Returns Unique IDs Oct 4, 2021
@shanemcd shanemcd marked this pull request as ready for review October 4, 2021 22:00
@shanemcd shanemcd requested a review from a team as a code owner October 4, 2021 22:00
beeankha and others added 5 commits October 4, 2021 18:20
While Path objects are immutable, the previous default had two main problems:

  1. Function defaults are processed on import. The previous default meant that
     quite a bit of work was done just when the function was imported.
  2. Since the value of Path.home() varies depending on the user invoking the
     function, the means the function default is not consistent.

Using None then setting the default value within the function is a bit noisier,
but addresses the two issues above.
Add parameters for accepting mode.
Add tests for ensuring the mode is set properly.
@samdoran samdoran added the gate label Oct 5, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@samdoran
Copy link
Contributor

samdoran commented Oct 5, 2021

recheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ansible-runner worker --worker-info is returning same UUID for 3 different VM's
4 participants