-
Notifications
You must be signed in to change notification settings - Fork 97
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
Unify unatteded uninstall check tools and start cluster #1221
base: unify-unatteded-uninstall-chose-component
Are you sure you want to change the base?
Unify unatteded uninstall check tools and start cluster #1221
Conversation
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.
LGTM
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.
cores=$(cat /proc/cpuinfo | grep -c processor )
Check if the file exist.
"tar" | ||
"awk" | ||
"free" | ||
"sed") |
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.
Include echo
, cat
, grep
, rm
, touch
, kill
, cp
, mv
, export
, mkdir
, curl
, chown
, sudo
, chmod
, in the list. Then, don't exit in the first not found command. Instead of this, make a list of not-found dependencies and print the list, avoiding the re-run for every command.
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.
Done!
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.
Installation simulation, modifying the list of tools by adding the false commands 'badbad' and 'notsobad'.
sudo bash /tmp/unattended_installer/wazuh_install.sh --local --development --ignore-health-check --disable-spinner --all-in-one
03/02/2022 16:41:02 INFO: ---------------------------------- Missing tool -----------------------------------
03/02/2022 16:41:02 INFO: Missing tool report: badbad, notsobad, are not installed. All this command's are necessary for the correct use of this tool.
Done! |
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.
Please review the requested changes and please attach examples of changes.
@@ -308,7 +351,12 @@ function checkPreviousCertificates() { | |||
|
|||
function checkSpecs() { | |||
|
|||
cores=$(cat /proc/cpuinfo | grep -c processor ) | |||
coresFile=/etc/resolv.conf |
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.
coresFile=/etc/resolv.conf | |
coresFile="/proc/cpuinfo" |
Simulation of a linux where the file does not exist: /proc/cpuinfo. Using the name /proc/cpuinfoTEST
Continuation of the installation process following the recommendation of the script.
|
…natteded-uninstall-check-tools-and-start-cluster
Installation simulation, modifying the list of tools by adding the false commands 'badbad' and 'notsobad'.
|
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.
Please review the requested changes
|
||
if [ -n "${missingtoolsList}" ]; then | ||
|
||
logger "---------------------------------- Missing tool -----------------------------------" |
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.
logger "---------------------------------- Missing tool -----------------------------------" | |
logger "---------------------------------- Missing tools -----------------------------------" |
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.
Done
if [ -n "${missingtoolsList}" ]; then | ||
|
||
logger "---------------------------------- Missing tool -----------------------------------" | ||
logger "Missing tool report: ${missingtoolsList} are not installed. All this command's are necessary for the correct use of this tool." |
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.
logger "Missing tool report: ${missingtoolsList} are not installed. All this command's are necessary for the correct use of this tool." | |
logger "The following command or commands are not present in the system: ${missingtoolsList} and must it is / they are necessary for the correct use of this tool." |
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.
Done
@@ -404,14 +407,6 @@ function main() { | |||
checkNames | |||
fi | |||
|
|||
# -------------- Prerequisites and Wazuh repo ---------------------- |
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.
Is the Dependencies installation uneccesary?
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.
Recovered, this was a mistake
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.
But why is moved before the preliminary steps?
If there is an error with the node names, etc, I prefer to know it before starting installing dependencies.
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.
I am sorry, I do not understand. Tell me if I understand correctly please.
Functions import. Line 332. Check the system tools.
Preliminary checks, Line 345. This step checks arguments and parameters.
Prerequisites and Wazuh repo, Line 375. This step installs dependencies
Preliminary steps, Line 383. It only validates checkPreviousCertificates.
I understand this would be correct.
logger "Check recommended minimum hardware requirements for Elasticsearch done." | ||
fi | ||
if [ -z "${cores}" ]; then | ||
logger -w "The script needs to parse the file '${coresFile}' to check the minimum required hardware of CPU cores." |
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.
If you plan to exit from the script run, this message is not a warning but an error. Only errors should be able to exit from a run.
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.
Done
exit 1 | ||
fi | ||
if [ -z "${ram_gb}" ]; then | ||
logger - w "The command 'free' is required to check the minimum required hardware of RAM." |
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 point: is there any way to reach this code without free
installed?
I like the idea of ${ram_gb}
not valid value management, but I don't think the message could happen in any situation.
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.
If the free command was not installed. /proc/meminfo can be used
Although I did not imagine a possible scenario where free was not installed, I preferred to add this check, in case there were any cases.
When I did some research later I found that free and other commands come specifically from a package.
PS: I add a workarrond for the stage that free is not.
…eckTools (order a-z)
…natteded-uninstall-check-tools-and-start-cluster
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.
Please review the requested changes
MEMinKB=$(cat "$memFile" | grep MemTotal | awk '/^MemTotal:/{print $2}') | ||
ram_gb=$(( $MEMinKB / 1024 )) | ||
else | ||
logger -e "The $coresFile does not exist." |
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.
Why $coresFile?
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.
Done 2761821
@@ -404,14 +407,6 @@ function main() { | |||
checkNames | |||
fi | |||
|
|||
# -------------- Prerequisites and Wazuh repo ---------------------- |
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.
But why is moved before the preliminary steps?
If there is an error with the node names, etc, I prefer to know it before starting installing dependencies.
if [ -n "${missingtoolsList}" ]; then | ||
|
||
logger "---------------------------------- Missing tools -----------------------------------" | ||
logger "The following command or commands are not present in the system: ${missingtoolsList} and must it is / they are necessary for the correct use of this tool." |
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.
logger "The following command or commands are not present in the system: ${missingtoolsList} and must it is / they are necessary for the correct use of this tool." | |
logger "The following command or commands are not present in the system: ${missingtoolsList}. Those tools are necessary for the correct use of this tool." |
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.
Done 2761821
Pending conflict resolution! |
Description
The script needs to be able to do: