Project

General

Profile

Bug #15153

Feature #15154: Update Puppet modules: 2017Q2 → 2018Q1 edition

Upgrade icinga2 Puppet module

Added by intrigeri over 1 year ago. Updated over 1 year ago.

Status:
Rejected
Priority:
Normal
Assignee:
Category:
Infrastructure
Target version:
Start date:
01/08/2018
Due date:
% Done:

100%

Feature Branch:
Type of work:
Sysadmin
Blueprint:
Starter:
Affected tool:

Description

I've just done a complete pass of my "Update Puppet modules" job but I had to skip this one: I could not find an upstream that has the commit we're currently running, and git tag --contains=4838b4e3e2b5f778d593695b94d755ffb6c9f362 returns nothing.

Could you please take a look and tell me what upstream I should upgrade to?


Related issues

Blocks Tails - Feature #13284: Core work: Sysadmin (Adapt our infrastructure) Confirmed 06/30/2017

Associated revisions

Revision bf9349d3 (diff)
Added by intrigeri 8 months ago

Temporarily workaround udisks2/libblockdev bug (refs: #15153)

We can revert this once our Vagrant build box runs Buster.

History

#1 Updated by intrigeri over 1 year ago

  • Blocks Feature #13284: Core work: Sysadmin (Adapt our infrastructure) added

#2 Updated by intrigeri over 1 year ago

  • Parent task set to #15154

#3 Updated by intrigeri over 1 year ago

If it may help, the only commit we have about this module is:

commit 61ead914875a394208cde949f73c7274a04a94e6
Author: bertagaz <bertagaz@ptitcanardnoir.org>
Date:   Fri Mar 18 13:36:58 2016 +0100

    Add the puppet-icinga2 module.

    We'll use parts of it, in the hope to completely switch to it once it
    will be stable.

    Refs: #8650

#4 Updated by intrigeri over 1 year ago

Hi bertagaz! As said elsewhere I want to complete this by the end of 2018Q1. Three weeks have passed since I've asked the question above and only 2 months are left for me to do the work. So I would greatly appreciate a prompt reply so I can do my work in a non-rushed, less stressful manner. Thanks in advance!

#5 Updated by bertagaz over 1 year ago

  • Assignee changed from bertagaz to intrigeri
  • QA Check changed from Info Needed to Dev Needed

intrigeri wrote:

Hi bertagaz! As said elsewhere I want to complete this by the end of 2018Q1. Three weeks have passed since I've asked the question above and only 2 months are left for me to do the work. So I would greatly appreciate a prompt reply so I can do my work in a non-rushed, less stressful manner. Thanks in advance!

Right, sorry forgot this.

Could you please take a look and tell me what upstream I should upgrade to?

I'v used https://github.com/Icinga/puppet-icinga2.git, from the develop branch.

But as said once when we discussed about this upstream module, its development is quite chaotic, so it may be difficult to update it. It also depends on full featured exported resources (to support hash parameters), so we're only using a tiny part of it. SO depending on the energy you want to put in there, we may also just stick to the version we have now.

#6 Updated by intrigeri over 1 year ago

  • Assignee changed from intrigeri to bertagaz
  • QA Check changed from Dev Needed to Info Needed

Could you please take a look and tell me what upstream I should upgrade to?

I'v used https://github.com/Icinga/puppet-icinga2.git, from the develop branch.

That's what I thought but it's good to have an authoritative confirmation, thanks!

Any idea what version we're running? As said initially on this ticket:

I could not find an upstream that has the commit we're currently running, and git tag --contains=4838b4e3e2b5f778d593695b94d755ffb6c9f362 returns nothing.

I'm asking because to plan the upgrade I need to inspect the changes made upstream since we cloned their repo, and I cannot do that with the info I have in hand currently. Or is there no useful versioning whatsoever on that develop branch?

As we can see on this ticket, it seems that their develop branch makes upgrades a PITA and is thus not a good fit for a production deployment such as ours. So I have to ask: is there any reason why we should stay on their develop branch, i.e. why we cannot switch to regular stable releases with versioning and no "interesting" Git history rewriting? (I'm sure there was a good reason in March 2016, but hopefully things have changed in the meantime.)

SO depending on the energy you want to put in there, we may also just stick to the version we have now.

OK, thanks for clarifying. While I could certainly skip that module and look elsewhere, I'm concerned we're running 2 years old code with no clear plan to upgrade it in the future. For example, I've no idea if/how we would notice in case a serious security issue was found in that module. To me, that does not sound like something we should be doing in production. So time permitting (within the budget I have for #15154) I'd like to do what's needed to make this situation manageable :)

#7 Updated by bertagaz over 1 year ago

  • Assignee changed from bertagaz to intrigeri
  • QA Check changed from Info Needed to Dev Needed

intrigeri wrote:

Any idea what version we're running? As said initially on this ticket:

I could not find an upstream that has the commit we're currently running, and git tag --contains=4838b4e3e2b5f778d593695b94d755ffb6c9f362 returns nothing.

I'm asking because to plan the upgrade I need to inspect the changes made upstream since we cloned their repo, and I cannot do that with the info I have in hand currently. Or is there no useful versioning whatsoever on that develop branch?

Doesn't seem so. The tags seem to be based on their master's branch commits. I've picked the tip of their develop branch at that time, because the master branch was way older and was not compatible with recent Icinga2 versions (the one in Debian and we were using). IIRC that's because at some point in Icinga2 development they decided to completely rewrite the code they had inherited from Icinga, and it worked/was configured very differently.

As we can see on this ticket, it seems that their develop branch makes upgrades a PITA and is thus not a good fit for a production deployment such as ours. So I have to ask: is there any reason why we should stay on their develop branch, i.e. why we cannot switch to regular stable releases with versioning and no "interesting" Git history rewriting? (I'm sure there was a good reason in March 2016, but hopefully things have changed in the meantime.)

Maybe nowadays their master branch do support recent Icinga2 and is in a better shape.

SO depending on the energy you want to put in there, we may also just stick to the version we have now.

OK, thanks for clarifying. While I could certainly skip that module and look elsewhere, I'm concerned we're running 2 years old code with no clear plan to upgrade it in the future. For example, I've no idea if/how we would notice in case a serious security issue was found in that module. To me, that does not sound like something we should be doing in production. So time permitting (within the budget I have for #15154) I'd like to do what's needed to make this situation manageable :)

Ack. Note that we're using a very tiny part of this upstream module code, so if that's too complecated to upgrade, another path could be to move that part of their code in our own and then drop this uptream module maybe.

#8 Updated by intrigeri over 1 year ago

  • Status changed from Confirmed to Rejected
  • % Done changed from 0 to 100
  • QA Check deleted (Dev Needed)

It looks like we're using exactly one resource from that module i.e. ::icinga2::object::service. In the version we're running, that defined resource is self-contained except it uses the icinga2/object/service.conf.erb template, that's itself self-contained as long as one does not pass a non-empty hash to the vars parameter (we don't). But in the current upstream version (v1.3.5) that defined resource has include ::icinga2::params (used for $::icinga2::params::conf_dir) and delegates to icinga2::object the actual config file deployment (which is now done with concat). The interface does not change much so in theory it should be doable to upgrade: I think the only required change is in the — now obsolete — target_* parameters we pass to ::icinga2::object::service.

OTOH, seriously: it does not make sense to me to rely on a complex external module merely to use one single defined resource that deploy a file from a template. I mean, the only piece of code we actually use in there is:

    file { "${target_dir}/${target_file_name}":
      ensure  => $target_file_ensure,
      owner   => $target_file_owner,
      group   => $target_file_group,
      mode    => $target_file_mode,
      content => template('icinga2/object/service.conf.erb'),
    }

… which we could do ourselves just as well. The icinga2/object/service.conf.erb template is entirely trivial.

Reusing that piece of code from the upstream icinga2 module made some sense back when we had a plan (#13285) to start using that module more extensively to replace our own code (that reimplements much of its functionality). That plan was added to our 2017Q2→2018Q1 roadmap in 2016, no progress was made and the we removed it from our 2018Q2→2019Q1 roadmap due to unclear cost/benefit.

So I won't bother upgrading this module: it's pretty unlikely that there's ever a security issue in the trivial piece of code we use from it, and I see no point in upgrading that trivial piece of code to a more complex one with exactly zero benefits.

I'm even tempted to drop that module entirely and to copy the bits we use into puppet-tails, so that nobody has to ever do this analysis again. If you agree, let me know and I'll create a dedicated ticket about it.

Also available in: Atom PDF