Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script. #8156

JamesDrummond · 2018-01-03T20:35:50Z

What does this PR do?

Allow user to local scripts and configuration files in lieu of generated by setting CHE_OPENSHIFT_GENERATE_SCRIPTS =false. Default will remain to generate scripts DEFAULT_CHE_OPENSHIFT_GENERATE_SCRIPTS =true.

What issues does this PR fix or reference?

NA

Release Notes

Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script.

Signed-off-by: James Drummond james@devcomb.com

codenvy-ci · 2018-01-03T20:36:02Z

Can one of the admins verify this patch?

codenvy-ci · 2018-01-03T20:39:01Z

Can one of the admins verify this patch?

…ted. Signed-off-by: James Drummond <james@devcomb.com>

riuvshin · 2018-01-04T12:52:12Z

dockerfiles/init/modules/openshift/files/scripts/ocp.sh

+      if [ $IMAGE_PULL_POLICY == "Always" ]; then
+          docker pull "$IMAGE_INIT"
+      fi
+      docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock -v "${CONFIG_DIR}":/data -e IMAGE_INIT="$IMAGE_INIT" -e CHE_MULTIUSER="$CHE_MULTIUSER" eclipse/che-cli:${CHE_IMAGE_TAG} destroy --quiet --skip:pull --skip:nightly


that is copy paste based on older version of script, please update it accordingly to the current state.
And personally, I don't like the idea of allowing to use local scripts which is error prone.

I don't like the idea for people not developing che using but for me it is much easier to use local scripts versus generated.

Will make the change of the copy paste issue.

there is a reason why we generate this, we can merge this but there should be some comments that this is errorprone and may not work as expected

Added warning comments.

Signed-off-by: James Drummond <james@devcomb.com>

riuvshin

before merge please, test carefully single / multi user mode and deploying from scratch

codenvy-ci · 2018-01-09T14:14:13Z

Can one of the admins verify this patch?

JamesDrummond · 2018-01-11T17:01:32Z

@benoitf I need another approval before I can merge. Let me know if 2 approvals are still needed for each PR per eclipse policy.

benoitf · 2018-01-11T17:03:55Z

@JamesDrummond well the PR has conflicting files

Signed-off-by: James Drummond <james@devcomb.com>

JamesDrummond · 2018-01-11T17:33:52Z

@benoitf Reply beat my fix :)

JamesDrummond · 2018-01-12T15:32:47Z

@benoitf @sleshchenko Can one of you review/approve?

sleshchenko · 2018-01-15T08:11:27Z

I tried to use your changes from your branch, so I execute

export CHE_GENERATE_SCRIPTS="false"
./deploy_che.sh --deploy-che

And I got the following output. As you can see deploy_che.sh script is invoked twice. Do you get the same or I did something wrong?

OCP output

[serg@sleschenko scripts]$ ./ocp.sh --deploy-che OCP using existing scripts in current folder.

            10100000                
        0110011001101101            
   01001001001001011011010110

1100110101010001000001011101000001
0111001001111010111011111000111100010001
1110111011010111000001100000111001110
101010011001001001 10011101000
1011000101000 0
100101011 0110
10100001 11011111
111101111011 111011110001
10111001000110101 00000111111011111
0110111111011100000001011001010101111111
1101000001101001000010010111101011011011
00000001111001000001110111100000
000110101101000100011110
1101011100001001
000101

[CHE] Logging on using OpenShift endpoint "https://172.19.20.21:8443"...done!
[CHE] Checking if project "eclipse-che" exists...Project does not exist...creating it...done!
[CHE] Switching to "eclipse-che"...done!
[CHE] Setting admin role to "che" service account...rolebinding "che" created
[CHE] Retrieving latest version of fabric8 tenant Che template...done! (v.2.0.78)

[CHE] Deploying Che on ocp (image eclipse/che-server:nightly)
serviceaccount "che" created
service "che-host" created
persistentvolumeclaim "che-data-volume" created
persistentvolumeclaim "claim-che-workspace" created
deploymentconfig "che" created
route "che" created

[CHE] Che deployment has been successufully bootstrapped
[CHE] -> To check OpenShift deployment logs: 'oc get events -w'
[CHE] -> To check Che server logs: 'oc logs -f dc/che'
[CHE] -> Once the deployment is completed Che will be available at:
[CHE] http://che-eclipse-che.172.19.20.21.nip.io

rm: can't remove '/to_remove': Resource busy
WARN: Did detect TTY but not in interactive mode
INFO: (che cli): nightly - using docker 17.12.0-ce / native
WARN: (che init): 'nightly' installations cannot be upgraded to non-nightly versions
INFO: (che init): Installing configuration and bootstrap variables:
INFO: (che init): CHE_HOST=172.19.20.21
INFO: (che init): CHE_VERSION=nightly
INFO: (che init): CHE_CONFIG=/tmp/che-config
INFO: (che init): CHE_INSTANCE=/tmp/che-config/instance
INFO: (che config): Generating che configuration...
INFO: (che config): Customizing docker-compose for running in a container

            10100000                
        0110011001101101            
   01001001001001011011010110

1100110101010001000001011101000001
0111001001111010111011111000111100010001
1110111011010111000001100000111001110
101010011001001001 10011101000
1011000101000 0
100101011 0110
10100001 11011111
111101111011 111011110001
10111001000110101 00000111111011111
0110111111011100000001011001010101111111
1101000001101001000010010111101011011011
00000001111001000001110111100000
000110101101000100011110
1101011100001001
000101

[CHE] Logging on using OpenShift endpoint "https://172.19.20.21:8443"...done!
[CHE] Checking if project "eclipse-che" exists...done!
[CHE] Switching to "eclipse-che"...done!
[CHE] Setting admin role to "che" service account...rolebinding "che" configured
[CHE] Retrieving latest version of fabric8 tenant Che template...done! (v.2.0.78)

[CHE] Deploying Che on ocp (image eclipse/che-server:nightly)
serviceaccount "che" configured
service "che-host" configured
persistentvolumeclaim "che-data-volume" configured
persistentvolumeclaim "claim-che-workspace" configured
deploymentconfig "che" configured
route "che" configured

[CHE] Che deployment has been successufully bootstrapped
[CHE] -> To check OpenShift deployment logs: 'oc get events -w'
[CHE] -> To check Che server logs: 'oc logs -f dc/che'
[CHE] -> Once the deployment is completed Che will be available at:
[CHE] http://che-eclipse-che.172.19.20.21.nip.io

[CHE] wait CHE pod booting...
[serg@sleschenko scripts]$ ^C

P.S. The same with CHE_GENERATE_SCRIPTS=true.

Signed-off-by: James Drummond <james@devcomb.com>

JamesDrummond · 2018-01-15T23:20:04Z

@sleshchenko Sorry it's my fault after I merged master changes from master didn't catch I had deploy part in there twice and out of generate script if statement. Fixed now.

sleshchenko · 2018-01-16T07:49:38Z

dockerfiles/init/modules/openshift/files/scripts/ocp.sh

+      #wipeout config folder
+      docker run -v "${CONFIG_DIR}":/to_remove alpine sh -c "rm -rf /to_remove/" || true
+      docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock -v "${CONFIG_DIR}":/data -e IMAGE_INIT="$IMAGE_INIT" -e CHE_MULTIUSER="$CHE_MULTIUSER" eclipse/che-cli:${CHE_IMAGE_TAG} config --skip:pull --skip:nightly
+      cd "${CONFIG_DIR}/instance/config/openshift/scripts/"


${CONFIG_DIR}/instance/config/openshift/scripts/ is used twice. Please consider moving it into a separated variable like OPENSHIFT_SCRIPTS_FOLDER

Added variable OPENSHIFT_SCRIPTS_FOLDER.

sleshchenko · 2018-01-16T07:51:59Z

dockerfiles/init/modules/openshift/files/scripts/ocp.sh

+      docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock -v "${CONFIG_DIR}":/data -e IMAGE_INIT="$IMAGE_INIT" -e CHE_MULTIUSER="$CHE_MULTIUSER" eclipse/che-cli:${CHE_IMAGE_TAG} config --skip:pull --skip:nightly
+      cd "${CONFIG_DIR}/instance/config/openshift/scripts/"
+    else
+      echo "OCP using existing scripts in current folder."


The previous message contains information about configuration files but this one doesn't. Should we add it here too?
To be sure that I understand configuration files correctly: is it about yaml files?

it is shell scripts and yaml files needed to deploy che multi user

Added configuration files to second mesage.

riuvshin · 2018-01-16T16:02:35Z

dockerfiles/init/modules/openshift/files/scripts/ocp.sh

+      docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock -v "${CONFIG_DIR}":/data -e IMAGE_INIT="$IMAGE_INIT" -e CHE_MULTIUSER="$CHE_MULTIUSER" eclipse/che-cli:${CHE_IMAGE_TAG} config --skip:pull --skip:nightly
+      cd "${CONFIG_DIR}/instance/config/openshift/scripts/"
+    else
+      echo "OCP using existing scripts in current folder."


it is shell scripts and yaml files needed to deploy che multi user

riuvshin · 2018-01-16T16:10:59Z

dockerfiles/init/modules/openshift/files/scripts/ocp.sh

+      #Repull init image only if IMAGE_PULL_POLICY is set to Always
+      if [ $IMAGE_PULL_POLICY == "Always" ]; then
+         docker pull "$IMAGE_INIT"
+      fi


+ if [ $IMAGE_PULL_POLICY == "Always" ]; then + docker pull "$IMAGE_INIT" + fi

this must be moved out from if statement because it should be considered in both cases

JamesDrummond · 2018-01-16T17:07:06Z

@riuvshin @sleshchenko Updates complete. Please review.

…SCRIPTS Signed-off-by: James Drummond <james@devcomb.com>

JamesDrummond added the kind/enhancement A feature request - must adhere to the feature request template. label Jan 3, 2018

JamesDrummond added this to the 6.0.0-M5 milestone Jan 3, 2018

JamesDrummond self-assigned this Jan 3, 2018

JamesDrummond requested review from benoitf and riuvshin as code owners January 3, 2018 20:35

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 3, 2018

Allow user to local scripts and configuration files in lieu of genera…

28f0f48

…ted. Signed-off-by: James Drummond <james@devcomb.com>

JamesDrummond force-pushed the openshift_init_patch3 branch from bfa09e4 to 28f0f48 Compare January 3, 2018 21:15

JamesDrummond changed the title ~~Allow user to local scripts and configuration files in lieu of generated when using openshift ocp.sh script.~~ Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script. Jan 3, 2018

riuvshin reviewed Jan 4, 2018

View reviewed changes

devcomb added 2 commits January 4, 2018 15:01

Fixes per Roman's comments.

3257fd2

Signed-off-by: James Drummond <james@devcomb.com>

Added warning comments to generate script variable.

c28f8c8

Signed-off-by: James Drummond <james@devcomb.com>

riuvshin approved these changes Jan 5, 2018

View reviewed changes

benoitf removed their request for review January 11, 2018 10:10

Fix up

eb1d541

Signed-off-by: James Drummond <james@devcomb.com>

JamesDrummond requested review from sleshchenko and benoitf January 12, 2018 15:32

devcomb added 2 commits January 15, 2018 19:39

Signed-off-by: James Drummond <james@devcomb.com>

f851c2a

Merge branch 'master' into openshift_init_patch3

405f09a

Signed-off-by: James Drummond <james@devcomb.com>

JamesDrummond force-pushed the openshift_init_patch3 branch from 94f0c52 to 405f09a Compare January 15, 2018 23:17

sleshchenko reviewed Jan 16, 2018

View reviewed changes

riuvshin suggested changes Jan 16, 2018

View reviewed changes

devcomb added 2 commits January 16, 2018 16:44

Signed-off-by: James Drummond <james@devcomb.com>

8717c36

Signed-off-by: James Drummond <james@devcomb.com>

75871ed

Changed variable from CHE_GENERATE_SCRIPTS to CHE_OPENSHIFT_GENERATE_…

dd2a97c

…SCRIPTS Signed-off-by: James Drummond <james@devcomb.com>

sleshchenko approved these changes Jan 17, 2018

View reviewed changes

riuvshin approved these changes Jan 17, 2018

View reviewed changes

JamesDrummond merged commit 6636d7e into master Jan 17, 2018

JamesDrummond deleted the openshift_init_patch3 branch January 17, 2018 16:13

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 17, 2018

riuvshin mentioned this pull request Jan 19, 2018

fix ocp.sh deploy with custom CLI image #8363

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script. #8156

Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script. #8156

JamesDrummond commented Jan 3, 2018 •

edited

Loading

codenvy-ci commented Jan 3, 2018

codenvy-ci commented Jan 3, 2018

riuvshin Jan 4, 2018

JamesDrummond Jan 4, 2018

riuvshin Jan 4, 2018

JamesDrummond Jan 4, 2018

riuvshin left a comment

codenvy-ci commented Jan 9, 2018

JamesDrummond commented Jan 11, 2018

benoitf commented Jan 11, 2018

JamesDrummond commented Jan 11, 2018

JamesDrummond commented Jan 12, 2018

sleshchenko commented Jan 15, 2018 •

edited

Loading

JamesDrummond commented Jan 15, 2018 •

edited

Loading

sleshchenko Jan 16, 2018

JamesDrummond Jan 16, 2018

sleshchenko Jan 16, 2018

riuvshin Jan 16, 2018

JamesDrummond Jan 16, 2018

riuvshin Jan 16, 2018

riuvshin Jan 16, 2018

JamesDrummond Jan 16, 2018

JamesDrummond commented Jan 16, 2018

Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script. #8156

Allow user to use local scripts and configuration files in lieu of generated when using openshift ocp.sh script. #8156

Conversation

JamesDrummond commented Jan 3, 2018 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Release Notes

codenvy-ci commented Jan 3, 2018

codenvy-ci commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riuvshin left a comment

Choose a reason for hiding this comment

codenvy-ci commented Jan 9, 2018

JamesDrummond commented Jan 11, 2018

benoitf commented Jan 11, 2018

JamesDrummond commented Jan 11, 2018

JamesDrummond commented Jan 12, 2018

sleshchenko commented Jan 15, 2018 • edited Loading

JamesDrummond commented Jan 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesDrummond commented Jan 16, 2018

JamesDrummond commented Jan 3, 2018 •

edited

Loading

sleshchenko commented Jan 15, 2018 •

edited

Loading

JamesDrummond commented Jan 15, 2018 •

edited

Loading