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

puppet installation fails when components value is a single item instead of a list

    Details

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

      Description

      If the value for components is a single value such as "hive" instead of a list of values then the following error occurs:

      components is not an hash or array when accessing it with 0 at /var/tmp/bigtop-0.8.0/bigtop-deploy/puppet/manifests/cluster.pp:123

      1. BIGTOP-1553.patch
        11 kB
        jay vyas
      2. BIGTOP-1553.patch
        0.1 kB
        jay vyas
      3. BIGTOP-1553.patch
        11 kB
        jay vyas
      4. BIGTOP-1553.patch
        11 kB
        jay vyas
      5. BIGTOP-1553.patch
        12 kB
        jay vyas
      6. BIGTOP-1553.patch
        12 kB
        jay vyas
      7. BIGTOP-1553.patch
        12 kB
        jay vyas

        Issue Links

          Activity

          Hide
          cos Konstantin Boudnik added a comment -

          Yup, this is a known issue, which I was sorta willing to let go because normally (with exception of HDFS) you'd like to install more than a single component, right? If you have a different opinion a patch would be very welcome!

          Show
          cos Konstantin Boudnik added a comment - Yup, this is a known issue, which I was sorta willing to let go because normally (with exception of HDFS) you'd like to install more than a single component, right? If you have a different opinion a patch would be very welcome!
          Hide
          rleidle Rob Leidle added a comment -

          Don't have an opinion, just wanted to report it. Would be fine with the installation just printing out a nicer message and not supporting this use case? Can close by-design if we think it is fine behavior.

          Show
          rleidle Rob Leidle added a comment - Don't have an opinion, just wanted to report it. Would be fine with the installation just printing out a nicer message and not supporting this use case? Can close by-design if we think it is fine behavior.
          Hide
          cos Konstantin Boudnik added a comment -

          Nicer message would be certainly great! Do you care to provide a fix? I don't know Puppet that well, but the only thing I can think of is to do an extra check to see how many elements were read from the config. Any better ideas?

          Show
          cos Konstantin Boudnik added a comment - Nicer message would be certainly great! Do you care to provide a fix? I don't know Puppet that well, but the only thing I can think of is to do an extra check to see how many elements were read from the config. Any better ideas?
          Hide
          jayunit100 jay vyas added a comment -

          Yup I got snagged on this last wk also. definetely need to fail fast when only single component specified , or find a fix .

          Show
          jayunit100 jay vyas added a comment - Yup I got snagged on this last wk also. definetely need to fail fast when only single component specified , or find a fix .
          Hide
          jayunit100 jay vyas added a comment -

          Any objection to make this a general puppet error handling JIRA ?

          • add comments where it is complex (i.e. BIGTOP-1522 mapred dependency comment)
          • add comment aroudn the "components" backward compatibility convention components[0]==undef
          • add error message and warning around single component lists.

          Soudns okay w/ you Konstantin Boudnik ? or separate JIRAs for each?

          Show
          jayunit100 jay vyas added a comment - Any objection to make this a general puppet error handling JIRA ? add comments where it is complex (i.e. BIGTOP-1522 mapred dependency comment) add comment aroudn the "components" backward compatibility convention components [0] ==undef add error message and warning around single component lists. Soudns okay w/ you Konstantin Boudnik ? or separate JIRAs for each?
          Hide
          cos Konstantin Boudnik added a comment -

          I think it is good way going forward. Let's do it here!

          Show
          cos Konstantin Boudnik added a comment - I think it is good way going forward. Let's do it here!
          Hide
          jayunit100 jay vyas added a comment -

          okay will attach patch shortly!

          Show
          jayunit100 jay vyas added a comment - okay will attach patch shortly!
          Hide
          jayunit100 jay vyas added a comment - - edited

          Konstantin Boudnik Roman Shaposhnik and others interested in puppet. I dont know what to do here, need advise.

          Well, after some hacking around, i guess puppet core syntax really doesnt support any way to

          • validate x is an array OR
          • catch an arbitrary exception with a simple error message
            So really, the only way to provide any sanity to our users if the components length is 1, is import stdlib as a bigtop puppet dependency or add a new custom .pp function of our own.

          Should we
          1) Add puppet stdlib to bigtop ? It might enable other cleanup tasks in the future. We can easily validate strings, cast to arrays and so on w/ stdlib puppet dependencies. or
          2) Add a custom function for checking that components is an array ? Advantage here is no external dep, but yet another 10 lines of code we carry around and maintain

          Im no puppet exper either, so either is neutral for me. Just let me know !

          Show
          jayunit100 jay vyas added a comment - - edited Konstantin Boudnik Roman Shaposhnik and others interested in puppet. I dont know what to do here, need advise. Well, after some hacking around, i guess puppet core syntax really doesnt support any way to validate x is an array OR catch an arbitrary exception with a simple error message So really, the only way to provide any sanity to our users if the components length is 1, is import stdlib as a bigtop puppet dependency or add a new custom .pp function of our own. Should we 1) Add puppet stdlib to bigtop ? It might enable other cleanup tasks in the future. We can easily validate strings, cast to arrays and so on w/ stdlib puppet dependencies. or 2) Add a custom function for checking that components is an array ? Advantage here is no external dep, but yet another 10 lines of code we carry around and maintain Im no puppet exper either, so either is neutral for me. Just let me know !
          Hide
          jayunit100 jay vyas added a comment -

          from irc , we will go w/ stdlib . It will allow us to do stuff like any2array and other arbitrary logic in the puppet recipes.

          <cos1> and while you're adding the stdlib could you get rid of that split call in 
          <cos1>   $components                        = extlookup("components",    split($components, ","))
          
          Show
          jayunit100 jay vyas added a comment - from irc , we will go w/ stdlib . It will allow us to do stuff like any2array and other arbitrary logic in the puppet recipes. <cos1> and while you're adding the stdlib could you get rid of that split call in <cos1> $components = extlookup("components", split($components, ","))
          Hide
          jayunit100 jay vyas added a comment - - edited

          heres what i got so far, makes sense
          ?

          needs cleanup but just pasting what i think is going to work. testing now.

          • uses any2array from puppet stdlib, to convert whatever components are to an array.
          • declares the default components as all so that its explicit boolean value for installing full stack instead of that weird if predicate.
          • adds comments about the mapred dependency if block.
          • updates the vagrant and docker recipes so that they install stdlib for you, so people have easy to follow example (also adds note of the dependency to the puppet readme)
          Show
          jayunit100 jay vyas added a comment - - edited heres what i got so far, makes sense ? needs cleanup but just pasting what i think is going to work. testing now. uses any2array from puppet stdlib, to convert whatever components are to an array. declares the default components as all so that its explicit boolean value for installing full stack instead of that weird if predicate. adds comments about the mapred dependency if block. updates the vagrant and docker recipes so that they install stdlib for you, so people have easy to follow example (also adds note of the dependency to the puppet readme)
          Hide
          jayunit100 jay vyas added a comment - - edited

          Okay patch attached. i did a simple test, seems to work.

          let me know if it looks good ... or if it needs more work.

          if good i can commit it, but just FYI i only did a very small test

          Show
          jayunit100 jay vyas added a comment - - edited Okay patch attached. i did a simple test, seems to work. let me know if it looks good ... or if it needs more work. if good i can commit it, but just FYI i only did a very small test
          Hide
          jayunit100 jay vyas added a comment -

          cleaned up patch attached, removed an extraneous commit

          Show
          jayunit100 jay vyas added a comment - cleaned up patch attached, removed an extraneous commit
          Hide
          cos Konstantin Boudnik added a comment -

          Good stuff!
          I see that you're doing the 2nd time

          +  # See above explanation of the $all convention. 
          +  $all = ($components[0]  == undef)
          

          What's the reason?

          Also, in a few places you do
          + if ($all == undef or "sqoop" in $components) {
          while in the most others you're simply checking the value of $all. Looks like a context replacement issue. Please make it uniformed if possible.

          Show
          cos Konstantin Boudnik added a comment - Good stuff! I see that you're doing the 2nd time + # See above explanation of the $all convention. + $all = ($components[0] == undef) What's the reason? Also, in a few places you do + if ($all == undef or "sqoop" in $components) { while in the most others you're simply checking the value of $all . Looks like a context replacement issue. Please make it uniformed if possible.
          Hide
          jayunit100 jay vyas added a comment -

          yup, i caught that guy that earlier as well, patch reattached, testing now;

          Show
          jayunit100 jay vyas added a comment - yup, i caught that guy that earlier as well, patch reattached, testing now;
          Hide
          jayunit100 jay vyas added a comment - - edited

          here we go, cleaned up, whitespace fixed

          ready for final review Konstantin Boudnik

          thanks !

          Show
          jayunit100 jay vyas added a comment - - edited here we go, cleaned up, whitespace fixed ready for final review Konstantin Boudnik thanks !
          Hide
          jayunit100 jay vyas added a comment -

          Rebased against the tachyon fixes. ready for final review again

          Show
          jayunit100 jay vyas added a comment - Rebased against the tachyon fixes. ready for final review again
          Hide
          rvs Roman Shaposhnik added a comment -

          +1. Please commit.

          Show
          rvs Roman Shaposhnik added a comment - +1. Please commit.
          Hide
          jayunit100 jay vyas added a comment - - edited

          okay ! great. now bigtop will support an individual component on puppet install...

          Thanks for catching and reporting this one Rob Leidle . this is one of those things thats easy to ignore !

          we willwant to do some more testing on this before release of 0.9, but i think now any component combination should work

          Show
          jayunit100 jay vyas added a comment - - edited okay ! great. now bigtop will support an individual component on puppet install... Thanks for catching and reporting this one Rob Leidle . this is one of those things thats easy to ignore ! we willwant to do some more testing on this before release of 0.9, but i think now any component combination should work

            People

            • Assignee:
              jayunit100 jay vyas
              Reporter:
              rleidle Rob Leidle
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 2h
                2h
                Remaining:
                Remaining Estimate - 2h
                2h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development