Skip to content

WIP-DO NOT MERGE: Add json output #222

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

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

WIP-DO NOT MERGE: Add json output #222

Show file tree
Hide file tree
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
6 changes: 5 additions & 1 deletion ansible.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
[ssh_connection]
pipelining = True
pipelining = True
[defaults]
stdout_callback = json
callback_whitelist = json
json_indent = 4
Copy link
Collaborator Author

@BillAnastasiadis BillAnastasiadis Apr 26, 2024

Choose a reason for hiding this comment

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

These changes do not matter, as the cfg file is not used. Look at the description to use it in the future

15 changes: 8 additions & 7 deletions scripts/qesap/lib/cmds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,12 @@ def ansible_command_sequence(configure_data_ansible, base_project, sequence, ver
ansible_cmd_seq.append({'cmd': ssh_share})
original_env = dict(os.environ)
original_env['ANSIBLE_PIPELINING'] = 'True'
if profile:
original_env['ANSIBLE_CALLBACK_WHITELIST'] = 'ansible.posix.profile_tasks'
original_env['ANSIBLE_CALLBACKS_ENABLED'] = 'json'
original_env['ANSIBLE_LOAD_CALLBACK_PLUGINS'] = '1'
original_env['ANSIBLE_STDOUT_CALLBACK'] = 'json'
Copy link
Collaborator Author

@BillAnastasiadis BillAnastasiadis Apr 26, 2024

Choose a reason for hiding this comment

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

These are the ones to use in order to have json output

Copy link
Contributor

@hadeskun hadeskun Apr 26, 2024

Choose a reason for hiding this comment

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

master what happend with the profile code? if we decided to include the json format

#original_env['ANSIBLE_CONFIG'] = os.path.join(base_project, 'ansible.cfg')
#if profile:
#original_env['ANSIBLE_CALLBACK_WHITELIST'] = 'json'
if 'roles_path' in configure_data_ansible:
original_env['ANSIBLE_ROLES_PATH'] = configure_data_ansible['roles_path']

Expand Down Expand Up @@ -345,18 +349,15 @@ def cmd_ansible(configure_data, base_project, dryrun, verbose, destroy=False, pr
return Status("ok")

inventory = os.path.join(base_project, 'terraform', configure_data['provider'], 'inventory.yaml')
ansible_cmd_seq = ansible_command_sequence(configure_data['ansible'], base_project, sequence, verbose, inventory, profile)
ansible_cmd_seq = ansible_command_sequence(configure_data['ansible'], base_project, sequence, False, inventory, profile)
Copy link
Collaborator Author

@BillAnastasiadis BillAnastasiadis Apr 26, 2024

Choose a reason for hiding this comment

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

Removed the verbose output, not sure if it impacts json output in any way, I think not

Copy link
Contributor

@hadeskun hadeskun Apr 26, 2024

Choose a reason for hiding this comment

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

Maybe not but will afect this pr already merge os-autoinst/os-autoinst-distri-opensuse#18787


for command in ansible_cmd_seq:
if dryrun:
print(' '.join(command['cmd']))
else:
ret, out = lib.process_manager.subprocess_run(**command)
log.debug("Ansible process return ret:%d", ret)
if ret == 0:
for out_line in out:
log.debug("> %s", out_line)
else:
if ret != 0:
log.error("command:%s returned non zero %d", command, ret)
return Status(ret)
return Status("ok")
8 changes: 7 additions & 1 deletion scripts/qesap/lib/process_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
All tools needed to manage external executable and processes
'''

import sys
import subprocess
import logging

Expand Down Expand Up @@ -31,6 +32,11 @@ def subprocess_run(cmd, env=None):
return (proc.returncode, [])
stdout = [line.decode("utf-8") for line in proc.stdout.splitlines()]

print(f"OUTPUT START FOR COMMAND: {cmd}")
sys.stdout.flush()
for line in stdout:
log.debug('OUTPUT: %s', line)
print(line)
sys.stdout.flush()
print(f"OUTPUT END FOR COMMAND: {cmd}")
sys.stdout.flush()
Copy link
Collaborator Author

@BillAnastasiadis BillAnastasiadis Apr 26, 2024

Choose a reason for hiding this comment

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

This part needed to change, otherwise the logging messages were mixed with the json output. This was strange and shouldn't happen, since this whole execution is serial and not parallel, but I think that the print commands stacked up instead of being immediately delivered to stdout, so at some point some of the latter logging messages were mixed in.

This was fixed by using sys.stdout.flush() after each line printing, to send it to stdout immediatelly, so... I was probably right? Dunno. But it works.

Copy link
Contributor

@hadeskun hadeskun Apr 26, 2024

Choose a reason for hiding this comment

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

nice master!!

return (0, stdout)