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.

Sign up for GitHub

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

Add package pre-install check for java binary #31343

Merged
merged 5 commits into from
Jun 25, 2018
Merged

Add package pre-install check for java binary #31343

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85ee81a
Add package pre-install check for java binary
hub-cap Jun 14, 2018
442023c
Fix the bash test flag
hub-cap Jun 15, 2018
a1a1e2c
Merge remote-tracking branch 'upstream/master' into fix_java_check
hub-cap Jun 15, 2018
127799e
Merge remote-tracking branch 'elastic/master' into pr/31343
jasontedor Jun 25, 2018
084f9ff
Use same error message as elasticsearch-env
jasontedor Jun 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions distribution/packages/src/common/scripts/preinst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@
# $1=1 : indicates an new install
# $1=2 : indicates an upgrade

# Check for these at preinst time due to failures in postinst if they do not exist
if [ -x "$JAVA_HOME/bin/java" ]; then
JAVA="$JAVA_HOME/bin/java"
else
JAVA=`which java`
fi

if [ ! -x "$JAVA" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Does this ever execute in a failing case? I think that these scripts are always executed with set +e or does it differ by packaging system? If we get here it's because $JAVA_HOME/bin/java exists which we set JAVA to, or which java failed and we should have exited?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea i copied this from https://github.com/elastic/elasticsearch/blob/master/distribution/packages/src/deb/init.d/elasticsearch#L83-L94 , and i think the logic is wrong. Ill fix up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the bash test check to see if $JAVA is set.

echo "Could not find any executable java binary. Please install java in your PATH or set JAVA_HOME"
Copy link
Member

Choose a reason for hiding this comment

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

If this block is needed we have could not find java; set JAVA_HOME or ensure java is in PATH in elasticsearch-env. Maybe we can be consistent?

exit 1
fi

case "$1" in

# Debian ####################################################
Expand Down
8 changes: 8 additions & 0 deletions qa/vagrant/src/test/resources/packaging/tests/30_deb_package.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ setup() {
[ "$status" -eq 1 ]
}

@test "[DEB] temporarily remove java and ensure the install fails" {
move_java
run dpkg -i elasticsearch-oss-$(cat version).deb
output=$status
unmove_java
Copy link
Member

Choose a reason for hiding this comment

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

💯

[ "$output" -eq 1 ]
}

@test "[DEB] install package" {
dpkg -i elasticsearch-oss-$(cat version).deb
}
Expand Down
8 changes: 8 additions & 0 deletions qa/vagrant/src/test/resources/packaging/tests/40_rpm_package.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ setup() {
[ "$status" -eq 1 ]
}

@test "[RPM] temporarily remove java and ensure the install fails" {
move_java
run rpm -i elasticsearch-oss-$(cat version).rpm
output=$status
unmove_java
Copy link
Member

Choose a reason for hiding this comment

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

💯

[ "$output" -eq 1 ]
}

@test "[RPM] install package" {
rpm -i elasticsearch-oss-$(cat version).rpm
}
Expand Down
21 changes: 19 additions & 2 deletions qa/vagrant/src/test/resources/packaging/utils/utils.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,11 @@ if [ ! -x "`which unzip 2>/dev/null`" ]; then
fi

if [ ! -x "`which java 2>/dev/null`" ]; then
echo "'java' command is mandatory to run the tests"
exit 1
# there are some tests that move java temporarily
if [ ! -x "`command -v java.bak 2>/dev/null`" ]; then
echo "'java' command is mandatory to run the tests"
exit 1
fi
fi

# Returns 0 if the 'dpkg' command is available
Expand Down Expand Up @@ -578,3 +581,17 @@ file_privileges_for_user_from_umask() {

echo $((0777 & ~$(sudo -E -u $user sh -c umask) & ~0111))
}

# move java to simulate it not being in the path
move_java() {
which_java=`command -v java`
assert_file_exist $which_java
mv $which_java ${which_java}.bak
}

# move java back to its original location
unmove_java() {
which_java=`command -v java.bak`
assert_file_exist $which_java
mv $which_java `dirname $which_java`/java
}