Opened 3 years ago

Last modified 3 years ago

#3574 closed change

Refactor Puppet class base into class adblockplus — at Version 19

Reported by: matze Assignee: matze
Priority: P3 Milestone:
Module: Infrastructure Keywords:
Cc: Blocked By: #3818
Blocking: #3576, #3587, #3638 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29334668
https://codereview.adblockplus.org/29334690
https://codereview.adblockplus.org/29335885
https://codereview.adblockplus.org/29337881
https://codereview.adblockplus.org/29339094
https://codereview.adblockplus.org/29337931
https://codereview.adblockplus.org/29338036
https://codereview.adblockplus.org/29338473
https://codereview.adblockplus.org/29340814
https://codereview.adblockplus.org/29366707
https://codereview.adblockplus.org/29366882
https://codereview.adblockplus.org/29366987

Description

Class base is always included by our node classifier, never by any other Puppet class. Thus the movement itself should be fairly trivial, though we may consider additional improvements now, i.e.:

  • Remove the possibly obsolete conditional around the definition of class apt
  • Avoid all explicit host records being realized due to Puppet issue 4145

Desired future improvements include:

  • Avoid APT update being executed on every single Puppet run
  • Remove private::users in favour of $adblockplus::users
  • Improve the node classifier, especially handling of Hiera/$servers parameters
  • Migrate base::explicit_host_record() into class adblockplus::host

This ticket is part of our long-term adventure of improving the infrastructure code-base.

Change History (19)

comment:1 Changed 3 years ago by matze

  • Blocking 3576 added
  • Cc fred removed
  • Ready set

comment:2 Changed 3 years ago by matze

  • Blocking 3587 added

comment:3 Changed 3 years ago by matze

  • Owner set to matze

comment:4 Changed 3 years ago by matze

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

In order to remove blocking status for #3587, the first step is a simple inclusion of class base within class adblockplus (and updating base.yaml accordingly).

comment:5 Changed 3 years ago by matze

  • Review URL(s) modified (diff)

comment:7 Changed 3 years ago by matze

  • Blocked By 3638 added

comment:8 Changed 3 years ago by matze

  • Blocked By 3638 removed
  • Blocking 3638 added
  • Review URL(s) modified (diff)

comment:10 Changed 3 years ago by matze

  • Review URL(s) modified (diff)

comment:11 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/infrastructure/rev/c87aa23fb276

comment:12 Changed 3 years ago by matze

  • Review URL(s) modified (diff)

comment:13 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/infrastructure/rev/398e6fe0cd7d

comment:14 Changed 3 years ago by matze

  • Review URL(s) modified (diff)

comment:15 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/infrastructure/rev/8a8f0de52ea1

comment:16 Changed 3 years ago by matze

  • Blocked By 3818 added

comment:17 Changed 3 years ago by matze

  • Review URL(s) modified (diff)

comment:18 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/infrastructure/rev/eeb831ca03b6

comment:19 Changed 3 years ago by matze

  • Review URL(s) modified (diff)

The list of packages hard-coded within Class['base'] is about to become replaced by parameter $adblockplus::packages, combining functions hiera_array() and ensure_packages() to provide a more flexible approach.

Note: See TracTickets for help on using tickets.