Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9475

Add install script support for CentOS

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 6.2
    • Fix Version/s: 6.3, master (7.0)
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None
    • Environment:

      Centos 7

      Description

      [root@ns521582 tmp]# sudo ./install_solr_service.sh solr-6.2.0.tgz

      id: solr: no such user
      Creating new user: solr
      adduser: group '--disabled-password' does not exist

      Extracting solr-6.2.0.tgz to /opt

      Installing symlink /opt/solr -> /opt/solr-6.2.0 ...

      Installing /etc/init.d/solr script ...

      /etc/default/solr.in.sh already exist. Skipping install ...

      /var/solr/data/solr.xml already exists. Skipping install ...

      /var/solr/log4j.properties already exists. Skipping install ...

      chown: invalid spec: ‘solr:’
      ./install_solr_service.sh: line 322: update-rc.d: command not found
      id: solr: no such user
      User solr not found! Please create the solr user before running this script.
      id: solr: no such user
      User solr not found! Please create the solr user before running this script.
      Service solr installed.

      Reference - http://stackoverflow.com/questions/39320647/unable-to-create-user-when-installing-solr-6-2-0-on-centos-7

      1. install_solr_service.sh
        11 kB
        Jan Høydahl
      2. SOLR-9475_osrelease_fix.patch
        3 kB
        Jan Høydahl
      3. SOLR-9475_osrelease_fix.patch
        0.7 kB
        Jan Høydahl
      4. SOLR-9475.patch
        3 kB
        Jan Høydahl

        Issue Links

          Activity

          Hide
          janhoy Jan Høydahl added a comment -

          Attaching patch which adds detection of CentOS as well as gives more reliable distro detection by using /etc/*-release with fallback to /proc/version and uname -a. This is important for virtualized environments.

          Also attaching resulting install_solr_service.sh for easier testing.

          Show
          janhoy Jan Høydahl added a comment - Attaching patch which adds detection of CentOS as well as gives more reliable distro detection by using /etc/*-release with fallback to /proc/version and uname -a . This is important for virtualized environments. Also attaching resulting install_solr_service.sh for easier testing.
          Hide
          janhoy Jan Høydahl added a comment -

          Nitin Surana, can you test the new script to see if it solves your issues?

          Show
          janhoy Jan Høydahl added a comment - Nitin Surana , can you test the new script to see if it solves your issues?
          Hide
          janhoy Jan Høydahl added a comment -

          Do anyone else see any disadvantage by resolving Distro from /etc/*-release? It works on my Docker containers with Ubuntu, Debian, RHEL, OpenSUSE and CentOS, so I suppose it works on bare-metal too...

          Show
          janhoy Jan Høydahl added a comment - Do anyone else see any disadvantage by resolving Distro from /etc/*-release ? It works on my Docker containers with Ubuntu, Debian, RHEL, OpenSUSE and CentOS, so I suppose it works on bare-metal too...
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 74bf88f8fe50b59e666f9387ca65ec26f821089d in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=74bf88f ]

          SOLR-9475: Add install script support for CentOS and better distro detection under Docker

          (cherry picked from commit a1bbc99)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 74bf88f8fe50b59e666f9387ca65ec26f821089d in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=74bf88f ] SOLR-9475 : Add install script support for CentOS and better distro detection under Docker (cherry picked from commit a1bbc99)
          Hide
          janhoy Jan Høydahl added a comment -

          Pushed to branch_6x and master.

          Nitin Surana, please test to see if it fixes your problem. You may fetch the new script from https://github.com/apache/lucene-solr/blob/branch_6x/solr/bin/install_solr_service.sh

          Show
          janhoy Jan Høydahl added a comment - Pushed to branch_6x and master. Nitin Surana , please test to see if it fixes your problem. You may fetch the new script from https://github.com/apache/lucene-solr/blob/branch_6x/solr/bin/install_solr_service.sh
          Show
          janhoy Jan Høydahl added a comment - Update RefGuide https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=50856198&selectedPageVersions=47&selectedPageVersions=46
          Hide
          hossman Hoss Man added a comment -

          NOTE: master commit was http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/a1bbc996


          Slurping in all of the files like this seems like a very bad idea and should be rolled back...

          +proc_version=`cat /etc/*-release 2>/dev/null`
          

          ...if for no other reason then how proc_version is used in the event of an OS we don't recognize...

             echo -e "\nERROR: Your Linux distribution ($proc_version) not supported by this script!\nYou'll need to setup
          Solr as a service manually using the documentation provided in the Solr Reference Guide.\n" 1>&2
           

          In general i would suggest that the correct behavior is to test, in order...

          • lsb_release -i
            • required by the "Linux Standard Base Core" spec (which AFAIK almost every linux distro supports even if they don't get fully certified)
          • uname -a
            • required by POSIX, and contains the distro name in most cases
          • /proc/version
            • should be on every machine running a linux kernel, but is only required to include the kernel version, not the distro info
          • cat /etc/*release

          The key element being that we should not just look to see if these exist in that order, but actually be testing these against our list of pattern strings ("Debian", "Red Hat", etc...) in sequence before moving on the the next on the list.

          For example: our current logic is (psuedo code)...

          try { 
            proc_version = `x` 
          } catch Error {
            try {
              proc_version = `y`
            } catch Error {
              try {
               proc_version = `z`
             } 
           }
          }
          for (d : known_distros) {
            if (proc_version contains d) {
              return d
            }
          }
          

          Instead we should be doing...

          for (cmd : { x, y, z }) {
            try {
              possibility = `cmd`
              for (d : known_distros) {
                if (possibility contains d) {
                  return d
               }
            }
          }
          

          ...that way we can test more reliable options (like `uname -a`) earlier in the list, but we can still proceed even if those commands exist & run but don't return the name of the distribution on some platforms. (ala CentOS apparently)

          (ie: we should not try the sketchy/risky stuff first, just because it's more likely we'll get a match. we should test the stuff that's more well defined and well structured first, even if we know there are many systems where the command might exist but may not give us useful info)

          Show
          hossman Hoss Man added a comment - NOTE: master commit was http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/a1bbc996 Slurping in all of the files like this seems like a very bad idea and should be rolled back... +proc_version=`cat /etc/*-release 2>/dev/ null ` ...if for no other reason then how proc_version is used in the event of an OS we don't recognize... echo -e "\nERROR: Your Linux distribution ($proc_version) not supported by this script!\nYou'll need to setup Solr as a service manually using the documentation provided in the Solr Reference Guide.\n" 1>&2 In general i would suggest that the correct behavior is to test, in order... lsb_release -i required by the "Linux Standard Base Core" spec (which AFAIK almost every linux distro supports even if they don't get fully certified) uname -a required by POSIX, and contains the distro name in most cases /proc/version should be on every machine running a linux kernel, but is only required to include the kernel version, not the distro info cat /etc/*release The key element being that we should not just look to see if these exist in that order, but actually be testing these against our list of pattern strings ("Debian", "Red Hat", etc...) in sequence before moving on the the next on the list. For example: our current logic is (psuedo code)... try { proc_version = `x` } catch Error { try { proc_version = `y` } catch Error { try { proc_version = `z` } } } for (d : known_distros) { if (proc_version contains d) { return d } } Instead we should be doing... for (cmd : { x, y, z }) { try { possibility = `cmd` for (d : known_distros) { if (possibility contains d) { return d } } } ...that way we can test more reliable options (like `uname -a`) earlier in the list, but we can still proceed even if those commands exist & run but don't return the name of the distribution on some platforms. (ala CentOS apparently) (ie: we should not try the sketchy/risky stuff first, just because it's more likely we'll get a match. we should test the stuff that's more well defined and well structured first, even if we know there are many systems where the command might exist but may not give us useful info)
          Hide
          janhoy Jan Høydahl added a comment -

          Slurping in all of the files like this seems like a very bad idea

          Yes, that is bad, and indeed it turns out it is totally unnecessary, since all the distros have agreed to support /etc/os-release. That is a nice structured file, and all we need is the NAME= line. So I have done one change to one line and I think we're good:

          - proc_version=`cat /etc/*-release 2>/dev/null`
          + proc_version=$(grep -E "^NAME=" /etc/os-release 2>/dev/null)
          

          I tested this change across a number of distros by extracting the detection code into a script distro.sh and then running this command

          for os in ubuntu debian centos opensuse gidikern/rhel-oracle-jre; do echo "Testing on $os:" && docker run -it -v `pwd`/distro.sh:/tmp/distro.sh $os bash /tmp/distro.sh ; done
          Testing on ubuntu:
          Contents of variable 'proc_version' before matching: NAME="Ubuntu"
          Contents of variable 'distro' after matching: Ubuntu
          Testing on debian:
          Contents of variable 'proc_version' before matching: NAME="Debian GNU/Linux"
          Contents of variable 'distro' after matching: Debian
          Testing on centos:
          Contents of variable 'proc_version' before matching: NAME="CentOS Linux"
          Contents of variable 'distro' after matching: CentOS
          Testing on opensuse:
          Contents of variable 'proc_version' before matching: NAME="openSUSE Leap"
          Contents of variable 'distro' after matching: SUSE
          Testing on gidikern/rhel-oracle-jre:
          Contents of variable 'proc_version' before matching: NAME="Red Hat Enterprise Linux Server"
          Contents of variable 'distro' after matching: RedHat
          

          Regarding lsb_release -i it finds info from /etc/lsb-release which, at least for common docker images, does not exist for the majority of distros. And the main reason why I'm checking /etc/os-release before {(/proc/version}} or uname -a is that in virtualized envs the latter two tend to print the docker VM's OS instead of the container's OS. So with the etc/os-release approach, we behave nicely in both cases.

          So if no objections I'll commit the one-line change outlined above.

          Show
          janhoy Jan Høydahl added a comment - Slurping in all of the files like this seems like a very bad idea Yes, that is bad, and indeed it turns out it is totally unnecessary, since all the distros have agreed to support /etc/os-release . That is a nice structured file, and all we need is the NAME= line. So I have done one change to one line and I think we're good: - proc_version=`cat /etc/*-release 2>/dev/ null ` + proc_version=$(grep -E "^NAME=" /etc/os-release 2>/dev/ null ) I tested this change across a number of distros by extracting the detection code into a script distro.sh and then running this command for os in ubuntu debian centos opensuse gidikern/rhel-oracle-jre; do echo "Testing on $os:" && docker run -it -v `pwd`/distro.sh:/tmp/distro.sh $os bash /tmp/distro.sh ; done Testing on ubuntu: Contents of variable 'proc_version' before matching: NAME= "Ubuntu" Contents of variable 'distro' after matching: Ubuntu Testing on debian: Contents of variable 'proc_version' before matching: NAME= "Debian GNU/Linux" Contents of variable 'distro' after matching: Debian Testing on centos: Contents of variable 'proc_version' before matching: NAME= "CentOS Linux" Contents of variable 'distro' after matching: CentOS Testing on opensuse: Contents of variable 'proc_version' before matching: NAME= "openSUSE Leap" Contents of variable 'distro' after matching: SUSE Testing on gidikern/rhel-oracle-jre: Contents of variable 'proc_version' before matching: NAME= "Red Hat Enterprise Linux Server" Contents of variable 'distro' after matching: RedHat Regarding lsb_release -i it finds info from /etc/lsb-release which, at least for common docker images, does not exist for the majority of distros. And the main reason why I'm checking /etc/os-release before {(/proc/version}} or uname -a is that in virtualized envs the latter two tend to print the docker VM's OS instead of the container's OS. So with the etc/os-release approach, we behave nicely in both cases. So if no objections I'll commit the one-line change outlined above.
          Hide
          janhoy Jan Høydahl added a comment -

          Attaching simple patch.

          Does this address your concerns Hoss Man?

          Show
          janhoy Jan Høydahl added a comment - Attaching simple patch. Does this address your concerns Hoss Man ?
          Hide
          hossman Hoss Man added a comment -

          Regarding lsb_release -i it finds info from /etc/lsb-release which, at least for common docker images, does not exist for the majority of distros. ...

          sure, but that was the crux of my point about checking for each of these sources in order to see if they match a known platform, rather then just checking to see if the commands execute (or the files exist) and then as soon as we find one that does, only check it – because that we could check lsb_release first, and even if it returns a valid status, but not a platform we recognize, we would then move on and check the rest...

          ... And the main reason why I'm checking /etc/os-release before /proc/version or uname -a is that in virtualized envs the latter two tend to print the docker VM's OS instead of the container's OS. ...

          that's an interesting property of docker I did not realize ... can you please add a comment to that effect in the install script?

          Does this address your concerns Hoss Man?

          I'd still feel better if we actually checked lsb_release -i (at least as a fall back since it's a documented standard), but i'm a big +1 to your latest patch since it definitely seems like a huge improvement.

          Show
          hossman Hoss Man added a comment - Regarding lsb_release -i it finds info from /etc/lsb-release which, at least for common docker images, does not exist for the majority of distros. ... sure, but that was the crux of my point about checking for each of these sources in order to see if they match a known platform, rather then just checking to see if the commands execute (or the files exist) and then as soon as we find one that does, only check it – because that we could check lsb_release first, and even if it returns a valid status, but not a platform we recognize, we would then move on and check the rest... ... And the main reason why I'm checking /etc/os-release before /proc/version or uname -a is that in virtualized envs the latter two tend to print the docker VM's OS instead of the container's OS. ... that's an interesting property of docker I did not realize ... can you please add a comment to that effect in the install script? Does this address your concerns Hoss Man? I'd still feel better if we actually checked lsb_release -i (at least as a fall back since it's a documented standard), but i'm a big +1 to your latest patch since it definitely seems like a huge improvement.
          Hide
          janhoy Jan Høydahl added a comment -

          New patch following Hoss' approach of doing one detection technique at a time, looking for our supported releases each time. Added lsb_release -i as the second approach. Also renamed a variable and went for case insensitive matching of distro name.

          Output from the same docker test as before:

          Testing on ubuntu:
          distro_str=NAME="Ubuntu"
          distro=Ubuntu
          Testing on debian:
          distro_str=NAME="Debian GNU/Linux"
          distro=Debian
          Testing on centos:
          distro_str=NAME="CentOS Linux"
          distro=CentOS
          Testing on opensuse:
          distro_str=NAME="openSUSE Leap"
          distro=SUSE
          Testing on gidikern/rhel-oracle-jre:
          distro_str=NAME="Red Hat Enterprise Linux Server"
          distro=RedHat
          

          Also made sure to test output from the other commands.

          Show
          janhoy Jan Høydahl added a comment - New patch following Hoss' approach of doing one detection technique at a time, looking for our supported releases each time. Added lsb_release -i as the second approach. Also renamed a variable and went for case insensitive matching of distro name. Output from the same docker test as before: Testing on ubuntu: distro_str=NAME="Ubuntu" distro=Ubuntu Testing on debian: distro_str=NAME="Debian GNU/Linux" distro=Debian Testing on centos: distro_str=NAME="CentOS Linux" distro=CentOS Testing on opensuse: distro_str=NAME="openSUSE Leap" distro=SUSE Testing on gidikern/rhel-oracle-jre: distro_str=NAME="Red Hat Enterprise Linux Server" distro=RedHat Also made sure to test output from the other commands.
          Hide
          hossman Hoss Man added a comment -

          jan: I didn't test it out, but your patch looks awesome!

          only problem is that the final ERROR when no supported distro is found still tries to refer to $proc_version ... maybe just make that "\nERROR: Unable to auto-detect your *NIX distribution!\nYou'll need to setup Solr as a service manually using the documentation provided in the Solr Reference Guide.\n" ?

          Show
          hossman Hoss Man added a comment - jan: I didn't test it out, but your patch looks awesome! only problem is that the final ERROR when no supported distro is found still tries to refer to $proc_version ... maybe just make that "\nERROR: Unable to auto-detect your *NIX distribution!\nYou'll need to setup Solr as a service manually using the documentation provided in the Solr Reference Guide.\n" ?
          Hide
          janhoy Jan Høydahl added a comment -

          Good catch. I'll commit with this wording

          Show
          janhoy Jan Høydahl added a comment - Good catch. I'll commit with this wording
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 2ad00826a3db17410a1bf70b4149d584fc4ddd02 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2ad0082 ]

          SOLR-9475: Imrpove distro detection by grepping /etc/os-release and adding lsb_release -i

          Show
          jira-bot ASF subversion and git services added a comment - Commit 2ad00826a3db17410a1bf70b4149d584fc4ddd02 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2ad0082 ] SOLR-9475 : Imrpove distro detection by grepping /etc/os-release and adding lsb_release -i
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit ba482681f3d12658b1591cef4a15ab75b4ccf605 in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ba48268 ]

          SOLR-9475: Imrpove distro detection by grepping /etc/os-release and adding lsb_release -i

          (cherry picked from commit 2ad0082)

          Show
          jira-bot ASF subversion and git services added a comment - Commit ba482681f3d12658b1591cef4a15ab75b4ccf605 in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ba48268 ] SOLR-9475 : Imrpove distro detection by grepping /etc/os-release and adding lsb_release -i (cherry picked from commit 2ad0082)
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Closing after 6.3.0 release.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.

            People

            • Assignee:
              janhoy Jan Høydahl
              Reporter:
              xcoder Nitin Surana
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development