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

Fixes #23622: Some jetty patches don't apply to 10.0.17 #2825

Conversation

fanf
Copy link
Member

@fanf fanf commented Oct 20, 2023

https://issues.rudder.io/issues/23622

SO. Almost everything changed in the start script between 10.0.12 and 10.0.17. This is unfortunate and feels risky.

Still, I was able to port almost all patches. There is one that I don't understand: jetty-init-prevent-false-failed-starts.patch

The other ones are mainly change in place, sometime a lot but it seems ok.

@fanf fanf requested a review from amousset October 20, 2023 08:45
[ -z "$(tail -1 $1 | grep FAILED 2>/dev/null)" ] || return 1
- local PID=$(cat "$2" 2>/dev/null) || return 1
- kill -0 "$PID" 2>/dev/null || return 1
echo -n ". "
Copy link
Member Author

Choose a reason for hiding this comment

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

this one, I really don't know what it became. Things seems to have change a lot in the jetty start script. And our git history is broken because of rudder-server new package. I'm not sure what to do with that.

@fanf
Copy link
Member Author

fanf commented Oct 20, 2023

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder-packages/pull/2825
-- Your faithful QA
Kant merge: "Science is organized knowledge. Wisdom is organized life."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/75190/console)

@amousset
Copy link
Member

OK, squash merging this PR

@amousset amousset force-pushed the bug_23622/some_jetty_patches_don_t_apply_to_10_0_17 branch from 5aa6425 to ef41f6f Compare October 24, 2023 07:47
@amousset
Copy link
Member

OK, merging this PR

1 similar comment
@amousset
Copy link
Member

OK, merging this PR

@amousset amousset merged commit ef41f6f into Normation:branches/rudder/7.3 Oct 24, 2023
3 checks passed
UMASK="0007"
echo "Setting umask to ${UMASK}"

+ # Checking if enough RAM is available for Jetty to use
+ # Metaspace is auomanaged in JDK8+. Rudder needs 150Mo of it.
+ checkAvailableRam $((${JAVA_XMX}+150))

if (( NO_START )); then
echo "Not starting ${NAME} - NO_START=1";
Copy link
Member

Choose a reason for hiding this comment

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

there muse have been an fi somewhere

+elif [ -n "${ns}" ]; then # we have namespaces
+ # the sed is here to prepend a fake user field that is removed by the -o option (it is never used)
+ PS_COMMAND="eval ps --no-header -e -O utsns | grep -E '^[[:space:]]*[[:digit:]]*[[:space:]]+${ns}' | sed 's/^/user /'"
+ PS_COMMAND="eval ps --no-header -e -O utsns | grep -E '^[[:space:]]*[[:digit:]]*[[:space:]]+${ns}' | sed 's/^/user >
Copy link
Member

Choose a reason for hiding this comment

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

end of line is missing

@@ -13,21 +13,21 @@
+ TOTAL_MEM_NEEDED=$((((${1})*100)/90))
+ TOTAL_MEM_AVAILABLE=$(($(free -m|awk '/^Mem:/{print $2}')+$(free -m|awk '/^Swap:/{print $2}')))
+ if [ ${TOTAL_MEM_AVAILABLE} -le ${TOTAL_MEM_NEEDED} ]; then
+ echo "WARNING: Not enough free memory to start Jetty (about ${TOTAL_MEM_NEEDED}MB are needed). Trying anyway, but the application is likely to fail."
+ echo "WARNING: Not enough free memory to start Jetty (about ${TOTAL_MEM_NEEDED}MB are needed). Trying anyway, but>
Copy link
Member

Choose a reason for hiding this comment

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

end of line is lost

This pull request was closed.
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.

4 participants