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

[WIP]Suppress error when hosts key not found in playbook #178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kmehant
Copy link
Member

@kmehant kmehant commented Oct 31, 2019

Context

  • partially fixes Support import_playbook #109
  • As ansible-bender tries to modify the host key in the playbook, perhaps fails when it is not found or hidden in any of the playbooks imported.

@ansible-zuul
Copy link
Contributor

ansible-zuul bot commented Oct 31, 2019

Build succeeded.

Comment on lines +210 to +219
try:
host = doc["hosts"]
host_present = True
except:
continue
finally:
if host_present:
logger.debug("play[%s], host = %s", idx, host)
doc["hosts"] = self.builder.ansible_host
break
Copy link
Member Author

@kmehant kmehant Oct 31, 2019

Choose a reason for hiding this comment

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

@TomasTomecek These changes would suppress the error in #109. Another part of this issue to be solved is regarding the import playbook. Usually, one case to consider would be a single playbook imported which I guess I would repeat the process as done on the original playbook to modify the hosts key. But when we have multiple playbooks imported, choosing them with extra-args would complex this problem, I wish to know your thoughts regarding your design strategy which we can follow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the playbooks are being imported from within a playbook: it means that users can import from 0 to infinity playbooks and bender needs to be able to cope with that; I'd say that extra-args don't play a role here

I'm personally completely unsure why the error happened in the first place - I'd suggest creating a test case using the playbook specified in the issue and check closely what exactly is going wrong

Copy link
Member Author

@kmehant kmehant Nov 1, 2019

Choose a reason for hiding this comment

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

@TomasTomecek As I went through the issue's playbooks, I understood that ab is trying to extract the host key from the primary playbook (which is not present) so throws a keyError, Perhaps as we have the options of defining host in the imported playbooks, now ansible has to go step deeper to dig the host key from the imported playbooks, Am I on the right track?
regarding my changes, I have used the boolean flag host_present just to make sure that the host key is not present in the primary playbook then workout in the imported playbooks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

extract the host key from the primary playbook (which is not present) so throws a keyError

I'm not sure if that's really happening or I may not understand what you're trying to say.

I still suggest to write a test case for the issue and diagnose the problem like that. I used import_playbook for CI purposes in my other PR so you can get inspired: https://github.com/ansible-community/ansible-bender/pull/179/files#diff-283706639170035ea2a7403ae0e60f2dR7

@kmehant kmehant changed the title Suppress error when hosts key not found in playbook [WIP]Suppress error when hosts key not found in playbook Nov 1, 2019
Copy link
Collaborator

@TomasTomecek TomasTomecek left a comment

Choose a reason for hiding this comment

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

Are my comments helpful or should I provide more details?

The thing is that I'd need to dig into this one myself to understand the issue well.

host = doc["hosts"]
host_present = True
except:
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

looking at my code, I can see that the host is retrieved only for logging purposes, so I'd instead do something like:

try:
  host = doc["hosts"]
except KeyError:
  host = "not set"

and continue with the former flow

Comment on lines +210 to +219
try:
host = doc["hosts"]
host_present = True
except:
continue
finally:
if host_present:
logger.debug("play[%s], host = %s", idx, host)
doc["hosts"] = self.builder.ansible_host
break
Copy link
Collaborator

Choose a reason for hiding this comment

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

the playbooks are being imported from within a playbook: it means that users can import from 0 to infinity playbooks and bender needs to be able to cope with that; I'd say that extra-args don't play a role here

I'm personally completely unsure why the error happened in the first place - I'd suggest creating a test case using the playbook specified in the issue and check closely what exactly is going wrong

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.

Support import_playbook
2 participants