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.

Sign up for GitHub

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
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions ansible_bender/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,18 @@ def build(self, db_path):
tmp_pb_path = os.path.join(tmp, "p.yaml")
with open(self.pb, "r") as fd_r:
pb_dict = yaml.safe_load(fd_r)
host_present = False
for idx, doc in enumerate(pb_dict):
host = doc["hosts"]
logger.debug("play[%s], host = %s", idx, host)
doc["hosts"] = self.builder.ansible_host
try:
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

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

@kmehant kmehant Oct 31, 2019 •
edited
Loading

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 •
edited
Loading

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

with open(tmp_pb_path, "w") as fd:
yaml.safe_dump(pb_dict, fd)
playbook_base = os.path.basename(self.pb).split(".", 1)[0]
Expand Down