-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add service tests to the logging job #153
base: master
Are you sure you want to change the base?
Conversation
Add service tests to the logging job adding the tasks from #123 into a spearate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As well as the comments inline, can you update the zuul job to run this? (based on #149)
@@ -0,0 +1,13 @@ | |||
--- | |||
- name: "[TEST] verify container status" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TEST prefix isn't chosen yet, so this may change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prefix will be "TEST"
ansible.builtin.shell: | ||
cmd: | | ||
podman ps -a --format "{{ '{{.Names}} {{.Status}}' }}" | grep "{{ container_name }}" | awk '{print $2}' | ||
changed_when: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a failed_when, if you're going to mark it with a TEST prefix, otherwise, it'll pass as long as the shell cmd returns something.
Alternatively, don't mark this task as a test, but mark the following task as a test.
Currently, we rely on the custom_logger to produce a file that gathers the test-ids=passed|failed output, so there should be a test ID in the second line of the task name.
Hopefully, we can remove this at some point soon, and add a check to report_result that checks the junit output instead.
- name: Fail if container is not 'Up' | ||
ansible.builtin.fail: | ||
msg: "Container '{{ container_name }}' is not in 'Up' status. Current status: {{ container_status.stdout }}" | ||
when: | ||
- "'Up' not in container_status.stdout" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, it makes it clear why the test failed.
However, this task should be the one one with TEST in the name.
Requirements | ||
------------ | ||
|
||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
podman is required
Role Variables | ||
-------------- | ||
Variable required for all tasks to run: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should only be adding the container tests
@@ -0,0 +1,13 @@ | |||
--- | |||
- name: "[TEST] verify container status" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prefix will be "TEST"
vars: | ||
container_name: "{{ item }}" | ||
|
||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--- |
# All containers need to be up in order for this task to pass | ||
- name: "[TEST] verify status of multiple containers" | ||
ansible.builtin.include_tasks: | ||
file: container_status.yml | ||
loop: "{{ containers }}" | ||
loop_control: | ||
label: "{{ item }}" | ||
vars: | ||
container_name: "{{ item }}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is separate from adding the service checks.
It should NOT be run unconditionally
container_name: "{{ item }}" | ||
|
||
--- | ||
- name: "[TEST] Verify service for logging" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a conditional here, so that the service_tests.yml will only be included when the right vars are defined
adding the tasks from #123 into a spearate PR
https://issues.redhat.com/browse/OSPRH-9659
Depends-On: openstack-k8s-operators/openstack-operator#1115