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.

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

Fixed the Customized Notification returning incorrect values for host_status_counts #9251

Conversation

Saurabh-Thakre
Copy link
Contributor

@Saurabh-Thakre Saurabh-Thakre commented Feb 6, 2021

Fixed the customized notification returning incorrect values for host_status_counts. Summarizing the issue in the following

SUMMARY

When running a Job Template with a customized notification associated with it, it is returning incorrect values for host_status_counts in SLACK channel. I could see that in the code that we are taking the values for host_status_counts from job_host_summaries for the first rack than taking it from the /api/v2/jobs/ID.

https://github.com/ansible/tower/blob/370440f63d51f424009665edfd35b10322ddbd06/awx/main/models/notifications.py#L387

The values shows in /api/v2/jobs/ID look perfectly correct to me than what shows in /api/v2/jobs/ID/job_host_summaries. In the job_host_summaries we calculate the host_status_counts values for dark,ok, processed, etc. individually for different hosts. And right now we are taking host_status_counts from the first rack of the job_host_summaries rather than just taking it from /api/v2/jobs/ID host_status_counts which is calculated correctly. Looking at the job_host_summaries API output I think we are taking the values from the first rack of job_host_summaries

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • API
AWX VERSION
* Tower version: 3.8.1
ADDITIONAL INFORMATION

With the PR, we could see the correct values are getting received in SLACK notifications.

slack_notification

Following data is from /api/v2/jobs/185 for host_status_counts
host_status_counts


@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@chrismeyersfsu
Copy link
Member

Looks like some tests need to be updated

2021-02-06 16:27:41.237095 | static | FAILED awx/main/tests/functional/models/test_notifications.py::TestJobNotificationMixin::test_context[InventoryUpdate]
2021-02-06 16:27:41.237200 | static | FAILED awx/main/tests/functional/models/test_notifications.py::TestJobNotificationMixin::test_context[SystemJob]
2021-02-06 16:27:41.237300 | static | FAILED awx/main/tests/functional/models/test_notifications.py::TestJobNotificationMixin::test_context[WorkflowJob]

you can run those tests individually in the dev environment:

source /var/lib/awx/venv/awx/bin/activate
py.test /awx_devel/awx/main/tests/functional/models/test_notifications.py

@Saurabh-Thakre
Copy link
Contributor Author

Looks like some tests need to be updated

2021-02-06 16:27:41.237095 | static | FAILED awx/main/tests/functional/models/test_notifications.py::TestJobNotificationMixin::test_context[InventoryUpdate]
2021-02-06 16:27:41.237200 | static | FAILED awx/main/tests/functional/models/test_notifications.py::TestJobNotificationMixin::test_context[SystemJob]
2021-02-06 16:27:41.237300 | static | FAILED awx/main/tests/functional/models/test_notifications.py::TestJobNotificationMixin::test_context[WorkflowJob]

you can run those tests individually in the dev environment:

source /var/lib/awx/venv/awx/bin/activate
py.test /awx_devel/awx/main/tests/functional/models/test_notifications.py

Sure, I'm on it.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@chrismeyersfsu
Copy link
Member

Looks like I was wrong about the if condition. Without it, an inventory update notification will fail on the line events = qs.only('event_data').filter(event='playbook_on_stats')

2021-02-09 12:16:47.269972 | static | ____________ TestJobNotificationMixin.test_context[InventoryUpdate] ____________
2021-02-09 12:16:47.270026 | static | [gw0] linux -- Python 3.6.8 /var/lib/awx/venv/awx/bin/python3
2021-02-09 12:16:47.270059 | static | Traceback (most recent call last):
2021-02-09 12:16:47.270141 | static |   File "/awx_devel/awx/main/tests/functional/models/test_notifications.py", line 123, in test_context
2021-02-09 12:16:47.270188 | static |     context = job.context(job_serialization)
2021-02-09 12:16:47.270249 | static |   File "/awx_devel/awx/main/models/notifications.py", line 387, in context
2021-02-09 12:16:47.270307 | static |     events = qs.only('event_data').filter(event='playbook_on_stats')
2021-02-09 12:16:47.270491 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/query.py", line 892, in filter
2021-02-09 12:16:47.270555 | static |     return self._filter_or_exclude(False, *args, **kwargs)
2021-02-09 12:16:47.270649 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/query.py", line 910, in _filter_or_exclude
2021-02-09 12:16:47.270692 | static |     clone.query.add_q(Q(*args, **kwargs))
2021-02-09 12:16:47.270779 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1290, in add_q
2021-02-09 12:16:47.270836 | static |     clause, _ = self._add_q(q_object, self.used_aliases)
2021-02-09 12:16:47.270924 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1318, in _add_q
2021-02-09 12:16:47.270968 | static |     split_subq=split_subq, simple_col=simple_col,
2021-02-09 12:16:47.271059 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1190, in build_filter
2021-02-09 12:16:47.271116 | static |     lookups, parts, reffed_expression = self.solve_lookup_type(arg)
2021-02-09 12:16:47.271220 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1049, in solve_lookup_type
2021-02-09 12:16:47.271291 | static |     _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta())
2021-02-09 12:16:47.271393 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1420, in names_to_path
2021-02-09 12:16:47.271454 | static |     "Choices are: %s" % (name, ", ".join(available)))
2021-02-09 12:16:47.271646 | static | django.core.exceptions.FieldError: Cannot resolve keyword 'event' into field. Choices are: counter, created, end_line, event_data, id, inventory_update, inventory_update_id, modified, start_line, stdout, uuid, verbosity

