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

Improve the agent_files_deletion test #2296

Merged
merged 9 commits into from
Jan 11, 2022

Conversation

yanazaeva
Copy link
Contributor

Related issue
#2282

Description

This closes #2282. In this PR we are adding two sleep functions in different places in order to:

  • allow the agent properly reconnect with the manager after the restart.
  • give some time to the synchronization in the cluster.

AdriiiPRodri
AdriiiPRodri previously approved these changes Dec 7, 2021
Copy link
Member

@Rebits Rebits left a comment

Choose a reason for hiding this comment

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

GJ, but some changes are required.

@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch from bca0a10 to 4b5d228 Compare December 10, 2021 18:21
@yanazaeva
Copy link
Contributor Author

After @Rebits review, the first sleep set in order to wait for the agent to properly restart and reconnect was replaced by a fixture to monitor the agent enrollment event in the ossec.log:

HostMonitor(inventory_path=inventory_path, messages_path=messages_path, tmp_path=tmp_path).run()

However, the other two sleep functions present in the code are necessary as it seems that there is no log registering the addition of information to the global.db database. This addition takes almost 50 seconds, so the time to sync is set to 60. Also, the first of these sleep functions were moved right after the agent restarted, which seems to be a more suitable place. The second one remains where it was originally.

Lastly, two lines were added at the beginning of the test to clean the old ossec.log and cluster.log before every execution.

Copy link
Member

@davidjiglesias davidjiglesias left a comment

Choose a reason for hiding this comment

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

'However, the other two sleep functions present in the code are necessary as it seems that there is no log registering the addition of information to the global.db database. This addition takes almost 50 seconds, so the time to sync is set to 60'

The addition of the agent information to global.db can be found in ossec.log with debug 2 for modulesd:

wazuh-modulesd:database[21279] wm_database.c:279 at wm_sync_agents(): DEBUG: Synchronizing agent 38990 '1-5srgA8E40i9Wj3tz-debian10'.

I suggest adding a new HostMonitor to detect this addition in the master node. Also, 50 seconds seems to be a lot of time for an already connected agent to appear in global.db.

@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch 2 times, most recently from a2871a6 to 3cdf73a Compare December 21, 2021 16:53
@yanazaeva
Copy link
Contributor Author

Modified the test according to the feedback and explained all the changes here.

@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch from 3cdf73a to d1e0a10 Compare December 22, 2021 10:53
@yanazaeva yanazaeva changed the title Add sleep function to files_deletion test Improve the agent_files_deletion test Dec 22, 2021
davidjiglesias
davidjiglesias previously approved these changes Dec 22, 2021
@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch 3 times, most recently from 03237e5 to 8bd17a4 Compare December 22, 2021 15:06
Rebits
Rebits previously approved these changes Dec 22, 2021
Copy link
Member

@Rebits Rebits left a comment

Choose a reason for hiding this comment

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

LGTM

@yanazaeva yanazaeva requested a review from Rebits December 27, 2021 08:09
davidjiglesias
davidjiglesias previously approved these changes Dec 28, 2021
@snaow
Copy link
Contributor

snaow commented Dec 29, 2021

Hi team, some comments after my review:

@yanazaeva yanazaeva dismissed stale reviews from davidjiglesias and Selutario via c2eba34 December 30, 2021 10:42
@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch 2 times, most recently from 9e43f99 to 1f9deb5 Compare December 30, 2021 14:20
@yanazaeva
Copy link
Contributor Author

Added all the changes proposed by @snaow.

Test result

Results
R1 🟢
R2 🟢
R3 🟢
R4 🟢
R5 🟢

@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch from 1f9deb5 to 95a2b6b Compare December 31, 2021 09:26
@yanazaeva yanazaeva requested a review from Selutario December 31, 2021 09:39
Selutario
Selutario previously approved these changes Dec 31, 2021
@yanazaeva
Copy link
Contributor Author

These are the test results after applying the changes proposed by @juliamagan:

Results
R1 🟢
R2 🟢
R3 🟢

@yanazaeva yanazaeva force-pushed the feature/2282-add-more-time-for-sync branch from 3c6e20c to 0f14e59 Compare January 5, 2022 15:27
response = host_manager.make_api_call(host=master_host, method='GET', token=master_token,
endpoint='/agents?status=active')
assert response['status'] == 200, 'Failed when trying to get the active agents'
if int(response['json']['data']['total_affected_items']) == 4:
Copy link
Member

Choose a reason for hiding this comment

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

We should check that the expected agent is active, not that there are 4 total_affected_items

Comment on lines +49 to +50
elif time() > timeout:
print("The agent 'wazuh-agent3' is not 'Active' yet.")
Copy link
Member

Choose a reason for hiding this comment

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

We are not exiting the while, so when we can't find the wazuh-agent3 active, the test will keep printing The agent 'wazuh-agent3' is not 'Active' yet. forever.

@yanazaeva
Copy link
Contributor Author

yanazaeva commented Jan 10, 2022

Added @juliamagan's changes. Tests results after modifications:

Results
R1 🟢
R2 🟢
R3 🟢

@yanazaeva yanazaeva requested a review from juliamagan January 10, 2022 10:04
@juliamagan
Copy link
Member

juliamagan commented Jan 10, 2022

Testing

Wazuh branch Wazuh-QA branch
v4.3.0-rc2 feature/2282-add-more-time-for-sync
Results
R1 🟢
R2 🟢
R3 🟢

@snaow snaow merged commit 8df834b into master Jan 11, 2022
@snaow snaow deleted the feature/2282-add-more-time-for-sync branch January 11, 2022 09:37
@snaow snaow mentioned this pull request Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate the error in the agent_files_deletion test
7 participants