Merge qe_common role #123
Merge qe_common role #123
Conversation
| - name: Get Endpoint | ||
| ansible.builtin.shell: | ||
| cmd: | | ||
| oc project openstack |
There was a problem hiding this comment.
oc rsh openstackclient ... does the same as kubectl exec.
You don't need to do -- to separate the command
You can pass the -n flag to tell it which namespace to run in
roles/qe_common/tasks/cred_tests.yml
Outdated
| cmd: | | ||
| oc get crd "{{ item }}" | ||
| register: output | ||
| failed_when: output.rc != 0 |
There was a problem hiding this comment.
This failure condition is implicit for the shell module, so it's usually omitted.
roles/qe_common/tasks/main.yml
Outdated
| @@ -0,0 +1,64 @@ | |||
| --- | |||
| - when: container_list is defined | |||
| name: "Verify container - {{ container_polar_id}}" | |||
There was a problem hiding this comment.
Please change the name of this var to container_test_id, rather than container_polar_id, since it's more intuitive, rather than needing to know that polar is short for Polarian and what Polarian is.
There was a problem hiding this comment.
Also, please add in defaults for these values in roles/qe_common/defaults/main.yml.
This is so that the test ID is optional, since you might want to use this for some pre-checks, without having a test_id.
roles/qe_common/tasks/main.yml
Outdated
| loop: "{{ proj_list }}" | ||
|
|
||
|
|
||
| - when: |
There was a problem hiding this comment.
| - when: | |
| - when: |
.ansible-lint
Outdated
| @@ -2,6 +2,8 @@ | |||
| exclude_paths: | |||
| - ci/ | |||
| - roles/telemetry_autoscaling | |||
| - roles/telemetry_logging | |||
| - roles/qe_common | |||
There was a problem hiding this comment.
Please try not to skip linting this role.
I ran ansible-lint locally, and found some errors that prevent the roles from running
There was a problem hiding this comment.
ansible-lint is really useful for catching small, hard to see errors in syntax or layout, as well as being able to provide some additional feedback that keeps the content easier to grok and maintain
There was a problem hiding this comment.
FYI: If you un-skip the "command-instead-of-module" check in .ansible-lint, it gives you suggestions for what modules could be used instead of shell for certain commands
.ansible-lint
Outdated
| @@ -2,6 +2,8 @@ | |||
| exclude_paths: | |||
| - ci/ | |||
| - roles/telemetry_autoscaling | |||
| - roles/telemetry_logging | |||
| - roles/qe_common | |||
There was a problem hiding this comment.
| - roles/qe_common |
roles/qe_common/tasks/main.yml
Outdated
| @@ -0,0 +1,64 @@ | |||
| --- | |||
| - when: container_list is defined | |||
| name: "Verify container - {{ container_polar_id}}" | |||
There was a problem hiding this comment.
https://ansible.readthedocs.io/projects/lint/rules/jinja/
jinja[spacing]: Jinja2 spacing could be improved: Verify container - {{ container_polar_id}} -> Verify container - {{ container_polar_id }} (warning)
roles/qe_common/tasks/main.yml
Outdated
| - when: node_list is defined | ||
| name: "Verify OSP node - {{ node_polar_id }}" | ||
| ansible.builtin.include_tasks: node_tests.yml | ||
| loop: "{{ node_list }}" |
There was a problem hiding this comment.
key-order[task]: You can improve the task key order to: name, when, ansible.builtin.include_tasks, loop
roles/qe_common/tasks/main.yml:32 Task/Handler: Verify OSP node - {{ node_polar_id }}
https://ansible.readthedocs.io/projects/lint/rules/key-order/
(Same for the other tasks in this file
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/838cb4e2e4644f569ca1f922fbe31a82 ❌ openstack-k8s-operators-content-provider FAILURE in 6m 06s |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/2fed2e1b296143c698fc94427414a526 ✔️ feature-verification-tests-noop SUCCESS in 5s |
This failure is unrelated to this change. |
|
|
||
| For subscription_tests.yml tasks: | ||
|
|
||
| subscription_polar_id |
There was a problem hiding this comment.
These var names need to be updated to match the updated var names (*_test_id)
| Example Playbook | ||
| ---------------- | ||
|
|
||
| Typically, for this role the tests should *not* use a "main.yml" and import or include all the tests in the role. On the contrary, a tests should explicitly include specific tests needed for a given job. |
There was a problem hiding this comment.
This can be removed.
The behaviour has changed to work by setting appropriate vars to select the tests that run. The vars are set and then used by the role on import.
Ideally, the vars would have the <role_name>_ prefix to them
| hosts: controller | ||
| gather_facts: no | ||
| vars: | ||
| proj_out_file: verify_logging_projects_exist_lresults.log |
There was a problem hiding this comment.
The project_out_file is not mentioned elsewhere.
| oc project openstack | ||
| kubectl exec openstackclient -- openstack endpoint list --service="{{ item[0] }}" --service="{{ item[1] }}" --interface="{{ item[2] }}" |
There was a problem hiding this comment.
You can use oc rsh openstackclient openstack endpoint list ....
the -n openstack should be included to specify the project/namespace
There was a problem hiding this comment.
What happens if the endpoint doesn't exist? Does the openstackclient return a non-zero return code?
| - name: Get container status | ||
| ansible.builtin.shell: | ||
| cmd: | | ||
| podman ps -a --format "{{ '{{.Names}} {{.Status}}' }}" | grep "{{ item }}" | awk '{print $2;}' |
There was a problem hiding this comment.
For readability, I recommend using something instead of item.
You can use a useful variable name, and tell the loop in main to use that same var name for its loop var.
The syntax would be similar to: https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_loops.html#stacking-loops-via-include-tasks, but without the nested loops.
I would suggest container_name as the loop var here, so that reading this task is easier.
Here's an example playbook:
---
- hosts: localhost
tasks:
- ansible.builtin.debug:
msg: "{{ item }}"
loop:
- "first"
- "second"
- ansible.builtin.debug:
msg: "{{ my_var }}"
loop:
- "third"
- "fourth"
loop_control:
loop_var: my_var
New readme file for role
new playbook
duplicate file
added service test file
added for easy of use
fixed typo
removed redundant results file generation
remove redundant results file generation
remove redundant results file generation.
remove redundant results file generation.
remove redundant results file generation.
remove redundant results file generation.
remove redundant results file generation
remove redundant results file generation
remove redundant results file generation
remove redundant results file generation
updated test_id var name
updated role name
moved role to new dir
initial file
qe_common role name change to common
test id var change
remove qe_common role dir
roles/qe_common name change to roles/common
qe_common role rename to common
remove unneeded line
remove unneeded line
remove unneeded line
remove unneeded line
remove unneeded line
lint changes
elfiesmelfie
force-pushed
the
alexy_logging2
branch
from
7f6cb87
to
6e3283c
Compare
| - roles/telemetry_logging | ||
| - roles/commmon |
There was a problem hiding this comment.
| - roles/telemetry_logging | |
| - roles/commmon |
| hosts: controller | ||
| gather_facts: no | ||
| vars: | ||
| proj_out_file: verify_logging_projects_exist_lresults.log |
There was a problem hiding this comment.
| proj_out_file: verify_logging_projects_exist_lresults.log |
|
|
||
| For subscription_tests.yml tasks: | ||
|
|
||
| subscription_polar_id |
There was a problem hiding this comment.
| subscription_polar_id | |
| subscription_test_id |
| Example Playbook | ||
| ---------------- | ||
|
|
||
| Typically, for this role the tests should *not* use a "main.yml" and import or include all the tests in the role. On the contrary, a tests should explicitly include specific tests needed for a given job. |
There was a problem hiding this comment.
| Typically, for this role the tests should *not* use a "main.yml" and import or include all the tests in the role. On the contrary, a tests should explicitly include specific tests needed for a given job. |
roles/telemetry_logging/README.md
Outdated
| @@ -0,0 +1,53 @@ | |||
| telemetry_logging | |||
| ========= | |||
There was a problem hiding this comment.
| ========= | |
| ================= |
roles/telemetry_logging/README.md
Outdated
| For journal_tests.yml | ||
|
|
||
| identifiers_test_id | ||
| - polarion id for test |
There was a problem hiding this comment.
Pleas update this description
| ansible.builtin.shell: | ||
| cmd: | ||
| tstamp=$(date -d '30 minute ago' "+%Y-%m-%d %H:%M:%S") | ||
| journalctl -t "{{ item }}" --no-pager -S "${tstamp}" | wc -l |
There was a problem hiding this comment.
You could convert this to a command if you want, by removing the use of wc and making use of journal_output.stdout_lines to count the number of returned lines. This would also make debugging easier, since using verbose output would let you see what was returned from the journalctl command, rather than just seeing a number.
The failed_when condition could become ``journal_wc.stdout_lines | length <= 1
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/6fb59fd84b444f8e82a02eaf41d8cc6a ✔️ feature-verification-tests-noop SUCCESS in 4s |
| @@ -0,0 +1,119 @@ | |||
| common | |||
| ========= | |||
There was a problem hiding this comment.
| ========= | |
| ====== |
| cmd: | | ||
| oc get crd "{{ item }}" | ||
| changed_when: false | ||
| register: output |
There was a problem hiding this comment.
Is there a fail condition for this? Does it need one?
| oc project openstack | ||
| kubectl exec openstackclient -- openstack endpoint list --service="{{ item[0] }}" --service="{{ item[1] }}" --interface="{{ item[2] }}" |
There was a problem hiding this comment.
What happens if the endpoint doesn't exist? Does the openstackclient return a non-zero return code?
roles/common/tasks/node_tests.yml
Outdated
There was a problem hiding this comment.
I don't believe these tests are valid.
Zuul doesn't know or care that the nodes are VMs, and the executer, which run the playbooks for the job, does not have these VMs running.
We pass nodesets into the jobs, but these are for zuul to give to nodepool, which provides the test servers.
In the case of the Zuul instance on rdo, nodepool talks to various cloud providers, which then provision VMs, based on the nodeset labels.
The only information that Zuul has/needs about the hosts (crc, computes, controller, etc) is the ansible inv file, which has the hostname, IP address, etc that zuul needs to run the playbooks that we pass to it.
| - name: Verify Project exists - "{{ item }}" | ||
| ansible.builtin.shell: | ||
| cmd: | | ||
| oc project "{{ item }}" |
There was a problem hiding this comment.
This will switch to a different project. This may cause unexpected behaviour of subsequent commands are run without a --namespace/-n argument.
A better check would be
oc project list | grep "{{ item}}"
(please verify that this command is correct)
A more efficient approach might be to get the project list and then loop through the output to make sure that all the expected project are there.
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/74a17349039442d8a6732bc2f65be016 ✔️ feature-verification-tests-noop SUCCESS in 4s |
| - name: Verify subscription | ||
| ansible.builtin.shell: | ||
| cmd: | | ||
| oc get subscriptions -n "{{ subscription_nspace }}" "{{ item }}" |
There was a problem hiding this comment.
IMHO, this should have a similar format to the endpoint tests, so that a single loop can cover multiple namespaces.
| - name: Get Pod Instance "{{ pod_status_str }}" | ||
| ansible.builtin.shell: | ||
| cmd: | | ||
| oc get pods -n "{{ pod_nspace }}" | grep "{{ item }}" | grep "{{ pod_status_str }}" | awk '{print $1;}' |
There was a problem hiding this comment.
If this fails, there is no indication until the next task.
There is also no helpful information to see what might have caused the error.
The next task will fail if there is nothing returned from here.
| oc get pods -n "{{ pod_nspace }}" | grep "{{ item }}" | grep "{{ pod_status_str }}" | awk '{print $1;}' | |
| oc get pods -n "{{ pod_nspace }}" | grep "{{ item }}" | grep "{{ pod_status_str }}" | awk '{print $1;}' | |
| failed_when: | |
| - podinstance.stdout_lines | length == 0 |
| - name: Check terminated pod | ||
| ansible.builtin.shell: | ||
| cmd: | | ||
| oc get pod -n "{{ pod_nspace }} {{ podinstance.stdout }}" |
There was a problem hiding this comment.
| oc get pod -n "{{ pod_nspace }} {{ podinstance.stdout }}" | |
| oc get pod -n "{{ pod_nspace }}" "{{ podinstance.stdout }}" |
Please don't apply these suggestions, they are reflective of what is going on in PR#149
Add service tests to the logging job adding the tasks from #123 into a spearate PR
Merge QE Common role into master