@Saurabh-Thakre
Copy link
Contributor Author

Looks like I was wrong about the if condition. Without it, an inventory update notification will fail on the line events = qs.only('event_data').filter(event='playbook_on_stats')

2021-02-09 12:16:47.269972 | static | ____________ TestJobNotificationMixin.test_context[InventoryUpdate] ____________
2021-02-09 12:16:47.270026 | static | [gw0] linux -- Python 3.6.8 /var/lib/awx/venv/awx/bin/python3
2021-02-09 12:16:47.270059 | static | Traceback (most recent call last):
2021-02-09 12:16:47.270141 | static |   File "/awx_devel/awx/main/tests/functional/models/test_notifications.py", line 123, in test_context
2021-02-09 12:16:47.270188 | static |     context = job.context(job_serialization)
2021-02-09 12:16:47.270249 | static |   File "/awx_devel/awx/main/models/notifications.py", line 387, in context
2021-02-09 12:16:47.270307 | static |     events = qs.only('event_data').filter(event='playbook_on_stats')
2021-02-09 12:16:47.270491 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/query.py", line 892, in filter
2021-02-09 12:16:47.270555 | static |     return self._filter_or_exclude(False, *args, **kwargs)
2021-02-09 12:16:47.270649 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/query.py", line 910, in _filter_or_exclude
2021-02-09 12:16:47.270692 | static |     clone.query.add_q(Q(*args, **kwargs))
2021-02-09 12:16:47.270779 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1290, in add_q
2021-02-09 12:16:47.270836 | static |     clause, _ = self._add_q(q_object, self.used_aliases)
2021-02-09 12:16:47.270924 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1318, in _add_q
2021-02-09 12:16:47.270968 | static |     split_subq=split_subq, simple_col=simple_col,
2021-02-09 12:16:47.271059 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1190, in build_filter
2021-02-09 12:16:47.271116 | static |     lookups, parts, reffed_expression = self.solve_lookup_type(arg)
2021-02-09 12:16:47.271220 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1049, in solve_lookup_type
2021-02-09 12:16:47.271291 | static |     _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta())
2021-02-09 12:16:47.271393 | static |   File "/var/lib/awx/venv/awx/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1420, in names_to_path
2021-02-09 12:16:47.271454 | static |     "Choices are: %s" % (name, ", ".join(available)))
2021-02-09 12:16:47.271646 | static | django.core.exceptions.FieldError: Cannot resolve keyword 'event' into field. Choices are: counter, created, end_line, event_data, id, inventory_update, inventory_update_id, modified, start_line, stdout, uuid, verbosity

I don't think it is failing because we removed the if condition. It because the InventoryUpdate is not aware of event . We may need to filter those out, the InventoryUpdate and WorkflowJob ??

@chrismeyersfsu
Copy link
Member

chrismeyersfsu commented Feb 10, 2021

try:
  has_event_property = any([f for f in self.event_class._meta.fields if f.name == 'event'])
except NotImplementedError:
  has_event_property = False

if has_event_property:
  ...

Below is how this would play out for the various Job types.
image

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@chrismeyersfsu
Copy link
Member

recheck

@chrismeyersfsu
Copy link
Member

regate

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Fixed the customized notification returning incorrect values for host_status_counts

Update notifications.py

Removed if condition

Added exception handling

A nitpick
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@chrismeyersfsu
Copy link
Member

(closed and reopened to get zuul working)

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed (gate pipeline). For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@chrismeyersfsu
Copy link
Member

regate

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 182a7e8 into ansible:devel Feb 19, 2021
@Saurabh-Thakre Saurabh-Thakre deleted the Saurabh-Thakre-patch-1 branch February 19, 2021 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants