Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 1.0.0
    • Component/s: debian, deployment
    • Labels:
      None

      Description

      Our puppet recipes currently only implement yum repo auto deploy. we shall have deb repo being automatically set up as well.

      1. BIGTOP-1676.1.patch
        4 kB
        Evans Ye
      2. BIGTOP-1676.2.patch
        4 kB
        Evans Ye
      3. BIGTOP-1676.3.patch
        5 kB
        Evans Ye
      4. BIGTOP-1676.4.patch
        5 kB
        Evans Ye

        Activity

        Hide
        cos Konstantin Boudnik added a comment -

        That'd be great!

        Show
        cos Konstantin Boudnik added a comment - That'd be great!
        Hide
        evans_ye Evans Ye added a comment - - edited

        The patch has 2 things:

        • implement deb repo auto deploy by utilizing puppetlab-apt module
        • fix wrong array representation for hadoop-zookeeper::server::ensemble in hieradata/bigtop/cluster.yaml
        Show
        evans_ye Evans Ye added a comment - - edited The patch has 2 things: implement deb repo auto deploy by utilizing puppetlab-apt module fix wrong array representation for hadoop-zookeeper::server::ensemble in hieradata/bigtop/cluster.yaml
        Hide
        michaelweiser Michael Weiser added a comment -

        Hi Evans Ye,
        good catch with the zookeeper ensemble. If I were a voter, you'd have my +1. Just like to point out to avoid confusion that ensemble still is an array but I did the encoding in cluster.yaml wrong, making the array member an array itself.

        Show
        michaelweiser Michael Weiser added a comment - Hi Evans Ye , good catch with the zookeeper ensemble. If I were a voter, you'd have my +1. Just like to point out to avoid confusion that ensemble still is an array but I did the encoding in cluster.yaml wrong, making the array member an array itself.
        Hide
        cos Konstantin Boudnik added a comment -

        Actually, Michael Weiser, your patch reviews and voting on them is highly encouraged. The vote isn't binding, but it helps to improve the code of the patches and allows people to learn more about different parts of the project. Thanks a lot for looking at it!

        Show
        cos Konstantin Boudnik added a comment - Actually, Michael Weiser , your patch reviews and voting on them is highly encouraged. The vote isn't binding, but it helps to improve the code of the patches and allows people to learn more about different parts of the project. Thanks a lot for looking at it!
        Hide
        cos Konstantin Boudnik added a comment -

        No to be pedantic, Evans Ye, but is ZK change related to the deb repo patch? If not - can we split them into separate JIRAs for clarity? Thanks!

        Show
        cos Konstantin Boudnik added a comment - No to be pedantic, Evans Ye , but is ZK change related to the deb repo patch? If not - can we split them into separate JIRAs for clarity? Thanks!
        Hide
        evans_ye Evans Ye added a comment -

        Thanks for the feedback Michael Weiser.
        Konstantin Boudnik you're absolutely right! These two things are independent. Sorry to mix them up. I'll separate out zookeeper to another JIRA.

        Show
        evans_ye Evans Ye added a comment - Thanks for the feedback Michael Weiser . Konstantin Boudnik you're absolutely right! These two things are independent. Sorry to mix them up. I'll separate out zookeeper to another JIRA.
        Hide
        evans_ye Evans Ye added a comment -

        Patch 2 uploaded, which eliminated the zookeeper fix. The patch now are all necessary changes to enable deb repo deployment.

        Show
        evans_ye Evans Ye added a comment - Patch 2 uploaded, which eliminated the zookeeper fix. The patch now are all necessary changes to enable deb repo deployment.
        Hide
        cos Konstantin Boudnik added a comment -

        +1 thanks!

        Show
        cos Konstantin Boudnik added a comment - +1 thanks!
        Hide
        evans_ye Evans Ye added a comment -

        Upload patch 3 to fix an issue that centos deployment failed after patch 2 applied. Should work on both side now

        Show
        evans_ye Evans Ye added a comment - Upload patch 3 to fix an issue that centos deployment failed after patch 2 applied. Should work on both side now
        Hide
        cos Konstantin Boudnik added a comment -

        Looks almost good. One last question. Doing this

             default: {
        -      notify{"WARNING: running on a non-yum platform -- make sure Bigtop repo is setup": }
        +       class { "apt": disable_keys => true }
        +       apt::source { "Bigtop":
        

        would also try to run apt-get on Suse, right? And it will of-course fail. Shall we explicitly tie the use of apt to debian-based systems?

        Show
        cos Konstantin Boudnik added a comment - Looks almost good. One last question. Doing this default : { - notify{ "WARNING: running on a non-yum platform -- make sure Bigtop repo is setup" : } + class { "apt" : disable_keys => true } + apt::source { "Bigtop" : would also try to run apt-get on Suse, right? And it will of-course fail. Shall we explicitly tie the use of apt to debian-based systems?
        Hide
        evans_ye Evans Ye added a comment -

        Yup! Good catch. Upload Patch 4 to explicitly tie the apt setup code snippets to Ubuntu and Debian Linux Distro. Thanks for the review Konstantin Boudnik.

        Show
        evans_ye Evans Ye added a comment - Yup! Good catch. Upload Patch 4 to explicitly tie the apt setup code snippets to Ubuntu and Debian Linux Distro. Thanks for the review Konstantin Boudnik .
        Hide
        cos Konstantin Boudnik added a comment -

        Looks good +1
        Please commit. Thanks!

        Show
        cos Konstantin Boudnik added a comment - Looks good +1 Please commit. Thanks!
        Hide
        evans_ye Evans Ye added a comment -

        Okay, committed.

        Show
        evans_ye Evans Ye added a comment - Okay, committed.

          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