Merge lp:~johnsca/charms/trusty/ibm-java/layer-review into lp:~ibmcharmers/charms/trusty/ibm-java/source

Proposed by Cory Johns on 2016-02-29
Status: Merged
Merged at revision: 2
Proposed branch: lp:~johnsca/charms/trusty/ibm-java/layer-review
Merge into: lp:~ibmcharmers/charms/trusty/ibm-java/source
Diff against target: 121 lines (+16/-21)
5 files modified
README.md (+0/-3)
config.yaml (+0/-5)
files/sftp_get.sh (+3/-5)
reactive/install (+9/-5)
tests/10-bundles-test.py (+4/-3)
To merge this branch: bzr merge lp:~johnsca/charms/trusty/ibm-java/layer-review
Reviewer Review Type Date Requested Status
Prabakaran (community) 2016-02-29 Approve on 2016-03-02
Review via email: mp+287511@code.launchpad.net

Description of the change

Recommended changes from review:

  * Fix lint errors in tests
  * Create archive dir if doesn't exist (gets dropped during `charm build`)
  * Improve error handling for download in expect script

To post a comment you must log in.
4. By Cory Johns on 2016-02-29

Remove unnecessary java-major config option

Prabakaran (prabacha) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.md'
2--- README.md 2016-02-15 08:55:57 +0000
3+++ README.md 2016-02-29 18:53:05 +0000
4@@ -125,9 +125,6 @@
5 should be altered by the user only when the package name is change. Else it
6 should not be altered.
7
8-- **java-major** Major version of Java. If not
9- set, it will be assigned to the default value 1.8.0.
10-
11 - **install-type** Type of install to perform. Valid options are 'jre' or 'full'.
12 The default is 'jre', which will install the IBM Java Runtime Environment (JRE).
13 Setting this to 'full' will install the IBM Java Development Kit in addition to the JRE.
14
15=== modified file 'config.yaml'
16--- config.yaml 2016-02-15 08:55:57 +0000
17+++ config.yaml 2016-02-29 18:53:05 +0000
18@@ -55,11 +55,6 @@
19 the integrity check. Refer README file to find out Checksum value for
20 downloaded package. Cryptographic value should be altered by the user
21 only when the package name is change. Else it should not be altered.
22- java-major:
23- type: string
24- default: 1.8.0
25- description: |
26- Major version of Java. Defaults to 1.8.0.
27 install-type:
28 type: string
29 default: jre
30
31=== modified file 'files/sftp_get.sh'
32--- files/sftp_get.sh 2016-02-15 08:55:57 +0000
33+++ files/sftp_get.sh 2016-02-29 18:53:05 +0000
34@@ -10,11 +10,9 @@
35 set localpath [lindex $argv 4]
36
37 set timeout 90
38-spawn sftp -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null $user@$host
39+spawn sftp -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null $user@$host:$r_dir $localpath
40 expect "password:"
41 send "$password\n"
42-expect "sftp>"
43 set timeout 3000
44-send "get -r $r_dir $localpath\n"
45-expect "sftp>"
46-send "exit\n"
47+catch wait result
48+exit [lindex $result 3]
49
50=== modified file 'reactive/install'
51--- reactive/install 2016-02-15 08:55:57 +0000
52+++ reactive/install 2016-02-29 18:53:05 +0000
53@@ -179,8 +179,10 @@
54 @when 'java.connected' 'license.accepted'
55 @when_not 'java.installed'
56 function install() {
57- java_major=$(config-get 'java-major')
58 install_type=$(config-get 'install-type')
59+ if [[ ! -d $ARCHIVE_DIR ]]; then
60+ mkdir $ARCHIVE_DIR
61+ fi
62 cd $ARCHIVE_DIR
63 echo -e "LICENSE_ACCEPTED=TRUE\nUSER_INSTALL_DIR=$JAVA_INSTALL_PATH\nINSTALLER_UI=silent" > installer.properties
64 download_java
65@@ -209,7 +211,7 @@
66 relation_call --state=java.connected set_ready $java_home $java_version
67
68 set_state 'java.installed'
69- status-set active "IBM Java ${java_major} installed"
70+ status-set active "IBM Java ${java_version} installed"
71 java -version
72 if [ $? == 0 ]; then
73 if [[ ${install_type} == "full" ]]; then
74@@ -229,10 +231,12 @@
75 }
76
77 @when 'java.connected' 'java.installed' 'license.accepted'
78+@when 'config.changed'
79 function check_version() {
80- java_major=$(config-get 'java-major')
81- java_major_installed=$(java -version 2>&1 | grep -i version | head -n 1 | awk -F '"' '{print $2}')
82- if [[ $java_major != $java_major_installed ]]; then
83+ if charms.reactive is_state 'config.changed.java_path_name' || \
84+ charms.reactive is_state 'config.changed.install-type' || \
85+ charms.reactive is_state 'config.changed.java_file_name' || \
86+ charms.reactive is_state 'config.changed.sha'; then
87 status-set maintenance "Uninstalling all IBM Java versions before installing new one"
88 remove_java_software
89 install
90
91=== modified file 'tests/10-bundles-test.py' (properties changed: -x to +x)
92--- tests/10-bundles-test.py 2016-02-15 08:55:57 +0000
93+++ tests/10-bundles-test.py 2016-02-29 18:53:05 +0000
94@@ -7,7 +7,6 @@
95 import amulet
96 import sys
97 import time
98-import requests
99
100 # Lots of prereqs on this charm so give it a large timeout accordingly
101 seconds_to_wait = 20000
102@@ -103,7 +102,8 @@
103 cmd = "java -version"
104 output, code = ibmjava_unit.run(cmd)
105 if code != 0:
106- message = 'Unable to run Java Command: As java is not installed or appropriate path variable is not set.'
107+ message = 'Unable to run Java Command: As java is not ' \
108+ 'installed or appropriate path variable is not set.'
109 amulet.raise_status(amulet.FAIL, msg=message)
110 else:
111 ibmjava_unit.run('mkdir -m a=rwx /home/test')
112@@ -121,7 +121,8 @@
113 output, code = ibmjava_unit.run(cmd)
114 print('Output of simple java program is %s' % output)
115 if code != 0:
116- message = 'Unable to run Java Program: As java is not installed or appropriate path variable is not set.'
117+ message = 'Unable to run Java Program: As java is not ' \
118+ 'installed or appropriate path variable is not set.'
119 amulet.raise_status(amulet.FAIL, msg=message)
120
121 if __name__ == '__main__':

Subscribers

People subscribed via source and target branches

to all changes: