Opened 5 years ago

Closed 4 years ago

#2314 closed defect (duplicate)

Replace Exec["fetch_sitescripts"] with Class["sitescripts"]

Reported by: matze Assignee: matze
Priority: P4 Milestone:
Module: Infrastructure Keywords:
Cc: fhd, trev, fred Blocked By: #2277, #3686
Blocking: #2313 Platform: Unknown
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Outside of module sitescripts, the exec {'fetch_sitescripts: ... directive should be considered internal to that module. So, instead of referencing Exec['fetch_sitescripts'] in require => [...] parameters and similar occasions, one should reference Class['sitescripts'] instead.

This is the only way to ensure compatibility with future changes inside that class.

Also, please note that module sitescripts is probably not the only one where we could improve in this fashion. But it's a start, and this ticket is a clean reference for the commit(s).

Change History (8)

comment:1 Changed 5 years ago by matze

  • Blocked By 2277 added

(This ticket should also be done immediately after #2277 is pushed, to allow for a straightforward commit history and clean patch-sets.)

comment:2 Changed 5 years ago by matze

  • Cc fhd trev fred added

comment:3 Changed 5 years ago by fhd

If require Class['sitescripts'] implicitly requires all resources in the class, including the exec, that's fine of course.

Tried to quickly double check that but it doesn't seem to work the way I thought it should:

In this example, Exec['/usr/local/bin/update_custom_timestamps.sh'] would happen after every resource in the ntp class, including the package, the file, and the service.

From: https://docs.puppetlabs.com/puppet/latest/reference/lang_containment.html

Doesn't seem like we would require all resources in the sitescripts class this way, but rather any - which wouldn't do the trick.

Last edited 5 years ago by fhd (previous) (diff)

comment:4 Changed 5 years ago by matze

I'm a bit confused now.. :)

"Doesn't seem like we would require all resources in the sitescripts class this way, but rather any - which wouldn't do the trick."

How do you come to that conclusion? The example you refer to demonstrates otherwise, and also states that referring to the class "... allows you to notify and subscribe to classes and defined resource types as though they were a single resource".

comment:5 Changed 5 years ago by fhd

OK I think I've actually misread the "after every resource" part. Phew :)

comment:6 Changed 5 years ago by matze

Very well ;)

Actually we've even done so in other places before. It worked like a charm. Thus I can confirm that the documentation is quite precise here.

By the way: I often compare puppet resources to regular programming concepts. Parameters and named types (defines) are part of the public API of a Puppet class, whilst resources inside the class should be considered private. Though other classes in the *same* module *may* be considered friends...

comment:7 Changed 4 years ago by matze

  • Blocked By 3686 added
  • Owner set to matze
  • Tester set to Unknown

comment:8 Changed 4 years ago by matze

  • Ready set
  • Resolution set to duplicate
  • Status changed from new to closed

After this ticket here has been forgotten for a while, we actually implemented the necessary patch-sets in the context of a newly created one with the exact same purpose, (but less useful explanations maybe). See #3686 for more information.

Note: See TracTickets for help on using tickets.