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

Add Vagrant installation to bigtop_toolchain

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 1.0.0
    • Component/s: build
    • Labels:
      None

      Description

      As Konstantin Boudnik's comment in BIGTOP-1564, this jira is going to "download the package and install it if the local vagrant version is lesser than expected".
      The patch will add a puppet recipe to automatically install Vagrant, helping to setup environment for running docker provisioner.

      1. BIGTOP-1576.1.patch
        3 kB
        Evans Ye
      2. BIGTOP-1576.2.patch
        6 kB
        Evans Ye
      3. BIGTOP-1576.3.patch
        4 kB
        Evans Ye

        Issue Links

          Activity

          Hide
          rvs Roman Shaposhnik added a comment -

          Apologies for being slow, but could you provide a little bit more context?

          Show
          rvs Roman Shaposhnik added a comment - Apologies for being slow, but could you provide a little bit more context?
          Hide
          evans_ye Evans Ye added a comment -

          Roman Shaposhnik, sorry for giving a vague description at the very beginning.
          This jira is about to add Vagrant installation in bigtop_toolchain.
          The first version of patch is available now, feel free to give comments on it.

          Show
          evans_ye Evans Ye added a comment - Roman Shaposhnik , sorry for giving a vague description at the very beginning. This jira is about to add Vagrant installation in bigtop_toolchain . The first version of patch is available now, feel free to give comments on it.
          Hide
          rvs Roman Shaposhnik added a comment -

          Oh, I see now. One quick concern, I think we need to start differentiating between stuff that we can't build Bigtop without vs. optional stuff in our toolchain. IOW, perhaps this should be conditional.

          Show
          rvs Roman Shaposhnik added a comment - Oh, I see now. One quick concern, I think we need to start differentiating between stuff that we can't build Bigtop without vs. optional stuff in our toolchain. IOW, perhaps this should be conditional.
          Hide
          cos Konstantin Boudnik added a comment -

          Yup, good point, Roman Shaposhnik

          Show
          cos Konstantin Boudnik added a comment - Yup, good point, Roman Shaposhnik
          Hide
          jayunit100 jay vyas added a comment - - edited

          Roman Shaposhnik Konstantin Boudnik to add more context.

          How this sort of off-beat idea came about

          • When roman was travelling, we discussed how for the CI we could easily use the bigtop-deploy/vm/docker-puppet recipes evans wrote, to test stuff.
          • At that point, iirc, cos mentioned : We should add vagrant to bigtop toolchain, so installation was taken for granted and we could leverage that wherever we wanted to, however we wanted to.

          My 2 cents

          Although not required per se itd be great to run the vagrant-docker recipes in the CI , after all

          • thats the easiest way for users to spin up bigtop ,and so good to autotest it works perfectly.
          • and now that we can run vagrant to read rpms from output/ , vagrant-docker can be used to test the components that are built from source, for free , so saves us some duplicate work on the CI.

          In any case — this is mostly Roman Shaposhnik's decision i think, as he's the ci maintainer right now. so he knows best wether it will be something he would like to use in jenkins.

          Show
          jayunit100 jay vyas added a comment - - edited Roman Shaposhnik Konstantin Boudnik to add more context. How this sort of off-beat idea came about When roman was travelling, we discussed how for the CI we could easily use the bigtop-deploy/vm/docker-puppet recipes evans wrote, to test stuff. At that point, iirc, cos mentioned : We should add vagrant to bigtop toolchain, so installation was taken for granted and we could leverage that wherever we wanted to, however we wanted to. My 2 cents Although not required per se itd be great to run the vagrant-docker recipes in the CI , after all thats the easiest way for users to spin up bigtop ,and so good to autotest it works perfectly. and now that we can run vagrant to read rpms from output/ , vagrant-docker can be used to test the components that are built from source , for free , so saves us some duplicate work on the CI. In any case — this is mostly Roman Shaposhnik 's decision i think, as he's the ci maintainer right now. so he knows best wether it will be something he would like to use in jenkins.
          Hide
          cos Konstantin Boudnik added a comment -

          I believe there are two different issues now:

          • adding vagrant to toolchain, so that someone can rely on it for getting correct version of vagrant and then being able to deploy a cluster to docker or else using Puppet recipes
          • how CI should do the deployment.
            While the former can contribute to the latter, I don't think we should just use them interchangeably. Roman's worry is about the size of the container, which toolbox's stuff contributes to quite a bit. And in the reality, vagrant won't add anything to how we build stack. That's why I think making it an optional toolchain dependency would make sense.
          Show
          cos Konstantin Boudnik added a comment - I believe there are two different issues now: adding vagrant to toolchain, so that someone can rely on it for getting correct version of vagrant and then being able to deploy a cluster to docker or else using Puppet recipes how CI should do the deployment. While the former can contribute to the latter, I don't think we should just use them interchangeably. Roman's worry is about the size of the container, which toolbox's stuff contributes to quite a bit. And in the reality, vagrant won't add anything to how we build stack. That's why I think making it an optional toolchain dependency would make sense.
          Hide
          jayunit100 jay vyas added a comment - - edited

          ah yes ! the toolchain is specifically for building packages and nothing else, then i totally agree, and maybe even suggest leave vagrant out of it... and if we want helper utils for vagrant stuff, we can create a different utility alltogether for creating "dev environments" which might have things like vagrant or whatever.

          I agree - keep the toolchain minimal. its the core description of bigtop infra.

          Show
          jayunit100 jay vyas added a comment - - edited ah yes ! the toolchain is specifically for building packages and nothing else, then i totally agree, and maybe even suggest leave vagrant out of it... and if we want helper utils for vagrant stuff, we can create a different utility alltogether for creating "dev environments" which might have things like vagrant or whatever. I agree - keep the toolchain minimal . its the core description of bigtop infra.
          Hide
          cos Konstantin Boudnik added a comment -

          I think it's a mea culpa: I have expressed a half-ass idea about adding this to the toolchain without much thinking. How about a somewhat better idea: what if we add a new manifest to the toolchain, that will not be tie-up into the build-specific toolchain, yet will be providing us with whatever we might need for deployment, etc.? Say, the build stuff is getting added via installer.pp manifest. What if we add deployment-tools.pp to handle other tools?

          Show
          cos Konstantin Boudnik added a comment - I think it's a mea culpa: I have expressed a half-ass idea about adding this to the toolchain without much thinking. How about a somewhat better idea: what if we add a new manifest to the toolchain, that will not be tie-up into the build-specific toolchain, yet will be providing us with whatever we might need for deployment, etc.? Say, the build stuff is getting added via installer.pp manifest. What if we add deployment-tools.pp to handle other tools?
          Hide
          rvs Roman Shaposhnik added a comment -

          Well, that's how toolchain got started – absolutely minimum amount of stuff to be provisioned on Jenkins slaves (and later containers). Doesn't mean we can't extend it to configure Developer environment as well. All I am saying is: if we do we need to make it configurable.

          Show
          rvs Roman Shaposhnik added a comment - Well, that's how toolchain got started – absolutely minimum amount of stuff to be provisioned on Jenkins slaves (and later containers). Doesn't mean we can't extend it to configure Developer environment as well. All I am saying is: if we do we need to make it configurable.
          Hide
          cos Konstantin Boudnik added a comment -

          And I think a separate manifest will address it while keeping all the toolchain stuff at the same place, won't it?

          Show
          cos Konstantin Boudnik added a comment - And I think a separate manifest will address it while keeping all the toolchain stuff at the same place, won't it?
          Hide
          evans_ye Evans Ye added a comment -

          Good points. I'll take the way to add a separated manifest deployment-tools.pp and supplement the readme to clarify what has been added and how to use it.

          Show
          evans_ye Evans Ye added a comment - Good points. I'll take the way to add a separated manifest deployment-tools.pp and supplement the readme to clarify what has been added and how to use it.
          Hide
          evans_ye Evans Ye added a comment -

          Got a refined patch out.
          Now the deployment-tools is separated from installer.pp(which is mainly for building packages).
          The readme has been updated as well so that it can better illustrate the usage.

          Show
          evans_ye Evans Ye added a comment - Got a refined patch out. Now the deployment-tools is separated from installer.pp (which is mainly for building packages). The readme has been updated as well so that it can better illustrate the usage.
          Hide
          cos Konstantin Boudnik added a comment -

          There seems to be an unrelated change in the license boilerplate in README.md - let's not add it to the patch. Other than that it seems to be fine. I've tried to run it and all seems to be working just fine. Also, could you please submit the patch in 'git format-patch' form to make it easier for me to commit?

          +1 pending the corrections above.

          Show
          cos Konstantin Boudnik added a comment - There seems to be an unrelated change in the license boilerplate in README.md - let's not add it to the patch. Other than that it seems to be fine. I've tried to run it and all seems to be working just fine. Also, could you please submit the patch in 'git format-patch' form to make it easier for me to commit? +1 pending the corrections above.
          Hide
          evans_ye Evans Ye added a comment -

          Sure, update the patch right away.

          Show
          evans_ye Evans Ye added a comment - Sure, update the patch right away.
          Hide
          evans_ye Evans Ye added a comment -

          Uploaded patch version 3.
          I've removed the unrelated change in license and make sure the patch is generated by git format-patch.
          Kindly seek for your review, Konstantin Boudnik.

          Show
          evans_ye Evans Ye added a comment - Uploaded patch version 3. I've removed the unrelated change in license and make sure the patch is generated by git format-patch. Kindly seek for your review, Konstantin Boudnik .
          Hide
          cos Konstantin Boudnik added a comment -

          +1 I will commit it shortly.
          The only small thing: the word "menifests" is misspelled. I will correct it before committing.

          Show
          cos Konstantin Boudnik added a comment - +1 I will commit it shortly. The only small thing: the word "menifests" is misspelled. I will correct it before committing.
          Hide
          cos Konstantin Boudnik added a comment -

          Pushed to master as
          bc80fd4..a0035a3 HEAD -> master
          Thanks Evans!

          Show
          cos Konstantin Boudnik added a comment - Pushed to master as bc80fd4..a0035a3 HEAD -> master Thanks Evans!
          Hide
          evans_ye Evans Ye added a comment -

          Oh, sorry about the typo and thanks for getting this in!

          Show
          evans_ye Evans Ye added a comment - Oh, sorry about the typo and thanks for getting this in!

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development