Re: [Puppet Users] if / case loop syntax help

54 views
Skip to first unread message

warron.french

unread,
May 1, 2017, 10:21:46 PM5/1/17
to puppet...@googlegroups.com
Samir, are you aware of the slack.puppet.com website?  It is free and you can chat/IM with people real-time about your code.  That's the first thing.

Secondly are you aware of the Puppet.com pages for code writing?  Check this page: https://docs.puppet.com/puppet/4.9/lang_conditional.html or this page - https://docs.puppet.com/puppet/4.9/lang_conditional.html#if-statements

Change the version drop-down box to match your Puppet Server version?

Also, are you aware of the builtin Puppet facts that you can use?  You mentioned that you are using $env variable "from?" hostname, there is a facter (fact) for hostname; are you aware of that?

I too am relatively newbie, but I am getting better with every new and challenging module I write.

Finally, what error are you getting when you use this module?  Copy/paste the output as the result of puppet parser validate /path/to/your/module/manifest/init.pp  (or whatever your .PP file is called).


--------------------------
Warron French


On Mon, May 1, 2017 at 7:36 PM, Samir Gahirwal <samir.g...@gmail.com> wrote:
Hi,

I am very new to puppet and writing my first class :) Please pardon me for asking basic question

I am trying to write module to run install script. Can someone help me with syntax or suggest better way to code following

define install-package {
if
( $env != 'test' or $env != 'qa') {
           
case $::package_type {
               
'abc': {
                    $variable
= '123'
                     
}
               
'xyz': {
                    $
variable = '345'
                     
}
   exec {"run_my_script":
          source => "puppet:///modules/mymodule/resources/my
installscript.sh",
          command => 'sh
myscript.sh $variable'
        }
 
             }
else {

            case $::package_type {
               
'abc': {
                    $variable
= '567'
                     
}
               
'xyz': {
                    $
variable = '789'
                     
}
             }

   exec {"run_my_script":
          source => "puppet:///modules/mymodule/resources/my
installscript.sh",
          command => 'sh
myscript.sh $variable'
        }

}

class mymodule::software {
    $package_type = unique([jdk,apache,tomcat])
    install-package { $
package_type:; }
}

I am deriving env variable from host name.

Let me know if you need additional details.

Thanks!
--SamirG

--
You received this message because you are subscribed to the Google Groups "Puppet Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/ecb68180-af56-4d36-a555-236a69efea50%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Rob Nelson

unread,
May 1, 2017, 11:02:05 PM5/1/17
to puppet...@googlegroups.com
What you've coded seems workable, aside from two issues. 1) the errant semicolon at the end (while that may work to 'end' the resource defaults, its bad style when you don't have defaults). 2) exec resource titles need to be unique if the defined type is used twice; adding the $name value of the defined type should provide uniqueness. This will not show up in your example where the type is only used once, but best to plan for long haul.

However, we don't know what you are trying to accomplish. Your first conditional should probably be an and, because otherwise the conditional will always be true ($env will always not be one of those values). The case selectors have no default, do you need one? Should you be using a global variable vs passing the value to a type parameter? Probably the latter. If you explain your goal, then we can offer specific suggestions on how to make the code accomplish it.

To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
--
Rob Nelson

Samir Gahirwal

unread,
May 2, 2017, 2:20:38 PM5/2/17
to Puppet Users
Hi Warron,

Thanks for your response. Yes, i looked up puppet manual pages and found syntax. But what I can't find is how to put command execution syntax inside loop.

Let me hop on to slack.puppet.com to get help as well.

Thanks!

--------------------------
Warron French


To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.

Samir Gahirwal

unread,
May 2, 2017, 2:32:22 PM5/2/17
to Puppet Users
Let me know if you need more details.


Thanks again!!


Hi Rob,

Thanks for your response. Please find answers inline

Your first conditional should probably be an and, because otherwise the conditional will always be true ($env will always not be one of those
values
).

Purpose of this module is to generate java keystores. I want to use different password for our test end and prod env for those certificates. The env value I am deriving from "$env = vd_get($::__vd, 'env')" and o/p could be test/qa/prod/colo. So test/qa should be true for first case. Else use other passwords.

The case selectors have no default, do you need one?
Thats great, actually I was looking for way to avoid using default.

Here is exact code snippet.

define certificate-generator {
        user
=> root,
       
group => root,

       
       
if ( $env != 'test' or $env != 'qa') {

           
case $::target_cert_type {
               
'type1': {
                    $password
= '123'
                     
}
               
'type2': {
                    $password
= '345'
                     
}
               
default: {
                    fail
("No password defined")
                     
}
           
}
         
exec { "certificate-generator":
              command
=> 'sh certgen.sh',
              path    
=> '/bin/bash',
              logoutput
=> true,
              onlyif
=>  "test ! -f ${cert_basedir}/${host}.jks"
              source
=> "puppet:///modules/mymodule/certgen/certgen.sh $password"
             
}
         
}
         
else {
             
case $::target_cert_type {
               
'type1': {
                    $password
= '567'
                     
}
               
'type2': {
                    $password
= '789'
                     
}
               
default: {
                    fail
("No password defined ")
                     
}    
         
exec { "certificate-generator":
              command
=> 'sh certgen.sh',
              path    
=> '/bin/bash',
              logoutput
=> true,
              onlyif
=>  "test ! -f ${cert_basedir}/${host}.jks"
              source
=> "puppet:///modules/mymodule/certgen/certgen.sh $password"
             
}    
         
}

class javakeystore::certgen{
    $env
= vd_get($::__vd, 'env')
    $cert_type
= vd_get($::__vd, 'certtype')
    $target_cert_type
= unique([$cert_type])
    certificate
-generator { $target_cert_type:; }
}




Rob Nelson

unread,
May 3, 2017, 9:21:52 AM5/3/17
to Puppet Users
I think some characters got gobbled during pasting or by Google Groups; it looks like the parameter block to your type is missing the start/finish. But that's syntax, I'm sure you've got it right or can fix any typos yourself.

The suggestion I have is about a common anti-pattern: Don't Repeat Yourself (DRY). You have two exec resource declarations, only of which is instantiated, with the same parameter list for both. The only difference is interpolation of the password, which is set inside the control flow. Moving the exec instantiation after the conditional blocks helps with maintenance in the long term, but also makes it easier for someone to read your code later (including future you!) and see that the end result is a single resource. That would look something like this:

define certificate-generator (

  user => root,
  group => root,
) {

  if ( $env != 'test' or $env != 'qa') {
    case $::target_cert_type {
      'type1': {
        $password = '123'
      }
      'type2': {
        $password = '345'
      }
      default: {
        fail("No password defined")
      }
    }
  }
  else {
    case $::target_cert_type {
      'type1': {
        $password = '567'
      }
      'type2': {
        $password = '789'
      }
      default: {
        fail("No password defined ")
      }   
    }
  }

  # Single exec resource here with variable inputs determined above
 
exec { "certificate-generator":

    command => 'sh certgen.sh',
    path    => '/bin/bash',
    logoutput => true,
    onlyif =>  "test ! -f ${cert_basedir}/${host}.jks"
    source => "puppet:///modules/mymodule/certgen/certgen.sh $password"
  }    
}

I still have a concern about your if statements, though. If `env` is `prod`, it obviously matches for if ( $env != 'test' or $env != 'qa'). But if `env` is `qa`, it STILL matches because $env != 'test'
is true and it never reaches the $env != 'qa' comparison. And if it's `test`, it still matches because after failing the first test, it passes the second test. The `else` condition is never reached that I can see. Changing that to an AND match, or using equality and swapping the if/else blocks, appears closer to the described desired state.

Samir Gahirwal

unread,
May 3, 2017, 12:28:45 PM5/3/17
to Puppet Users
Thanks for your help Rob!

Yes even, I have second doubts about if statement, I will find some other way to fix this conditional statement.

Henrik Lindberg

unread,
May 3, 2017, 5:32:18 PM5/3/17
to puppet...@googlegroups.com
On 03/05/17 18:28, Samir Gahirwal wrote:
> Thanks for your help Rob!
>
> Yes even, I have second doubts about if statement, I will find some
> other way to fix this conditional statement.
>

You probably want:

if !($env == 'test' or $env == 'qa) {
#
}

i.e. "neither of", which you can also write as

unless $env == 'test' or $env == 'qa' {
}

Or do the reverse and move the logic you have in the "else" part to the
if part and drop the "not".

i.e.

if $env == 'test' or $env == 'qa {
# what you have in else body
} else {
# what you now have in if body
}

That is much easier to read.

>
> On Wednesday, May 3, 2017 at 6:21:52 AM UTC-7, Rob Nelson wrote:
>
> I think some characters got gobbled during pasting or by Google
> Groups; it looks like the parameter block to your type is missing
> the start/finish. But that's syntax, I'm sure you've got it right or
> can fix any typos yourself.
>
> The suggestion I have is about a common anti-pattern: Don't Repeat
> Yourself (DRY). You have two exec resource declarations, only of
> which is instantiated, with the same parameter list for both. The
> only difference is interpolation of the password, which is set
> inside the control flow. Moving the exec instantiation *after* the
> conditional blocks helps with maintenance in the long term, but also
> makes it easier for someone to read your code later (including
> future you!) and see that the end result is a single resource. That
> would look something like this:
>
> ||
> |
> |
> define certificate-generator (

You cannot use hyphens ('-') in names. Change to use underscore for
example "certificate_generator".

Best,
- henrik

> user =>root,
> group=>root,
> ) {
> if($env !='test'or$env !='qa'){
> case$::target_cert_type {
> 'type1':{
> $password ='123'
> }
> 'type2':{
> $password ='345'
> }
> default:{
> fail("No password defined")
> }
> }
> }
> else{
> case$::target_cert_type {
> 'type1':{
> $password ='567'
> }
> 'type2':{
> $password ='789'
> }
> default:{
> fail("No password defined ")
> }
> }
> || }
>
> # Single exec resource here with variable inputs determined above
> || exec{"certificate-generator":
> command =>'sh certgen.sh',
> path =>'/bin/bash',
> logoutput =>true,
> onlyif => "test ! -f ${cert_basedir}/${host}.jks"
> source =>"puppet:///modules/mymodule/certgen/certgen.sh $password"
> }
> }
> |
> |
>
> I still have a concern about your if statements, though. If `env` is
> `prod`, it obviously matches for ||if($env !='test'or$env !='qa')||.
> But if `env` is `qa`, it STILL matches because ||$env !='test'
> || is true and it never reaches the ||$env !='qa'|| comparison. And
> if it's `test`, it still matches because after failing the first
> test, it passes the second test. The `else` condition is never
> reached that I can see. Changing that to an AND match, or using
> equality and swapping the if/else blocks, appears closer to the
> described desired state.
>

--

Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/

Reply all
Reply to author
Forward
0 new messages