Skip to content
This repository has been archived by the owner on Sep 10, 2020. It is now read-only.

Simplifies group membership update task #90

Merged
merged 1 commit into from
Nov 30, 2016

Conversation

conorsch
Copy link
Contributor

The original intent here is to add all authorized users to the fpsd
group so that developers can run commands as the fpsd service account
without directly su'ing as the service account user. Makes sense.

The previous implementation was using a two-pass approach to accommodate
for improper idempotence handling in Ansible's user module. Was able
to reproduce the issue reliably on Ansible v2.1.1, but Ansible v2.2.0
has resolved the issue, and the role applies cleanly on multiple runs
with the new one-task approach.

with_together:
- "{{ groups_result.results }}"
- "{{ fpsd_database_usernames }}"
failed_when: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

failed_when: true seems like a mistake, but it doesn't always trigger due to the when: conditional. Either way, not a problem anymore.

@conorsch conorsch force-pushed the simplify-group-membership-tasks branch from d70436e to 436e065 Compare November 30, 2016 23:33
@conorsch
Copy link
Contributor Author

Rebased on top of latest master, given merge of #57. No conflicts.

The original intent here is to add all authorized users to the `fpsd`
group so that developers can run commands as the `fpsd` service account
without directly su'ing as the service account user. Makes sense.

The previous implementation was using a two-pass approach to accommodate
for improper idempotence handling in Ansible's `user` module. Was able
to reproduce the issue reliably on Ansible v2.1.1, but Ansible v2.2.0
has resolved the issue, and the role applies cleanly on multiple runs
with the new one-task approach.
@conorsch conorsch force-pushed the simplify-group-membership-tasks branch from 436e065 to dad2278 Compare November 30, 2016 23:39
@conorsch
Copy link
Contributor Author

Rebased on top of latest master, given merge of #89. (We needed the tbb version bump, since the Travis tests were failing on a 404 due to old version.)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.727% when pulling dad2278 on simplify-group-membership-tasks into 7538b1b on master.

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

LGTM

@redshiftzero redshiftzero merged commit 4f96bc0 into master Nov 30, 2016
@psivesely psivesely deleted the simplify-group-membership-tasks branch February 7, 2017 00:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants