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

Assertion Error, time delta/format in 'wait_for_sync()', of capsule content 'last_sync_time' #14066

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

damoore044
Copy link
Contributor

@damoore044 damoore044 commented Feb 13, 2024

Problem Statement

4 Failures in recent runs of API::CapsuleContent, for AssertionError:

tests/foreman/api/test_capsulecontent.py:190: in test_positive_checksum_sync
    module_capsule_configured.wait_for_sync()
robottelo/host_helpers/capsule_mixins.py:75: in wait_for_sync
    or datetime.strptime(sync_status['last_sync_time'], '%Y-%m-%d %H:%M:%S UTC')
E   AssertionError

Locally I can see the last_sync_time format from logs as:

nailgun.client - DEBUG - Received HTTP 200 response: {"last_sync_time":"2024-02-13 15:05:10 UTC", ...

Solution

  • Split the assertions checking last_sync_time (check if None /Falsey first), then checking length of any in-progress sync tasks, avoiding possibly ambiguous assertion- where last_sync_time has not updated with a recent task, and is still an old sync time/None.
    The prior Assertions with AND, OR, etc fail without showing the compared values. Provided error messages with assertion failure details.
  • Apply a one second margin of safety to start_time, due to rounding. Often the assertion fails for one second delta, which is start time recorded and sync done at pretty much the same time, but the seconds from sync_status were rounded to one second before start_time. This is inconsistent, depending on the milliseconds when last_sync_time is recorded.
  • From dateutil.parser, use parse(date-time string) method on the start_time, and any found last_sync_time. So when checking the last sync time is newer, parse will account for different formats of the same time. Similar solution to PR Fix Date assertion failure in CI errata:e2e  #13121

Related Issue >> From 'sat-6.15-rhel8-Capsule-Content'

4 Failed api scenarios due to the AssertionError in this method. Inconsistently, others will fail locally for the same error.
6.14.z and 6.15.0 (most recent Build 13, Mar 7 2024, 12:43 AM)

PRT Case

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@damoore044 damoore044 requested a review from a team as a code owner February 13, 2024 17:46
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@damoore044 damoore044 added CherryPick PR needs CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Feb 13, 2024
@damoore044 damoore044 requested a review from a team February 13, 2024 17:47
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5706
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement
Test Result : =========== 6 failed, 15 passed, 1357 warnings in 6107.27s (1:41:47) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Feb 13, 2024
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@damoore044 damoore044 changed the title Assertion Error, datetime format in 'wait_for_sync()', of capsule content 'last_sync_time' Assertion Error, time delta/format in 'wait_for_sync()', of capsule content 'last_sync_time' Feb 13, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5708
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement
Test Result : =========== 2 failed, 19 passed, 1404 warnings in 5961.97s (1:39:21) ===========

@damoore044 damoore044 force-pushed the wait_for_sync_fix branch 3 times, most recently from 940efbe to 1b83c35 Compare February 14, 2024 02:18
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5709
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement
Test Result : =========== 2 failed, 19 passed, 1423 warnings in 6162.18s (1:42:42) ===========

Copy link
Contributor

@lhellebr lhellebr left a comment

Choose a reason for hiding this comment

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

Left two comments, those things seem weird to me.
Conceptually this LGTM.
PRT needs to pass.

robottelo/host_helpers/capsule_mixins.py Outdated Show resolved Hide resolved
robottelo/host_helpers/capsule_mixins.py Show resolved Hide resolved
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5719
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement
Test Result : =========== 2 failed, 19 passed, 1390 warnings in 5992.92s (1:39:52) ===========

@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5727
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement
Test Result : ================ 21 passed, 1521 warnings in 6806.82s (1:53:26) ================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Feb 14, 2024
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@damoore044 damoore044 removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Mar 11, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6018
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement --external-logging
Test Result : ========== 20 passed, 1 skipped, 1398 warnings in 5808.92s (1:36:48) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Mar 11, 2024
@damoore044 damoore044 force-pushed the wait_for_sync_fix branch 2 times, most recently from c50991a to 17a1cd8 Compare March 11, 2024 21:27
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

1 similar comment
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@damoore044 damoore044 removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Mar 13, 2024
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

change to sca-only orgs, entitlement orgs are failing

assert capsule sync task(s) when invoked, and sync status

pre-commit fix, refactor wait_for_sync() into concise steps
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6051
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement --external-logging
Test Result : =========== 1 failed, 20 passed, 1428 warnings in 6410.09s (1:46:50) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Mar 13, 2024
@damoore044
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6056
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_capsulecontent.py::TestCapsuleContentManagement --external-logging
Test Result : ================ 21 passed, 1431 warnings in 5734.36s (1:35:34) ================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Mar 13, 2024
Copy link
Member

@jyejare jyejare left a comment

Choose a reason for hiding this comment

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

ACK , looks great but seems like we are verifying some things twice or more which is done by synchronous operation.

If you think its not, feel free to merge the PR.

)
# No failed or active tasks remaining
assert len(updated_status['last_failed_sync_tasks']) == 0
assert len(updated_status['active_sync_tasks']) == 0
Copy link
Member

Choose a reason for hiding this comment

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

I think we dont need to verify this as maybe
self.nailgun_capsule.content_get_sync(timeout=timeout, synchronous=True) being synchronus activity verifies that ?

Copy link
Member

Choose a reason for hiding this comment

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

And anything similar assertions we do above are not needed ?

Copy link
Contributor Author

@damoore044 damoore044 Mar 14, 2024

Choose a reason for hiding this comment

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

It appears content_get_sync does not run synchronously with the active sync task(s).
It seems to return any sync tasks in progress for the capsule, not matter if run synchronously w/ timeout or not.

Here, it is possible for a task to have been active, then finished just before querying content_get_sync. If that task failed and did so before querying, we would want to check that there are no failed tasks, otherwise we could have a recently failed sync that was not checked.

Copy link
Member

Choose a reason for hiding this comment

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

@damoore044 I think if synchronus is not behaving as it suppose to be we should raise a bug ! immediately !!

@vsedmik vsedmik merged commit c10982f into SatelliteQE:master Mar 14, 2024
11 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 14, 2024
…ontent 'last_sync_time' (#14066)

Assertion Error for datetime format of 'last sync time'

change to sca-only orgs, entitlement orgs are failing

assert capsule sync task(s) when invoked, and sync status

pre-commit fix, refactor wait_for_sync() into concise steps

(cherry picked from commit c10982f)
damoore044 added a commit that referenced this pull request Mar 19, 2024
…ontent 'last_sync_time' (#14066)

Assertion Error for datetime format of 'last sync time'

change to sca-only orgs, entitlement orgs are failing

assert capsule sync task(s) when invoked, and sync status

pre-commit fix, refactor wait_for_sync() into concise steps

(cherry picked from commit c10982f)
damoore044 added a commit that referenced this pull request Mar 19, 2024
…ontent 'last_sync_time' (#14066)

Assertion Error for datetime format of 'last sync time'

change to sca-only orgs, entitlement orgs are failing

assert capsule sync task(s) when invoked, and sync status

pre-commit fix, refactor wait_for_sync() into concise steps

(cherry picked from commit c10982f)
vsedmik pushed a commit that referenced this pull request Mar 19, 2024
… in 'wait_for_sync()' (#14412)

Assertion Error, time delta/format in 'wait_for_sync()', of capsule content 'last_sync_time' (#14066)

Assertion Error for datetime format of 'last sync time'

change to sca-only orgs, entitlement orgs are failing

assert capsule sync task(s) when invoked, and sync status

pre-commit fix, refactor wait_for_sync() into concise steps

(cherry picked from commit c10982f)

Co-authored-by: David Moore <[email protected]>
shweta83 pushed a commit to shweta83/robottelo that referenced this pull request Apr 10, 2024
…ontent 'last_sync_time' (SatelliteQE#14066)

Assertion Error for datetime format of 'last sync time'

change to sca-only orgs, entitlement orgs are failing

assert capsule sync task(s) when invoked, and sync status

pre-commit fix, refactor wait_for_sync() into concise steps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR Stream Introduced in or relating directly to Satellite Stream/Master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants