Export ansible output to a file #234

hadeskun · 2024-05-27T07:37:40Z

Let qesap.py to always write ansible log to a separate file like terraform already did.
VR: https://openqaworker15.qa.suse.cz/tests/285632#
Ticket: https://jira.suse.com/browse/TEAM-6844

scripts/qesap/lib/cmds.py

mpagot · 2024-06-06T14:35:33Z

scripts/qesap/lib/cmds.py

            log.debug("Ansible process return ret:%d", ret)
+            playbook_path = command['cmd'][4]


does a selection as simple as "5th element" always works to get the playbook name? Is it not the presence or absence of -vvv enough to break this mechanism? Is it covered by UT?

If there's a motivation that we are aware that will make it always work we should put it as code comment (I'll do the same in the terraform function later in a next PR)

How this works for the first command that is ansible and not ansible-playbook so does not have any playbook file?

Yes have a file for ansible command and it is called ansible.all.log.txt

scripts/qesap/lib/cmds.py

mpagot · 2024-06-06T14:38:36Z

scripts/qesap/lib/cmds.py

            if ret != 0:
                log.error("command:%s returned non zero %d", command, ret)
-                return Status(ret)
+                return Status(f"Error at {command}")


why this is needed? Is it right?

Yes it is right, just to show the command and the error

But it is not showing the value of ret anymore

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

mpagot

Please add e2e tests, terraform already have e2e tests about the .log.txt files

I can also provide test to you if you like.

Please evaluate to add UT that cover this new feature

mpagot · 2024-06-10T14:37:59Z

scripts/qesap/test/e2e/test.sh

+
+test_step "[test_6.yaml] Run Ansible with playbooks"
+rm ansible.*.log.txt || echo "Nothing to delete"
+cp timbio.yaml "${QESAPROOT}/ansible/playbooks/timbio.yaml"


I think that as content is mostly the same, none of the 3 new are needed. Please just do:

cp sambuconero.yaml "${QESAPROOT}/ansible/playbooks/timbio.yaml" cp sambuconero.yaml "${QESAPROOT}/ansible/playbooks/buga.yaml" cp sambuconero.yaml "${QESAPROOT}/ansible/playbooks/purace.yaml"

Good point thx

mpagot · 2024-06-10T14:39:04Z

scripts/qesap/test/e2e/test.sh

+cp purace.yaml "${QESAPROOT}/ansible/playbooks/purace.yaml"
+qesap.py -b ${QESAPROOT} -c test_6.yaml ansible || test_die "test_6.yaml fail on ansible"
+ansible_logs_number=$(find . -type f -name "ansible.*.log.txt" | wc -l)
+[[ $ansible_logs_number -eq 4 ]] || test_die "ansible .log.txt are not 3 files but has ${ansible_logs_number}"


Suggested change

[[ $ansible_logs_number -eq 4 ]] || test_die "ansible .log.txt are not 3 files but has ${ansible_logs_number}"

# 3 playbooks plus a log file for the initial ansible (not ansible-playbook) call

[[ $ansible_logs_number -eq 4 ]] || test_die "ansible .log.txt are not 4 files but has ${ansible_logs_number}"

mpagot · 2024-06-10T14:42:46Z

scripts/qesap/test/unit/test_qesap_lib_cmd_terraform.py

@@ -187,4 +187,4 @@ def test_cmd_terraform(subprocess_run, tmpdir):

    assert ret == 0

-    subprocess_run.assert_has_calls(calls)
+    subprocess_run.assert_has_calls(calls)


last line missing

mpagot · 2024-06-10T14:45:59Z

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

+@mock.patch("lib.process_manager.subprocess_run")
+def test_cmd_ansible(subprocess_run, tmpdir):
+    '''
+    This test checks if the ansible execution generates log files


Which part of the test checks if the ansible execution generates log files ?

Suggested change

This test checks if the ansible execution generates log files

Given an ansible-playbook command line and some string to simulate an ansible stdout, this test verify that the function write them to file.

scripts/qesap/test/e2e/test.sh

mpagot

LGTM, any additional fixes can be applied later

mpagot · 2024-06-12T07:37:07Z

scripts/qesap/lib/cmds.py

    return Status("ok")
+
+
+def export_ansible_output(command, out):


Suggested change

def export_ansible_output(command, out):

def export_ansible_output(command, out):

"""

Utility function that get the ansible command line and the command output.

Function calculate the log name by extracting the ansible playbook name from the command line.

Function take the content of out and write it to a file in the current directory

"""

mpagot · 2024-06-12T07:41:52Z

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

+    '''
+
+    test_dir = os.getcwd()
+    test_file = os.path.join(test_dir, 'ansible.testAll.log.txt')


What is the purpose of

'test1':'test_value1', 'test2':'test_value2', 'test3':'test_value3', 'test4':'test_value4',

?

Is it to check that test_export_ansible_output ignore them?

mpagot · 2024-06-12T07:42:23Z

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

+    test_file = os.path.join(test_dir, 'ansible.testAll.log.txt')
+    command_to_sent = {'test1':'test_value1', 'test2':'test_value2', 'test3':'test_value3', 'test4':'test_value4',
+                        'cmd':['/tmp/exec_venv/bin/ansible', '-vvvv', '-i', '/root/qe-sap-deployment/terraform/aws/inventory.yaml',
+                              'testAll', '-a', '--ssh-extra-args="-l cloudadmin -o UpdateHostKeys=yes -o StrictHostKeyChecking=accept-new"']}


Suggested change

'testAll', '-a', '--ssh-extra-args="-l cloudadmin -o UpdateHostKeys=yes -o StrictHostKeyChecking=accept-new"']}

'/some/immaginary/path/testAll.yaml', '-a', '--ssh-extra-args="-l cloudadmin -o UpdateHostKeys=yes -o StrictHostKeyChecking=accept-new"']}

mpagot · 2024-06-12T07:45:05Z

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

+    command_to_sent = {'test1':'test_value1', 'test2':'test_value2', 'test3':'test_value3', 'test4':'test_value4',
+                        'cmd':['/tmp/exec_venv/bin/ansible', '-vvvv', '-i', '/root/qe-sap-deployment/terraform/aws/inventory.yaml',
+                              'testAll', '-a', '--ssh-extra-args="-l cloudadmin -o UpdateHostKeys=yes -o StrictHostKeyChecking=accept-new"']}
+    ansible_output = """DEBUG    OUTPUT: ansible [core 2.13.5]


does no matter at all for export_ansible_output what is the content of this string, but generally speaking content will not be like this but

ansible_output = """ansible [core 2.13.5] config file = None configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules'] ansible python module location = /tmp/exec_venv/lib64/python3.11/site-packages/ansible ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections executable location = /tmp/exec_venv/bin/ansible python version = 3.11.5 (main, Sep 06 2023, 11:21:05) [GCC] jinja version = 3.1.4 libyaml = True"""

yes indeed doesn't matter

mpagot · 2024-06-12T07:49:01Z

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

+    export_ansible_output(command_to_sent, ansible_output)
+
+    assert os.path.isfile(test_file), f"Ansible output file {test_file} was not created."
+    os.remove(test_file)


it is ok that you remove the file after the test. Generally speaking I do no t like when a UT write file around. I like to try to always let the UT to confine the place to read and write the FS to https://docs.pytest.org/en/7.1.x/how-to/tmp_path.html

This comment is a generic suggestion and we cal look at this later.

mpagot · 2024-06-12T10:44:33Z

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py

+log = logging.getLogger(__name__)
+
+
+def test_export_ansible_output():


