-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat(sensor_download): adds lock files to prevent collision when downloading similar sensors #569
Merged
carlosmmatos
merged 14 commits into
CrowdStrike:main
from
carlosmmatos:add-lock-mechanism-sensor-download
Oct 23, 2024
Merged
feat(sensor_download): adds lock files to prevent collision when downloading similar sensors #569
carlosmmatos
merged 14 commits into
CrowdStrike:main
from
carlosmmatos:add-lock-mechanism-sensor-download
Oct 23, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes CrowdStrike#567 This PR fixes potential speed issues when working with large amounts of hosts because for each host, it used to create a temp directory and download the sensor. This of course is not very efficient, thus by specifying a default such as /tmp - this should speed up download times as hosts with the same sensor will not have to re-download every time.
This fixes an issue that will cause operations like copying the sensor to create a new temp directory on the target host everytime it runs. For example, in scenarios where something happens before the sensor is cleaned, then running the role again will cause a new directory and another full copy operation instead of checking to see if the sensor is already copied.
…ision This feature allows multiple hosts downloading the same file to not overstep on each other as was the case before. This route improves overall download times as the module will only download a sensor at most once (assuming uniqueness) while other systems wait.
TBD - consider making this an option in the future?
carlosmmatos
added
bugfixes
Bug fixes or minor enhancements
enhancement
New feature or request
minor_changes
New features, like plugin or module options
labels
Oct 22, 2024
The win_file module does not return a path attribute. Instead of using path, we'll just the variable instead as this is the path.
To prevent thundering herd!
windows sucks... nuff said :(
Explain that a 0-byte lock file is created and can be safely deleted
The refactoring was not needed and this makes more sense from an organizational perspective.
evanstoner
approved these changes
Oct 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bugfixes
Bug fixes or minor enhancements
enhancement
New feature or request
minor_changes
New features, like plugin or module options
ok-to-test
Run tests
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #567
This feature allows multiple hosts downloading the same file to not overstep on each other as was the case before. This route improves overall download times as the module will only download a sensor at most once (assuming uniqueness) while other systems wait.
On top of that - added sane defaults (#567) for download directories to ensure we aren't creating unnecessary directories on multiple runs when a failure might occur.
The reason for the module updates is that after additional testing when implementing the same dest dir - I noticed that each system was trying to download the same file at the same time, causing no visible improvement in time for instances where there are a large amount of hosts.