Auto_update not idempotent

[jeff1evesque]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 4.10.x
• Ruby: 2.1.9 (via facter rubyversion)
• Distribution: centos7x
• Module version: 5.1.0 (https://forge.puppet.com/puppet/php)

How to reproduce (e.g Puppet code you use)

    class { '::php':
        settings   => {
            'PHP/max_execution_time'  => $max_execution,
            'PHP/max_input_time'      => $max_input,
            'PHP/memory_limit'        => $memory_limit,
            'PHP/post_max_size'       => $post_max,
            'PHP/upload_max_filesize' => $upload_max,
            'Date/date.timezone'      => $timezone,
        },
        manage_repos => true,
        proxy_type   => 'https',
        proxy_server => $my_proxy,
        composer     => true,
        extensions   => {
            pdo       => { },
            pdo_mysql => { },
        }
    }

What are you seeing

The module is not idempotent, and I keep seeing:

Notice: /Stage[main]/Php::Composer::Auto_update/Exec[update composer]/returns: executed successfully

What behaviour did you expect instead

I expected the module to be idempotent, with respect to composer.

Any additional information you'd like to impart

If possible, I suggest making the following logic from composer.pp idempotent, by maybe creating a fact, to check if there is an update available, then run auto_update (otherwise, don't run):

  if $auto_update {
    class { '::php::composer::auto_update':
      max_age      => $max_age,
      source       => $source,
      path         => $path,
      proxy_type   => $proxy_type,
      proxy_server => $proxy_server,
    }
}
}

If my suggestion seems reasonable, I could potentially create the puppet fact, and integrate it, to the above class? For the time being, I'm going to try to set $auto_update to false.

[jeff1evesque]

Doesn't seem auto_update property is defined within init.pp. So, I don't think I can set it to false? Would anyone object, if I submitted a PR to allow auto_update to occur conditionally. I could create a fact that determines the current installed version of composer, via composer --version. Then, I could determine the current version, by querying from https://getcomposer.org/download. If the two aren't equal (composer --version < the url version), then perform the above?

[jeff1evesque]

I found max_age, which will make it seem idempotent. But, it's kind of arbitrary. Attempt an update after max_days = xx, regardless if an update exists or not.

[jeff1evesque]

Also, maybe it's appropriate to have auto_update: false, while still installing composer. I can make this change, as well, if preferred.

[joekohlsdorf]

The logic is that the update command is run after X days with the default for X being 30. The amount of days passed since the last update is determined by the age of the /usr/local/bin/composer file.
I cannot reproduce the issue but we have the following unhandled edge case: If we pass max_age but no update is available, an update will be attempted on every Puppet run because the binary doesn't change.

An easy and effective fix would be to touch /usr/local/bin/composer to update its modification time regardless of an update being available or not when the update procedure is run.