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

Playbook fixes #534

Merged
merged 2 commits into from
Jul 4, 2022
Merged

Playbook fixes #534

merged 2 commits into from
Jul 4, 2022

Conversation

abadger
Copy link
Member

@abadger abadger commented Jul 1, 2022

Fixes two playbook issues. These issues prevented the ansible playbook from converting centos machines. People could still run convert2rhel from the command line.

https://issues.redhat.com/browse/RHELC-650

abadger added 2 commits July 1, 2022 10:12
As reported in oamg#532, the logic in the ansible playbook was to fail when
either the version check showed an unhandled version OR the
skip_os_version_check was set to true. This meant that the toggle would
either check the version or the playbook will fail.

By using AND in that logic, we get the behaviour we want.  Either the
toggle will check the version or the playbook's check will succeed

It is confusing because it is a `fail:` task.  So a true value in the
conditional WILL fail the playbook while a false value in the
conditional means that the playbook WILL NOT fail here.

Fixes oamg#532
As reported in oamg#533, the loop syntax we have in the ansible playbook is
not quite right.  Instead of simply writing the variable name as we have
it now, we are responsible for asking for it to be expanded in the loop
construct (using "{{ }}").

Fixes oamg#533
@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #534 (d49e94c) into main (7e45e34) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #534   +/-   ##
=======================================
  Coverage   87.45%   87.45%           
=======================================
  Files          17       17           
  Lines        2392     2392           
  Branches      414      414           
=======================================
  Hits         2092     2092           
  Misses        239      239           
  Partials       61       61           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e45e34...d49e94c. Read the comment docs.

@abadger
Copy link
Member Author

abadger commented Jul 1, 2022

note: The ansible playbooks are not shipped in the rpms to customers so this change does not need an integration test and does not appear in the release notes.

Copy link
Member

@r0x0d r0x0d left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Clean and clear changes.

@r0x0d
Copy link
Member

r0x0d commented Jul 4, 2022

Thanks, @abadger, for the quick fix on this.

@r0x0d r0x0d merged commit 239b259 into oamg:main Jul 4, 2022
@abadger abadger deleted the playbook-fixes branch July 5, 2022 17:22
danmyway pushed a commit to Andrew-ang9/convert2rhel that referenced this pull request Jul 12, 2022
* Fix setup playbook feature allowing user to skip os version check.

As reported in oamg#532, the logic in the ansible playbook was to fail when
either the version check showed an unhandled version OR the
skip_os_version_check was set to true. This meant that the toggle would
either check the version or the playbook will fail.

By using AND in that logic, we get the behaviour we want.  Either the
toggle will check the version or the playbook's check will succeed

It is confusing because it is a `fail:` task.  So a true value in the
conditional WILL fail the playbook while a false value in the
conditional means that the playbook WILL NOT fail here.

Fixes oamg#532

* Fix syntax of looping over centos8_repos.

As reported in oamg#533, the loop syntax we have in the ansible playbook is
not quite right.  Instead of simply writing the variable name as we have
it now, we are responsible for asking for it to be expanded in the loop
construct (using "{{ }}").

Fixes oamg#533
@bocekm bocekm mentioned this pull request Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants