Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-2775

Make open jdk 8 available on bigtop/puppet:debian-8

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.1
    • Component/s: docker
    • Labels:
      None

      Description

      It seems we need to grap openjdk 8 from debian jessie-backports repo.
      The bigtop/slaves image has addressed the issue by adding backports repo in bigtop_toolchain/manifests/.jdk.pp, however, bigtop/puppet:debian-8 for Bigtop Docker Provisioner and Sandbox has openjdk 8 missing.

      1. 0001-BIGTOP-2775-Make-open-jdk-8-available-on-bigtop-pupp.patch
        9 kB
        Olaf Flebbe
      2. BIGTOP-2775.3.patch
        14 kB
        Olaf Flebbe
      3. BIGTOP-2775.4.patch
        14 kB
        Evans Ye
      4. BIGTOP-2775.patch
        12 kB
        Evans Ye

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user evans-ye opened a pull request:

          https://github.com/apache/bigtop/pull/215

          BIGTOP-2775. Make open jdk 8 available on bigtop/puppet:debian-8

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/evans-ye/bigtop BIGTOP-2775

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/bigtop/pull/215.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #215


          commit 77c81a6c7f7c7f2943bc631d16729c2452bf9aaf
          Author: Evans Ye <evansye@apache.org>
          Date: 2017-05-23T15:54:50Z

          BIGTOP-2775. Make open jdk 8 available on bigtop/puppet:debian-8


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user evans-ye opened a pull request: https://github.com/apache/bigtop/pull/215 BIGTOP-2775 . Make open jdk 8 available on bigtop/puppet:debian-8 You can merge this pull request into a Git repository by running: $ git pull https://github.com/evans-ye/bigtop BIGTOP-2775 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/bigtop/pull/215.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #215 commit 77c81a6c7f7c7f2943bc631d16729c2452bf9aaf Author: Evans Ye <evansye@apache.org> Date: 2017-05-23T15:54:50Z BIGTOP-2775 . Make open jdk 8 available on bigtop/puppet:debian-8
          Hide
          evans_ye Evans Ye added a comment -

          Olaf Flebbe I'm sure you have abundant knowledge for this. Can you provide some professional suggestion?

          Show
          evans_ye Evans Ye added a comment - Olaf Flebbe I'm sure you have abundant knowledge for this. Can you provide some professional suggestion?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on the issue:

          https://github.com/apache/bigtop/pull/215

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on the issue: https://github.com/apache/bigtop/pull/215 +1
          Hide
          oflebbe Olaf Flebbe added a comment -

          I do not think this pull request 215 will work.

          Show
          oflebbe Olaf Flebbe added a comment - I do not think this pull request 215 will work.
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          From my point of view it is very questionable how we provide the JDK at deploy time the way we do.

          I would recommend to run the jdk.pp at deploy time. We should add it to the deploy puppet stack as a prerequisite.

          If you need me to make a concrete patch proposal, let me know.

          The jdk version should be made configurable as well as other parameters at hiera level

          Show
          oflebbe Olaf Flebbe added a comment - - edited From my point of view it is very questionable how we provide the JDK at deploy time the way we do. I would recommend to run the jdk.pp at deploy time. We should add it to the deploy puppet stack as a prerequisite. If you need me to make a concrete patch proposal, let me know. The jdk version should be made configurable as well as other parameters at hiera level
          Hide
          evans_ye Evans Ye added a comment -

          I think I know what you're purposing.

          Currently we rely on bigtop-deploy Puppet recipes to install desired JDK for us at deploy time.
          The JDK version is configurable in site.yaml.

          Personally I think that backports in toolchain is like a workaround, hence I want to make a minimal or no dependency on it.
          What's your take on this?

          Show
          evans_ye Evans Ye added a comment - I think I know what you're purposing. Currently we rely on bigtop-deploy Puppet recipes to install desired JDK for us at deploy time. The JDK version is configurable in site.yaml. Personally I think that backports in toolchain is like a workaround, hence I want to make a minimal or no dependency on it. What's your take on this?
          Hide
          oflebbe Olaf Flebbe added a comment -

          really ?

          AFAIK the jdk for the provisioner is configured by

          /Users/olaf/Devel/bigtop/provisioner/docker/config.yaml:jdk: "java-1.8.0-openjdk-devel.x86_64"
          /Users/olaf/Devel/bigtop/provisioner/docker/config_centos6.yaml:jdk: "java-1.8.0-openjdk-devel.x86_64"
          /Users/olaf/Devel/bigtop/provisioner/docker/config_centos7.yaml:jdk: "java-1.8.0-openjdk-devel.x86_64"
          /Users/olaf/Devel/bigtop/provisioner/docker/config_debian8.yaml:jdk: "openjdk-8-jdk"
          /Users/olaf/Devel/bigtop/provisioner/docker/config_ubuntu_xenial.yaml:jdk: "openjdk-8-jdk"
          

          I would propose to get rid of this and reuse jdk.pp from toolchain at deploy time.

          The backports repository in toolchain is not a workaround. It is needed for debian-8 (jessie) since there was no stable java8 for debian at jessie release time. It is debian release policy not to upgrade packages within a stable distro, well almost no.

          In debian-9 (sid) it will not be necessary any more. The code in toolchain is already prepared for debian-9.

          Show
          oflebbe Olaf Flebbe added a comment - really ? AFAIK the jdk for the provisioner is configured by /Users/olaf/Devel/bigtop/provisioner/docker/config.yaml:jdk: "java-1.8.0-openjdk-devel.x86_64" /Users/olaf/Devel/bigtop/provisioner/docker/config_centos6.yaml:jdk: "java-1.8.0-openjdk-devel.x86_64" /Users/olaf/Devel/bigtop/provisioner/docker/config_centos7.yaml:jdk: "java-1.8.0-openjdk-devel.x86_64" /Users/olaf/Devel/bigtop/provisioner/docker/config_debian8.yaml:jdk: "openjdk-8-jdk" /Users/olaf/Devel/bigtop/provisioner/docker/config_ubuntu_xenial.yaml:jdk: "openjdk-8-jdk" I would propose to get rid of this and reuse jdk.pp from toolchain at deploy time. The backports repository in toolchain is not a workaround. It is needed for debian-8 (jessie) since there was no stable java8 for debian at jessie release time. It is debian release policy not to upgrade packages within a stable distro, well almost no. In debian-9 (sid) it will not be necessary any more. The code in toolchain is already prepared for debian-9.
          Hide
          oflebbe Olaf Flebbe added a comment -

          I am a little bit in a hurry since I am out for a trip the next few days. Don't know if I am ready this evening.

          Show
          oflebbe Olaf Flebbe added a comment - I am a little bit in a hurry since I am out for a trip the next few days. Don't know if I am ready this evening.
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          Ok let me look at the actual code. Right now we have the option that the user knows what he does and he choose to install a jdk by other means. That functionality should stay.

          The other option should be to install the jdk we use at compile time, I think

          Show
          oflebbe Olaf Flebbe added a comment - - edited Ok let me look at the actual code. Right now we have the option that the user knows what he does and he choose to install a jdk by other means. That functionality should stay. The other option should be to install the jdk we use at compile time, I think
          Hide
          oflebbe Olaf Flebbe added a comment -

          Ah I see the jdk I am refering to is edited by docker-hadoop.sh into site.yaml. I do not like this concept since it is not expressive enough to have backports installed as a prerequisite. This is the point where we are coming from.

          Show
          oflebbe Olaf Flebbe added a comment - Ah I see the jdk I am refering to is edited by docker-hadoop.sh into site.yaml. I do not like this concept since it is not expressive enough to have backports installed as a prerequisite. This is the point where we are coming from.
          Hide
          oflebbe Olaf Flebbe added a comment -

          Well, this is going to be interesting... will have to tweak the jdk.pp for the provisioner to work. Will have to backport it to toolchain later ..

          Show
          oflebbe Olaf Flebbe added a comment - Well, this is going to be interesting... will have to tweak the jdk.pp for the provisioner to work. Will have to backport it to toolchain later ..
          Hide
          oflebbe Olaf Flebbe added a comment -

          This turned out more complicated, since we have some really weird puppet constructs within our codebase, giving circular dependencies.

          This worked for me once on debian, should be tested on other platforms as well ...

          What do you think?

          Show
          oflebbe Olaf Flebbe added a comment - This turned out more complicated, since we have some really weird puppet constructs within our codebase, giving circular dependencies. This worked for me once on debian, should be tested on other platforms as well ... What do you think?
          Hide
          evans_ye Evans Ye added a comment -

          Okay, so your solution is to properly install jdk by bigtop puppet.
          I think it's good!
          The only concern I have is the hard-coded jdk 1.8 package names.
          Either for RPM and DEB, I think we should extract out an variable for them in hiera so that we're good for future upgrades.

          Show
          evans_ye Evans Ye added a comment - Okay, so your solution is to properly install jdk by bigtop puppet. I think it's good! The only concern I have is the hard-coded jdk 1.8 package names. Either for RPM and DEB, I think we should extract out an variable for them in hiera so that we're good for future upgrades.
          Hide
          oflebbe Olaf Flebbe added a comment -

          Evans Ye I understand your concerns, but I see how we use a single variable to note something like – a jdk9 prerelease from oracle adapted to ... - We an only support one JDK, the use to compile. Period. That is the usecase for 99,99% of our users, I think.

          If someone likes to have his preferred JDK installed. He should do it and use the switch

          bigtop::jdk_preinstalled: true
          
          Show
          oflebbe Olaf Flebbe added a comment - Evans Ye I understand your concerns, but I see how we use a single variable to note something like – a jdk9 prerelease from oracle adapted to ... - We an only support one JDK, the use to compile. Period. That is the usecase for 99,99% of our users, I think. If someone likes to have his preferred JDK installed. He should do it and use the switch bigtop::jdk_preinstalled: true
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          Did you test my patch on centos or fedora ?

          Evans Ye Should I commit it ?

          Show
          oflebbe Olaf Flebbe added a comment - - edited Did you test my patch on centos or fedora ? Evans Ye Should I commit it ?
          Hide
          evans_ye Evans Ye added a comment -

          Okay. I still think it's better to separate them out, but it a tiny thing. The patch has been tested by Provisioner manually with centos-6 and debian-8 provisnioed successfully. However, I have made slightly changes on your patch:

          • Stripped white spaces
          • Remove jdk in both Docker and Vagrant Provisioner

          The updated patch attached.

          Show
          evans_ye Evans Ye added a comment - Okay. I still think it's better to separate them out, but it a tiny thing. The patch has been tested by Provisioner manually with centos-6 and debian-8 provisnioed successfully. However, I have made slightly changes on your patch: Stripped white spaces Remove jdk in both Docker and Vagrant Provisioner The updated patch attached.
          Hide
          evans_ye Evans Ye added a comment -

          Olaf Flebbe are you good with the changes?

          Show
          evans_ye Evans Ye added a comment - Olaf Flebbe are you good with the changes?
          Hide
          oflebbe Olaf Flebbe added a comment -

          Evans Ye : LGTM. Thanks for improving.

          Show
          oflebbe Olaf Flebbe added a comment - Evans Ye : LGTM. Thanks for improving.
          Hide
          evans_ye Evans Ye added a comment -

          I've double tested this. This will introduce failures since the require bigtop_repo will be executed AFTER components' deployment.
          Any idea to make it executed first?

          Show
          evans_ye Evans Ye added a comment - I've double tested this. This will introduce failures since the require bigtop_repo will be executed AFTER components' deployment. Any idea to make it executed first?
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          I thought I already resolved that issue before. Could you please tell me on what platform you observed the problem ?

          Oh I see. the problem. The require should go into the node

          { }

          , and we should fix a a potential rat problem. Testing right now.

          Show
          oflebbe Olaf Flebbe added a comment - - edited I thought I already resolved that issue before. Could you please tell me on what platform you observed the problem ? Oh I see. the problem. The require should go into the node { } , and we should fix a a potential rat problem. Testing right now.
          Hide
          oflebbe Olaf Flebbe added a comment -

          Reworked the patch. Tested on macos for debian8 and centos7

          Show
          oflebbe Olaf Flebbe added a comment - Reworked the patch. Tested on macos for debian8 and centos7
          Hide
          oflebbe Olaf Flebbe added a comment -

          Evans Ye Thanks for your patience. Would you try this patch again, too?

          Show
          oflebbe Olaf Flebbe added a comment - Evans Ye Thanks for your patience. Would you try this patch again, too?
          Hide
          evans_ye Evans Ye added a comment -

          Hi Olaf Flebbe,
          My tests against BIGTOP-2775.3.patch are still failing, but I think I've figured out the problems.

          • Apt::Source<||> -> Exec['bigtop-apt-update'] -> Package<||> is needed to make sure repo is ready for all following installations.
          • Class['jdk'] -> Service<||> is required so that before daemons to start there will be jdk properly installed.

          Patch BIGTOP-2775.4.patch uploaded.
          I've tested it on CentOS 6 and Debian 8.
          Can you double confirm?

          Show
          evans_ye Evans Ye added a comment - Hi Olaf Flebbe , My tests against BIGTOP-2775 .3.patch are still failing, but I think I've figured out the problems. Apt::Source<||> -> Exec ['bigtop-apt-update'] -> Package<||> is needed to make sure repo is ready for all following installations. Class ['jdk'] -> Service<||> is required so that before daemons to start there will be jdk properly installed. Patch BIGTOP-2775 .4.patch uploaded. I've tested it on CentOS 6 and Debian 8. Can you double confirm?
          Hide
          oflebbe Olaf Flebbe added a comment -

          Works for me as well. Will commit this in a minute

          Show
          oflebbe Olaf Flebbe added a comment - Works for me as well. Will commit this in a minute
          Hide
          oflebbe Olaf Flebbe added a comment -

          Evans Ye It was a pleasure for me to resolve problems together. Thank you very much.

          Show
          oflebbe Olaf Flebbe added a comment - Evans Ye It was a pleasure for me to resolve problems together. Thank you very much.

            People

            • Assignee:
              evans_ye Evans Ye
              Reporter:
              evans_ye Evans Ye
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development