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

Fix 'Fail by Design' tasks #759

Merged
merged 2 commits into from
Mar 30, 2020
Merged

Fix 'Fail by Design' tasks #759

merged 2 commits into from
Mar 30, 2020

Conversation

cigamit
Copy link
Contributor

@cigamit cigamit commented Mar 27, 2020

SUMMARY

I expect this PR to be a bit controversal, but I think its one that we should give some thought / discussion.

Through the various workshops, we have multiple sections of code which utilize what I would call "Fail by Design" tasks. This is any task where we always expect the result to fail the first time the provisioner is ran. While using "block and rescue" to implement these tasks can be elegant, I think it is really misusing its original purpose and instead we are using it as a logic gate. It also leaves us with a nasty side effect, all the Angry Red Failure notices on every run of the provisioner. I know for myself, every change I make, I have to scroll back through all the failures and make sure they were all skipped and not ones caused by my change. I can't imagine what someone who runs this provisioner for the first time thinks.

What I propose instead, is taking any task that we expect to fail, and adding an actual check to the failed_when that excludes the statuses we know we will get when running it (to allow to actually fail if something else is completely wrong) or add a "failed_when: false" to it to stop the Angry Red Text. If it used a block / rescue, we would instead use blocks for the "rescue" tasks with a when statement that checks for the actual results we wanted (like status being 201, or the cert being present, etc...)

We would leave any block / rescue that are not "Fail by Design". We would also leave any tasks that we expect to fail but has "retries" set or an "until".

This PR removes all the failures you get when running the Windows workshop. I will have to do a few more commits to resolve any from the other workshops, but wanted to discuss this topic before putting in the effort.

"Failed by Design" Fixes

  • TASK [aws_dns : CHECK TO SEE IF SSL CERT ALREADY APPLIED]
  • TASK [code_server : check to see if SSL cert already applied]
  • TASK [control_node : check to see if ansible tower is already up and running]
  • TASK [gitlab-server : GitLab post | check to see if SSL cert already applied]
  • TASK [gitlab-server : GitLab Post | Check if root user password is set]
  • TASK [gitlab-client : Configure workshop repository]

"Fails on rerun because we are blindly executing awx-cli"

  • TASK [populate_tower : Remove demo job template]
  • TASK [populate_tower : Remove demo project]
  • TASK [populate_tower : Remove demo credential]
  • TASK [populate_tower : Remove demo inventory]
  • TASK [populate_tower : Create workshop inventory]
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • provisioner

@cloin
Copy link
Contributor

cloin commented Mar 27, 2020

The angry red bothers me as well

@IPvSean
Copy link
Contributor

IPvSean commented Mar 30, 2020

CI is passing except for a red herring with security
image

the security issue looks like a rhui issue->
image

this is good to merge

@IPvSean IPvSean merged commit dfa6536 into ansible:devel Mar 30, 2020
@cigamit cigamit deleted the errors branch March 31, 2020 02:50
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.

3 participants