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

Puppet stdlib should be automatically installed by toolchain

    Details

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

      Description

      Right now we have an explicit instruction for a user to install puppet stdlib before doing the deployment. This is a bad UX.

      I think the installation should be done in the bigtop_toolchain::deployment_tools

      1. 0001-BIGTOP-1693.-Puppet-stdlib-should-be-automatically-i.patch
        3 kB
        Konstantin Boudnik
      2. BIGTOP-1693.patch
        3 kB
        Konstantin Boudnik
      3. BIGTOP-1693.patch
        4 kB
        Konstantin Boudnik
      4. BIGTOP-1693.patch
        3 kB
        Konstantin Boudnik
      5. BIGTOP-1693.patch
        3 kB
        Konstantin Boudnik
      6. BIGTOP-1693.patch
        2 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Hide
          cos Konstantin Boudnik added a comment -

          Here's the patch that sets the stdiib module automatically.

          Show
          cos Konstantin Boudnik added a comment - Here's the patch that sets the stdiib module automatically.
          Hide
          cos Konstantin Boudnik added a comment -

          I have introduce new manifest puppet-modules so that if you only need to install the modules you can do it. It is also linked to the deployment-tools to set them all at once.

          Show
          cos Konstantin Boudnik added a comment - I have introduce new manifest puppet-modules so that if you only need to install the modules you can do it. It is also linked to the deployment-tools to set them all at once.
          Hide
          cos Konstantin Boudnik added a comment -

          Iteration two: now with hookup to the gradle buiild via toolchain-deployment task.

          Show
          cos Konstantin Boudnik added a comment - Iteration two: now with hookup to the gradle buiild via toolchain-deployment task.
          Hide
          cos Konstantin Boudnik added a comment -

          Adding missed ASL header into the new file.

          Show
          cos Konstantin Boudnik added a comment - Adding missed ASL header into the new file.
          Hide
          evans_ye Evans Ye added a comment -

          Hi Konstantin Boudnik, the patch is in git diff format instead of git format-patch format. Could you please reformat it?
          I didn't test it yet, but I have a comment:
          I think we don't need to create '/etc/puppet/modules/stdlib' by ourselves. The puppet install process will do us the favor. Besides that, it looks like the creates in 'install-puppet-stdlib' will prevent stdlib to be actually installed because the directory has been created by another puppet resource.

          Show
          evans_ye Evans Ye added a comment - Hi Konstantin Boudnik , the patch is in git diff format instead of git format-patch format. Could you please reformat it? I didn't test it yet, but I have a comment: I think we don't need to create '/etc/puppet/modules/stdlib' by ourselves. The puppet install process will do us the favor. Besides that, it looks like the creates in 'install-puppet-stdlib' will prevent stdlib to be actually installed because the directory has been created by another puppet resource.
          Hide
          cos Konstantin Boudnik added a comment -

          diff format works all the same, actually .Instead of git am you do git apply. But anyway - new patch is attached.

          As for the possible conflict of file and creates: I haven't discovered it in the testing, but looks like it works just fine without this file dependency, so let's keep simple. Thanks for the feedback.

          Show
          cos Konstantin Boudnik added a comment - diff format works all the same, actually .Instead of git am you do git apply . But anyway - new patch is attached. As for the possible conflict of file and creates : I haven't discovered it in the testing, but looks like it works just fine without this file dependency, so let's keep simple. Thanks for the feedback.
          Hide
          evans_ye Evans Ye added a comment - - edited

          I just thought that git am can preserve the author and commit messages
          In the newest patch, vagrant installation in deployment-tools.pp has been commented out. Is that intended?
          Sorry to bring this up late. I think originally the deployment_tools wrapper is used to setup a controller node for running docker or vm provisioner. Things putting in it might be vagrant, docker and some vagrant plugins installation recipes. Puppet modules should be a different story. Maybe we can have another wrapper called puppet_modules to install all the puppet modules. (besides stdlib, I've to add apt module installation in it). How do you think?

          Show
          evans_ye Evans Ye added a comment - - edited I just thought that git am can preserve the author and commit messages In the newest patch, vagrant installation in deployment-tools.pp has been commented out. Is that intended? Sorry to bring this up late. I think originally the deployment_tools wrapper is used to setup a controller node for running docker or vm provisioner. Things putting in it might be vagrant, docker and some vagrant plugins installation recipes. Puppet modules should be a different story. Maybe we can have another wrapper called puppet_modules to install all the puppet modules. (besides stdlib, I've to add apt module installation in it). How do you think?
          Hide
          cos Konstantin Boudnik added a comment -

          git am preserves these things. Which might be great when you commit other folks contributions - for review it might more of a hassle as you end up doing branches in your repo or hard-resetting your master, etc. Anyway...

          Commented out vagrant shouldn't be there of course - my bad, was just doing it to speed-up the testing. I actually was thinking of not hooking it up to the deployment manifest. So, I like the reassurance on that - new patch is attached. Thanks!

          Show
          cos Konstantin Boudnik added a comment - git am preserves these things. Which might be great when you commit other folks contributions - for review it might more of a hassle as you end up doing branches in your repo or hard-resetting your master, etc. Anyway... Commented out vagrant shouldn't be there of course - my bad, was just doing it to speed-up the testing. I actually was thinking of not hooking it up to the deployment manifest. So, I like the reassurance on that - new patch is attached. Thanks!
          Hide
          evans_ye Evans Ye added a comment -

          Tried the patch and it failed because of puppet package set ensure => '3.x':

          Error: Could not update: Failed to update to version 3.*, got version 3.5.1-1.el6 instead
          Wrapped exception:
          Failed to update to version 3.*, got version 3.5.1-1.el6 instead
          Error: /Stage[main]/Bigtop_toolchain::Puppet-modules/Package[puppet]/ensure: change from 3.6.0-1.el6 to 3.* failed: Could not update: Failed to update to version 3.*, got version 3.5.1-1.el6 instead
          Notice: /Stage[main]/Bigtop_toolchain::Puppet-modules/Exec[install-puppet-stdlib]: Dependency Package[puppet] has failures: true
          Warning: /Stage[main]/Bigtop_toolchain::Puppet-modules/Exec[install-puppet-stdlib]: Skipping because of failed dependencies
          

          This will keep downgrading puppet's version if executed repeatedly and the dependency failure is preventing stdlib to be actually installed.
          It looks like that official puppet does not support something like version >= 3.0 (https://tickets.puppetlabs.com/browse/PUP-1519) since it does not make sense in real world operations.

          Probably we should check puppet version by ourselves using something like this:

          [ `facter puppetversion |cut -d'.' -f 1` -ge 3 ]
          
          Show
          evans_ye Evans Ye added a comment - Tried the patch and it failed because of puppet package set ensure => '3.x' : Error: Could not update: Failed to update to version 3.*, got version 3.5.1-1.el6 instead Wrapped exception: Failed to update to version 3.*, got version 3.5.1-1.el6 instead Error: /Stage[main]/Bigtop_toolchain::Puppet-modules/Package[puppet]/ensure: change from 3.6.0-1.el6 to 3.* failed: Could not update: Failed to update to version 3.*, got version 3.5.1-1.el6 instead Notice: /Stage[main]/Bigtop_toolchain::Puppet-modules/Exec[install-puppet-stdlib]: Dependency Package[puppet] has failures: true Warning: /Stage[main]/Bigtop_toolchain::Puppet-modules/Exec[install-puppet-stdlib]: Skipping because of failed dependencies This will keep downgrading puppet's version if executed repeatedly and the dependency failure is preventing stdlib to be actually installed. It looks like that official puppet does not support something like version >= 3.0 ( https://tickets.puppetlabs.com/browse/PUP-1519 ) since it does not make sense in real world operations. Probably we should check puppet version by ourselves using something like this: [ `facter puppetversion |cut -d'.' -f 1` -ge 3 ]
          Hide
          cos Konstantin Boudnik added a comment -

          Weird... I think that check worked on my Ubuntu, but perhaps it was a false positive. Anyway, how about the latest version of the patch? Thanks for the facter idea!

          Show
          cos Konstantin Boudnik added a comment - Weird... I think that check worked on my Ubuntu, but perhaps it was a false positive. Anyway, how about the latest version of the patch? Thanks for the facter idea!
          Hide
          evans_ye Evans Ye added a comment -

          Sure. Latest would be ok as long as we have puppet 3 only documented. I've tried and found that other toolchain recies also do not support puppet 2. So we can just leave a message in toolchain readme.

          Show
          evans_ye Evans Ye added a comment - Sure. Latest would be ok as long as we have puppet 3 only documented. I've tried and found that other toolchain recies also do not support puppet 2. So we can just leave a message in toolchain readme.
          Hide
          cos Konstantin Boudnik added a comment -

          Perhaps then it would make sense to enforce Puppet 3x installation at the higher level somewhere? Eg perform a check and throw an Error instead of documenting the requirement? In my experience docs are rotting out pretty easily unless someone has a job to support them

          How do you feel about taking care of this enforcement in a separate JIRAs to avoid mixing different topics in one commit?

          Show
          cos Konstantin Boudnik added a comment - Perhaps then it would make sense to enforce Puppet 3x installation at the higher level somewhere? Eg perform a check and throw an Error instead of documenting the requirement? In my experience docs are rotting out pretty easily unless someone has a job to support them How do you feel about taking care of this enforcement in a separate JIRAs to avoid mixing different topics in one commit?
          Hide
          evans_ye Evans Ye added a comment -

          Yes. That would be awesome. Let's do it in another JIRA.

          Show
          evans_ye Evans Ye added a comment - Yes. That would be awesome. Let's do it in another JIRA.
          Hide
          cos Konstantin Boudnik added a comment -

          Is it a formal +1 then? Shall this one be committed?

          Show
          cos Konstantin Boudnik added a comment - Is it a formal +1 then? Shall this one be committed?
          Hide
          evans_ye Evans Ye added a comment -

          Here's my formal +1. sorry for arriving late. Let me commit this.

          Show
          evans_ye Evans Ye added a comment - Here's my formal +1. sorry for arriving late. Let me commit this.
          Hide
          evans_ye Evans Ye added a comment -

          Committed. Thanks for taking my opinions.

          Show
          evans_ye Evans Ye added a comment - Committed. Thanks for taking my opinions.
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks Evans! Appreciate the commit help! And thanks for your feedback - I think the new implementation is a way cleaner that my initial fumbling. I reckon my Puppet foo needs another year or two of polishing

          Show
          cos Konstantin Boudnik added a comment - Thanks Evans! Appreciate the commit help! And thanks for your feedback - I think the new implementation is a way cleaner that my initial fumbling. I reckon my Puppet foo needs another year or two of polishing

            People

            • Assignee:
              cos Konstantin Boudnik
              Reporter:
              cos Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development