-
Notifications
You must be signed in to change notification settings - Fork 685
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
Remove machine pruning from gating #10453
Remove machine pruning from gating #10453
Conversation
A different way of mitigate what this is proposing: #10387 |
1dbe470
to
398ea18
Compare
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 testing farm job on CS8 and 9 fails due to #10450 and isn't related to this PR.
tests/test_rule_in_container.sh
Outdated
printf '\t%s\n' "<rule>: The short rule ID. Wildcards are supported." | ||
printf '\t%s\n' "-n, --name: Name of the test image (default: 'ssg_test_suite')" | ||
printf '\t%s\n' "-s, --scenarios: Regex to reduce selection of tested scenarios (no default)" | ||
printf '\t%s\n' "-d, --datastream: Path to the datastream to use in tests. Autodetected by default. (no default)" | ||
printf '\t%s\n' "-r, --remediate-using: What to remediate with. Can be one of: 'oscap', 'bash' and 'ansible' (default: 'oscap')" | ||
printf '\t%s\n' "-l, --logdir: Directory where logs will be stored (no default)" | ||
printf '\t%s\n' "--dontclean, --no-dontclean: Dont remove HTML reports from the log directory. (off by default)" | ||
printf '\t%s\n' "--dontclean, --no-dontclean: Don't remove HTML reports from the log directory. (off by default)" | ||
printf '\t%s\n' "--keep-machine-platform, --no-keep-machine-platform: Don't remove machine platforms. (off by default)" |
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 can confuse someone. Does "off by default" mean that if people don't use this option the "machine" platforms will be kept or does it mean that they will be removed?
I suggest renaming this to --remove-machine-platform
and the default behavior should be to not remove the machine platform.
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.
Another naming proposal would be to rename it to --remove-machine-only
which would be consistent with Automatus as this script is a wrapper over Automatus.
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.
It makes sense, I'm going to rename to --remove-machine-only
but make it on by default so it doesn't change the behavior of the script if there is already someone using it.
It seems to work as expected:
|
398ea18
to
4430928
Compare
7ae366e
to
aba93fc
Compare
aba93fc
to
06d377c
Compare
Code Climate has analyzed commit 06d377c and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 52.4% (0.0% change). View more on Code Climate. |
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.
Thanks for the changes!
The TF on CS 8 and 9 fails due to #10450 , this fail is unrelated to the contents of this PR.
The commit 06d377c was not supposed to be merged :( It was just for testing purpose. I'm going to open a PR to remove undo the change. |
Description:
Rationale
There is a commit test to trigger Automatus gating test which should not be part of the final PR.