Here you can see the execution of the single new test in this file [test_qesap_lib_cmd_ansible.py](https://github.com/SUSE/qe-sap-deployment/pull/234/files#diff-64bd1de2060c01d07e450f4bb3a2ce75e4827907a733d818310885ab8ccced55) --> log https://github.com/SUSE/qe-sap-deployment/actions/runs/9481063837/job/26123325056#step:7:33

And here an execution example with verbose mode on
https://github.com/SUSE/qe-sap-deployment/actions/runs/9481063837/job/26123325056#step:8:838

lpalovsky

LGTM

mpagot reviewed May 27, 2024

View reviewed changes

scripts/qesap/lib/cmds.py Outdated Show resolved Hide resolved

hadeskun force-pushed the exportAnsible branch from 43d8328 to 0acfaec Compare June 6, 2024 14:14

mpagot reviewed Jun 6, 2024

View reviewed changes

scripts/qesap/lib/cmds.py Outdated Show resolved Hide resolved

mpagot reviewed Jun 6, 2024

View reviewed changes

scripts/qesap/lib/cmds.py Outdated Show resolved Hide resolved

mpagot reviewed Jun 6, 2024

View reviewed changes

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py Outdated Show resolved Hide resolved

mpagot reviewed Jun 6, 2024

View reviewed changes

scripts/qesap/test/unit/test_qesap_lib_cmd_ansible.py Outdated Show resolved Hide resolved

mpagot reviewed Jun 6, 2024

View reviewed changes

hadeskun force-pushed the exportAnsible branch 6 times, most recently from 2de3e5e to dc9a436 Compare June 7, 2024 14:53

mpagot reviewed Jun 10, 2024

View reviewed changes

scripts/qesap/test/e2e/test.sh Outdated Show resolved Hide resolved

mpagot approved these changes Jun 11, 2024

View reviewed changes

mpagot reviewed Jun 12, 2024

View reviewed changes

hadeskun force-pushed the exportAnsible branch 2 times, most recently from f712813 to a1d2348 Compare June 12, 2024 10:06

mpagot reviewed Jun 12, 2024

View reviewed changes

hadeskun force-pushed the exportAnsible branch from 735ca3d to 775dd2c Compare June 12, 2024 12:14

lpalovsky approved these changes Jun 12, 2024

View reviewed changes

lpalovsky merged commit c48019e into SUSE:main Jun 12, 2024
4 checks passed

Export ansible output by ansible playbook for qesap framework

775dd2c

mpagot mentioned this pull request Oct 8, 2024

Qesap log ansible filename #281

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export ansible output to a file #234

Export ansible output to a file #234

hadeskun commented May 27, 2024 •

edited

Loading

mpagot Jun 6, 2024

mpagot Jun 6, 2024

hadeskun Jun 10, 2024

mpagot Jun 6, 2024

hadeskun Jun 10, 2024

mpagot Jun 12, 2024

mpagot left a comment

mpagot Jun 10, 2024 •

edited

Loading

hadeskun Jun 11, 2024

mpagot Jun 10, 2024

mpagot Jun 10, 2024

mpagot Jun 10, 2024

hadeskun Jun 12, 2024

mpagot Jun 12, 2024

mpagot left a comment

mpagot Jun 12, 2024

mpagot Jun 12, 2024

hadeskun Jun 12, 2024

mpagot Jun 12, 2024 •

edited

Loading

mpagot Jun 12, 2024

hadeskun Jun 12, 2024

mpagot Jun 12, 2024

mpagot Jun 12, 2024

mpagot Jun 12, 2024

lpalovsky left a comment

		log.debug("Ansible process return ret:%d", ret)
		playbook_path = command['cmd'][4]

	[[ $ansible_logs_number -eq 4 ]] \|\| test_die "ansible .log.txt are not 3 files but has ${ansible_logs_number}"
	# 3 playbooks plus a log file for the initial ansible (not ansible-playbook) call
	[[ $ansible_logs_number -eq 4 ]] \|\| test_die "ansible .log.txt are not 4 files but has ${ansible_logs_number}"

	This test checks if the ansible execution generates log files
	Given an ansible-playbook command line and some string to simulate an ansible stdout, this test verify that the function write them to file.

-def export_ansible_output(command, out):
+def export_ansible_output(command, out):
+    """
+    Utility function that get the ansible command line and the command output.
+    Function calculate the log name by extracting the ansible playbook name from the command line.
+    Function take the content of out and write it to a file in the current directory
+    """

	'testAll', '-a', '--ssh-extra-args="-l cloudadmin -o UpdateHostKeys=yes -o StrictHostKeyChecking=accept-new"']}
	'/some/immaginary/path/testAll.yaml', '-a', '--ssh-extra-args="-l cloudadmin -o UpdateHostKeys=yes -o StrictHostKeyChecking=accept-new"']}

		log = logging.getLogger(__name__)


		def test_export_ansible_output():

Export ansible output to a file #234

Export ansible output to a file #234

Conversation

hadeskun commented May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpagot left a comment

Choose a reason for hiding this comment

mpagot Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpagot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpagot Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpalovsky left a comment

Choose a reason for hiding this comment

hadeskun commented May 27, 2024 •

edited

Loading

mpagot Jun 10, 2024 •

edited

Loading

mpagot Jun 12, 2024 •

edited

Loading