Bug 1498457

Not sure how upgrade process works exactly, but during regular deployments the enterprise parts of the puppet module are disabled as can be seen in [1]. So my guess is that the upgrade process should contain the same hardcoded hiera values as can be seen in [1]. Will consult upgrade specialist and get back.

[1] https://github.com/openstack/tripleo-heat-templates/blob/stable/newton/puppet/services/monitoring/sensu-base.yaml#L66,L69

FYI, During updates we override the package definitions globally to switch them all to ensure => latest.

https://github.com/openstack/puppet-tripleo/blob/master/manifests/packages.pp#L49-L57

The issue here is that the latest version of the packages is missing a dependency in a repo. So this seems to be a missing package issue for sensu.

Ok, so during upgrade process the same hieradata is being used. Given the conditional in [2] there is no way the package installation could be forced with correct hieradata even with "Package <| |> { ensure => 'latest' }". Anyway, ::sensu::enterprise_version is 'latest' by default, so the override does not have any effect on that piece of code. Can you please write here a reproducer? Either hieradata was overriden incorrectly or the problem is somewhere else.


[2] https://github.com/sensu/sensu-puppet/blob/7f60b2c5f708ed03b7769b69a1e86018db598aa9/manifests/enterprise/package.pp#L11
[3] https://github.com/sensu/sensu-puppet/blob/7f60b2c5f708ed03b7769b69a1e86018db598aa9/manifests/init.pp#L341

And it would also be helpful to attach sosreport from the node where the puppet deployment fails.

(In reply to Alex Schultz from comment #3)
> FYI, During updates we override the package definitions globally to switch
> them all to ensure => latest.
> 
> https://github.com/openstack/puppet-tripleo/blob/master/manifests/packages.
> pp#L49-L57
> 
> The issue here is that the latest version of the packages is missing a
> dependency in a repo. So this seems to be a missing package issue for sensu.

No.

The issue is that because your global switch also overrides all occurrences of "ensure => absent" with "ensure => latest". Which will then try to newly install a package which shouldn't be installed.

So using "ensure => latest" on all occurrences of "package" globally is actually a bad idea and the bug here that needs fixing.

(In reply to Martin Magr from comment #4)
> Ok, so during upgrade process the same hieradata is being used. Given the
> conditional in [2] there is no way the package installation could be forced
> with correct hieradata even with "Package <| |> { ensure => 'latest' }".
> Anyway, ::sensu::enterprise_version is 'latest' by default, so the override
> does not have any effect on that piece of code. Can you please write here a
> reproducer? Either hieradata was overriden incorrectly or the problem is
> somewhere else.
> 
> 
> [2]
> https://github.com/sensu/sensu-puppet/blob/
> 7f60b2c5f708ed03b7769b69a1e86018db598aa9/manifests/enterprise/package.pp#L11
> [3]
> https://github.com/sensu/sensu-puppet/blob/
> 7f60b2c5f708ed03b7769b69a1e86018db598aa9/manifests/init.pp#L341

Afaik reproducing is as simple as:

1. Install OSP10 with Sensu agent enabled (which will then pull in the Sensu puppet modules)
2. Run an "overcloud update" (which will then fail as it'll try to install Sensu Enterprise packages which we don't ship).

David, "ensure => latest" is the default for puppet resource installing Sensu enterprise package, so this should not a problem. So you did not modify the hieradata in anyway?

Please provide detailed reproducer with all environment files used. The one provided in comment 7 is not enough. The more data you will provide the faster we will be able to help you.

(In reply to Martin Magr from comment #10)
> David, "ensure => latest" is the default for puppet resource installing
> Sensu enterprise package, so this should not a problem. So you did not
> modify the hieradata in anyway?

Martin, it doesn't matter if I change the hieradata or not. The issue is happening purely in the puppet code and the update code.

But let's go step by step:

1. it's the puppet-enterprise-dashboard package which is at fault, not the puppet-enterprise package. This is clearly outlined in the original description of the BZ, both from the code examples as well as from the error message.

2. We do not ship sensu-enterprise-dashboard (or sensu-enterprise for that matter). The sensu-enterprise-dashboard package is only available via Sensu Enterprise which you would need to purchase from Sensu, Inc.
Based on this it logically follows that it is not set to "ensure => latest" by default, because that cannot work! It would always fail because it would try to install a package which you would need to get from Sensu, Inc. and is not part of our product.

But let's actually look at the code:

Here's /usr/share/openstack-puppet/modules/sensu/manifests/enterprise/dashboard/package.pp as available in OSP10 (puppet-sensu-2.1.0-4.7f60b2cgit.el7ost.noarch on my reproducer env):

# = Class: sensu::enterprise::dashboard::package
#
# Manages the sensu-enterprise-dashboard package
#
class sensu::enterprise::dashboard::package {

  if $caller_module_name != $module_name {
    fail("Use of private class ${name} by ${caller_module_name}")
  }

  if $::sensu::enterprise_dashboard {
    $ensure = $::sensu::enterprise_dashboard_version
  } else {
    $ensure = 'absent'
  }

  package { 'sensu-enterprise-dashboard':
    ensure => $ensure,
  }

}

So reading this you can see that even if "$::sensu::enterprise_dashboard_version" is set to latest, this doesn't matter as the if/else clause will evaluate $ensure = 'absent' because $::sensu::enterprise_dashboard is false. Let's verify this:

[stack@undercloud manifests]$ grep enterprise_dashboard /usr/share/openstack-puppet/modules/sensu/manifests/init.pp
# [*enterprise_dashboard*]
  $enterprise_dashboard           = false,
  $enterprise_dashboard_version   = 'latest',
[...]

So the above is correct and we end up with ensure = 'absent' which would then further down ensure the package is absent (as it has to be).

And this again will get overridden by the "Package <| |> { ensure => 'latest' }" which is what is failing here.


And just for completeness sake, puppet-enterprise is not giving any issues because there the if/else is set differently to actually encompass the "package" resource:

cat /usr/share/openstack-puppet/modules/sensu/manifests/enterprise/package.pp
# = Class: sensu::package
#
# Installs the Sensu packages
#
class sensu::enterprise::package {

  if $caller_module_name != $module_name {
    fail("Use of private class ${name} by ${caller_module_name}")
  }

  if $::sensu::enterprise {

    package { 'sensu-enterprise':
      ensure  => $sensu::enterprise_version,
    }
[...]

So we can see that as long as "$::sensu::enterprise" is false, it doesn't actually matter what the "ensure" is set to.

And that is what is happening:

[stack@undercloud manifests]$ grep enterprise /usr/share/openstack-puppet/modules/sensu/manifests/init.pp
# [*enterprise*]
# [*enterprise_dashboard*]
#   Boolean.  Whether or not to install sensu-enterprise-dashboard
  $enterprise                     = false,
  $enterprise_version             = 'latest',
[...]

So in this case it doesn't matter that "$enterprise_version = 'latest'", because this package resource never gets evaluated. And consequently the update code doesn't lead to an issue here either.


In summary:
1. puppet-enterprise-dashboard is not shipped in our product
2. the corresponding package resource is by default set to "ensure => absent"
3. the "Package <| |> { ensure => 'latest' }" which is part of the update code overrides this
4. this fails because we cannot install a package which we don't ship
5. a "fix" would be to change the if/else clause in /usr/share/openstack-puppet/modules/sensu/manifests/enterprise/dashboard/package.pp to be more like the one in /usr/share/openstack-puppet/modules/sensu/manifests/enterprise/package.pp
6. this is still a hack, I disagree with globally setting "Package <| |> { ensure => 'latest' }" as this pulls in other packages which are not installed by default.

You're right David, the problem is in the puppet-sensu code. Sadly I was checking OSP11 version of the puppet module originally, where the code is updated and makes more sense then the code in current OSP10 version: 

class sensu::enterprise::dashboard::package {

  <snip>

  if $::sensu::enterprise_dashboard {
    package { 'sensu-enterprise-dashboard':
      ensure => $::sensu::enterprise_dashboard_version,
    }
  }
}

I will patch the OSP10 module. Sorry for the noise.

puppet-sensu-2.1.0-5.7f60b2cgit.el7ost

updated from osp10z4 to osp10z8

update finished with status COMPLETE.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:0365