Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PUP-9323) Allow deferred functions to adhere to puppet relationships and ordering #8902

Merged
merged 4 commits into from
May 18, 2022

Conversation

joshcooper
Copy link
Contributor

@joshcooper joshcooper commented Apr 16, 2022

By default, deferred functions are "preprocessed" before the catalog is applied (before prefetching, expanding the relationship graph, etc). However, in this mode puppet cannot install a prerequisite needed for the deferred function and call the function. Worse, puppet never eventually converges, because the deferred function failure prevents all resources from being managed.

This PR makes it possible to disable preprocessing so that deferred functions work in the way users expect. In other words, if a resource's parameter or property is a deferred value, then the deferred function will be called when it's resource is managed. Therefore, the function will adhere to normal puppet relationships and ordering. And if the deferred function fails, then only that resource will fail, and downstream resources that depend on it will be skipped.

In order to accomplish this, we have to delay the calls to munge, validate and unmunge. See 983f652 and 815c9c7 for details.

@joshcooper joshcooper force-pushed the deferred branch 2 times, most recently from 1e5b430 to 9beaebe Compare April 21, 2022 23:15
@joshcooper
Copy link
Contributor Author

$file = '/path/to/new/file' 
file { $file:
  ensure => file,
  content => "1234",
} ->
notify { 'deferred':
  message => Deferred('binary_file', [$file])
}

Using puppet#main it fails because binary_file is called before the file exists:

$ bx puppet apply deferred.pp
Notice: Compiled catalog for localhost in environment production in 0.02 seconds
Error: binary_file(): The given file '/tmp/newfile' does not exist

With this PR:

$ bx puppet apply deferred.pp
Notice: Compiled catalog for localhost in environment production in 0.02 seconds
Notice: /Stage[main]/Main/File[/tmp/newfile]/ensure: defined content as '{sha256}03ac674216f3e15c761ee1a5e255f067953623c8b388b4459e13f978d7c846f4'
Notice: MTIzNA==
Notice: /Stage[main]/Main/Notify[deferred]/message: defined 'message' as Binary("MTIzNA==")
Notice: Applied catalog in 0.02 seconds

@joshcooper
Copy link
Contributor Author

jenkins please test this with servertests

When the agent converts the resource catalog (downloaded from the server) to a
RAL catalog, it metaprograms a new class for each type of resource, e.g.
`Puppet::Type::File`, and corresponding classes for each parameter/property,
e.g. `Puppet::Type::File::Mode`. The conversion to RAL also triggers calls to
`munge`, `validate` and `unmunge` for each parameter. This gives the parameter a
chance to convert from the DSL form to an internal/canonical form, e.g. "owner
=> root" and "owner => 0" are equivalent.

However, if the parameter's value is deferred, then we won't know its value
until later when the resource is evaluated. So we need to delay calls to
`munge`, `validate` and `unmunge` until after the deferred value is known.

It is possible to intercept calls to `munge` and `validate`, because there is a
level of indirection between the transaction and the munge/validate methods
implemented in a custom parameter. For example, the transaction sets the desired
value on the property[1], which triggers a call to the `munge` instance method.
If the custom type has defined a `munge { |value| ...}` DSL method, then it
metaprograms an `munge_unsafe` method[2], which is called by the `munge` instance
method[3].

However, the `unmunge` logic doesn't have this level of indirection -- the
transaction calls the custom `unmunge` method directly.

This commit introduces a level of indirection to `unmunge` following the same
pattern used by `munge`.

[1] https://github.com/puppetlabs/puppet/blob/7.16.0/lib/puppet/property.rb#L551
[2] https://github.com/puppetlabs/puppet/blob/7.16.0/lib/puppet/parameter.rb#L176
[3] https://github.com/puppetlabs/puppet/blob/7.16.0/lib/puppet/parameter.rb#L430
Allow deferred values to be resolved lazily as each resource is evaluated.
This also means validation, munging and unmunging are delayed until the
resolved value is set back on the parameter or property.

The deferred evaluation is similar to how Puppet::Type#eval_generate works, but
since we're not modifying the graph, we don't need to worry about containment
edges, tags, noop, etc.
If `Puppet[:preprocess_deferrred]` is true (the default), then the `binary_file`
deferred function will fail to read the file, since it hasn't been created yet.

If the value is false, then the file is created before `binary_file` is called.
@joshcooper joshcooper changed the title Deferred (PUP-9323) Allow deferred functions to adhere to puppet relationships and ordering May 16, 2022
@joshcooper joshcooper marked this pull request as ready for review May 16, 2022 22:14
@joshcooper joshcooper requested a review from a team as a code owner May 16, 2022 22:14
@joshcooper joshcooper requested a review from a team May 16, 2022 22:14
@joshcooper
Copy link
Contributor Author

jenkins please test this with servertests

1 similar comment
@joshcooper
Copy link
Contributor Author

jenkins please test this with servertests

@joshcooper joshcooper marked this pull request as draft May 17, 2022 18:12
If any parameters/properties for a resource are deferred, then also defer the
per-resource validate method[1]. The latter is often used in cases when
parameters are mutually exclusive or certain combinations are not allowed.

Once all parameters for a resource are evaluated, then the resource's `validate`
method will be called, if one is defined using the puppet DSL.

[1] https://github.com/puppetlabs/puppet-specifications/blob/master/language/resource_types.md#validate
@joshcooper joshcooper marked this pull request as ready for review May 18, 2022 05:09
@joshcooper
Copy link
Contributor Author

jenkins please test this with servertests

1 similar comment
@joshcooper
Copy link
Contributor Author

jenkins please test this with servertests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants