Fix Che using different database path in different versions #8073

mkuznyetsov · 2017-12-27T15:18:54Z

What does this PR do?

Export Che environment variables only if there were not defined
Move database files, that were stored in different paths until and after Che 6.0.0-M3 version:

Database will be stored at path "/data/storage/db/che.mv.db"
If there is a database at old path present ("data/db/che.mv.db"), a warning will be shown in Che container logs, and, if no database is present by the new path, it will be moved there

What issues does this PR fix or reference?

should fix #8068

Fix Che using different database path in different versions.
entypoint.sh will not overwrite Che environment variables, if they were already defined

Release Notes

Docs PR

N/A

…from CLI

riuvshin

please read comments, we need to handle ONLY vars that set value, vars like

export VAR=${VAR:-${DEFAULT_VAR}}

are OK as they already pick vars from ENV

riuvshin · 2017-12-27T18:04:31Z

dockerfiles/che/entrypoint.sh

@@ -47,10 +47,10 @@ Variables:

  # Must be exported as this will be needed by Tomcat's JVM
  DEFAULT_CHE_REGISTRY_HOST=localhost
-  export CHE_REGISTRY_HOST=${CHE_REGISTRY_HOST:-${DEFAULT_CHE_REGISTRY_HOST}}


in that case we don;t need any change because it already pick value fom ENV VAR

riuvshin · 2017-12-27T18:04:43Z

dockerfiles/che/entrypoint.sh


  DEFAULT_CHE_PORT=8080
-  export CHE_PORT=${CHE_PORT:-${DEFAULT_CHE_PORT}}


in that case we don;t need any change because it already pick value fom ENV VAR

riuvshin · 2017-12-27T18:05:06Z

dockerfiles/che/entrypoint.sh

@@ -59,7 +59,7 @@ Variables:
  CHE_LOG_LEVEL=${CHE_LOG_LEVEL:-${DEFAULT_CHE_LOG_LEVEL}}

  DEFAULT_CHE_LOGS_DIR="${CATALINA_HOME}/logs/"
-  export CHE_LOGS_DIR=${CHE_LOGS_DIR:-${DEFAULT_CHE_LOGS_DIR}}


in that case we don;t need any change because it already pick value fom ENV VAR

…igured from CLI

riuvshin · 2018-01-02T12:15:00Z

dockerfiles/che/entrypoint.sh

  CHE_DATA_HOST=$(get_che_data_from_host)

  CHE_USER=${CHE_USER:-root}
-  export CHE_USER=$CHE_USER
+  [ -z "$CHE_USER" ] && export CHE_USER=$CHE_USER


why you changed that?

riuvshin · 2018-01-02T12:15:23Z

dockerfiles/che/entrypoint.sh

  if [ "$CHE_USER" != "root" ]; then
    if [ ! $(getent group docker) ]; then
      echo "!!!"
      echo "!!! Error: The docker group doesn't exist."
      echo "!!!"
      exit 1
    fi
-    export CHE_USER_ID=${CHE_USER}
+    [ -z "$CHE_USER_ID" ] && export CHE_USER_ID=${CHE_USER}


why you changed that?

riuvshin · 2018-01-02T12:15:52Z

dockerfiles/che/entrypoint.sh


  # CHE_DOCKER_IP_EXTERNAL must be set if you are in a VM.
  HOSTNAME=${CHE_DOCKER_IP_EXTERNAL:-$(get_docker_external_hostname)}
  if has_external_hostname; then
    # Internal property used by Che to set hostname.
-    export CHE_DOCKER_IP_EXTERNAL=${HOSTNAME}
+    [ -z "$CHE_DOCKER_IP_EXTERNAL" ] && export CHE_DOCKER_IP_EXTERNAL=${HOSTNAME}


why you changed that?

…ypoint

…ix-entrypoint

… into fix-entrypoint

sleshchenko · 2018-01-10T07:44:13Z

dockerfiles/che/entrypoint.sh

+    echo "See issue https://github.com/eclipse/che/issues/8068 for details"
+    #in case if there is existing database in old path, back it up
+    if [ -f ${CHE_DATA}/db/che.mv.db ]; then
+      mv ${CHE_DATA}/db/che.mv.db ${CHE_DATA}/db/che.mv.db.old


It can be useful to add logs that old database is backed up

…pse/che into fix-entrypoint

benoitf · 2018-01-10T10:38:33Z

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties

@@ -11,7 +11,7 @@

 ### CHE SERVER
 # Folder where Che will store internal data objects
-che.database=${che.home}/storage
+che.database=${che.home}


why it's not anymore in a subfolder ?

It was like so in Che instances, that were running through CLI for a long while because of this che.env.erb:
https://github.com/eclipse/che/blob/56866159228bf4ba1d3b7fc8d2206c60b129579b/dockerfiles/init/modules/che/templates/che.env.erb#L36
So, the values from the che.properties or entrypoint.sh, which would point to path with "storage" directory had no effect, until recently (which was the issue cause)

I decided to consolidate everything by old path, though if you think extra "storage" directory is better, it can be changed to that

I would say that it looks odd to have a "root" directory as path for che database so yes using a subfolder would be better IMHO

https://github.com/eclipse/che/blob/93b974d1e0ec7836635f3d7da196b7651da8ad1b/dockerfiles/che/entrypoint.sh#L233

@benoitf Am I understand you correctly. You want to change db file location.
Add extra path "storage" to what we have in Che5?

yes. From what I see there is a 'db' subfolder added
It's not directly stored inside the che.home folder

mkuznyetsov · 2018-01-10T16:23:11Z

@benoitf I changed the PR, and updated the description, describing how it works now

database will be stored by path "/data/storage/db/che.mv.db"
if upgrading Che from versions, that were using different path, it will leave a warning in the container logs, and would place it in the proper path, unless there is already DB there.

benoitf · 2018-01-10T16:35:40Z

dockerfiles/che/entrypoint.sh

@@ -84,7 +84,7 @@ set_environment_variables () {
  # CHE_DOCKER_IP is used internally by Che to set its IP address
  if [[ -z "${CHE_DOCKER_IP}" ]]; then
    if [[ -n "${CHE_IP}" ]]; then
-        export CHE_DOCKER_IP="${CHE_IP}"
+      export CHE_DOCKER_IP="${CHE_IP}"


maybe you can skip this change from this PR's commit as it's unrelated (it's more a indent fix)

Entrypoint.sh won't override env variables that have been configured …

bb4dba8

…from CLI

mkuznyetsov added the kind/bug Outline of a bug - must adhere to the bug report template. label Dec 27, 2017

mkuznyetsov self-assigned this Dec 27, 2017

mkuznyetsov requested review from benoitf and riuvshin as code owners December 27, 2017 15:18

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 Dec 27, 2017

riuvshin suggested changes Dec 27, 2017

View reviewed changes

mkuznyetsov added 4 commits December 28, 2017 09:40

fixup! Entrypoint.sh won't override env variables that have been conf…

e322211

…igured from CLI

Merge branch 'master' of github.com:eclipse/che into fix-entrypoint

bff33e1

Merge branch 'master' of github.com:eclipse/che into fix-entrypoint

50329bd

fixup

f4d1ce4

riuvshin reviewed Jan 2, 2018

View reviewed changes

mkuznyetsov added 4 commits January 4, 2018 11:41

Merge branch 'master' of github.com:eclipse/che into fix-entrypoint

6ac0e9d

Merge branch 'master' of github.com:eclipse/che into fix-entrypoint

fdbb5be

Add migration for database stored in the wrong path

1f4865c

Merge branch 'fix-entrypoint' of github.com:eclipse/che into fix-entr…

6da1c42

…ypoint

mkuznyetsov requested a review from skabashnyuk as a code owner January 9, 2018 15:23

fixup! Merge branch 'fix-entrypoint' of github.com:eclipse/che into f…

0d16a64

…ix-entrypoint

mkuznyetsov changed the title ~~export Che environment variables in entrypoint.sh only if they were not already defined~~ Fix Che using different database path in different versions Jan 9, 2018

fixup! fixup! Merge branch 'fix-entrypoint' of github.com:eclipse/che…

26e32f7

… into fix-entrypoint

sleshchenko reviewed Jan 10, 2018

View reviewed changes

fixup! fixup! fixup! Merge branch 'fix-entrypoint' of github.com:ecli…

b17b070

…pse/che into fix-entrypoint

riuvshin approved these changes Jan 10, 2018

View reviewed changes

skabashnyuk approved these changes Jan 10, 2018

View reviewed changes

benoitf reviewed Jan 10, 2018

View reviewed changes

mkuznyetsov added 2 commits January 10, 2018 17:00

Change database to the newer path instead

368f415

fixup! Change database to the newer path instead

545f930

benoitf approved these changes Jan 10, 2018

View reviewed changes

benoitf reviewed Jan 10, 2018

View reviewed changes

mkuznyetsov added 2 commits January 15, 2018 09:58

Merge branch 'master' of github.com:eclipse/che into fix-entrypoint

012c0e0

fixup

93fb8c3

mkuznyetsov merged commit 3d7f1a1 into master Jan 15, 2018

mkuznyetsov deleted the fix-entrypoint branch January 15, 2018 08:02

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 15, 2018

benoitf added this to the 6.0.0-M5 milestone Jan 15, 2018

This was referenced Jan 16, 2018

Fix moving database to new path #8307

Merged

Fix CHE_DATABASE env. var to point at proper database path #8367

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Che using different database path in different versions #8073

Fix Che using different database path in different versions #8073

mkuznyetsov commented Dec 27, 2017 •

edited

Loading

riuvshin left a comment

riuvshin Dec 27, 2017

riuvshin Dec 27, 2017

riuvshin Dec 27, 2017

riuvshin Jan 2, 2018

riuvshin Jan 2, 2018

riuvshin Jan 2, 2018

sleshchenko Jan 10, 2018

mkuznyetsov Jan 10, 2018

benoitf Jan 10, 2018

mkuznyetsov Jan 10, 2018

benoitf Jan 10, 2018

skabashnyuk Jan 10, 2018

benoitf Jan 10, 2018

mkuznyetsov commented Jan 10, 2018

benoitf Jan 10, 2018 •

edited

Loading


		DEFAULT_CHE_PORT=8080
		export CHE_PORT=${CHE_PORT:-${DEFAULT_CHE_PORT}}

Fix Che using different database path in different versions #8073

Fix Che using different database path in different versions #8073

Conversation

mkuznyetsov commented Dec 27, 2017 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

riuvshin left a 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

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

mkuznyetsov commented Jan 10, 2018

benoitf Jan 10, 2018 • edited Loading

Choose a reason for hiding this comment

mkuznyetsov commented Dec 27, 2017 •

edited

Loading

benoitf Jan 10, 2018 •

edited

Loading