-
Notifications
You must be signed in to change notification settings - Fork 225
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
(GH-3310) Ensure plugin code for core types is available on local transport #3314
Conversation
@joshcooper I think the concern here would be if bolt were syncing over core types for the version it is based off (in our case puppet 7) to an agent where those would be different. It looks like modern agents (7 and 8) are pretty similar for core types. I was thinking this will be the simplest solution. Do you think we risk diverging core types between agents? |
@donoghuc I think we're probably pretty safe with the core types, I can't imagine we ever want to change them much at all. If an issue came up the solution is trivial anyway, just install a compatible agent version on the target. |
Looks like the apply integration tests are failing due to the large plugin tarball being passed to the fake apply task. Going to see about how best to address that in the tests. Throwing a DNM on it for now. |
Ideally we could both compile and apply catalogs based on the plugins shipped in bolt's package for local transport. For apply, we prepare a single plugin tarball regardless of transports and apply it. There is no established or clear way to switch behavior based on whether or not we are running on the local transport. The local transport aside, I'm thinking perhaps it may be best anyway to have the plugins from bolt's package in the plugin tarball so that agents (regardless of transport) would use the same code as the catalog was compiled against. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small thing
lib/bolt/applicator.rb
Outdated
@@ -284,6 +284,7 @@ def apply_ast(raw_ast, targets, options, plan_vars = {}) | |||
'catalog' => Puppet::Pops::Types::PSensitiveType::Sensitive.new(catalog), | |||
'plugins' => Puppet::Pops::Types::PSensitiveType::Sensitive.new(plugins), | |||
'apply_settings' => @apply_settings, | |||
'bolt_builtin_content' => @modulepath.full_modulepath - @modulepath.user_modulepath || [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing this calculation, we should add a new function to https://github.com/puppetlabs/bolt/blob/main/lib/bolt/config/modulepath.rb and document what it is there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YEah, i will have to do a significant amount of cleanup i think around all the mocking etc in tests. Just wanted to show this approach (and test it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see.
Yeah this approach is a good way to go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it looks like we just pass in resolved arrays into applicator anyway. I have tested this change on a fresh vmpooler image and it appears to work. Looks like our tests dont blow up with an extra task parameter so i'm just going to leave the comment and call it good.
… local transport When using the local transport defaulting to `bundled-ruby: true` it is assumed an agent is on the target. Generally when a `puppet-agent` feature is applied to a target this is assumed to be the case. This provides the core types and providers when an catalog is applied. We explicitly do not want to pluginsync all the core types and providers when we do not have to. This commit updates the apply_catalog task to take advantage of the fact that bolt packages have the core types in them. This works by sending as a task parameter the install location of bundled module content in bolt packages. After unpacking the plugins sent to the apply_catalog task, if the task finds any modules shipped with bolt, it will add those to the `LOAD_PATH`. This allows us to continue to avoid excessive pluginsync and also support the bundled-ruby config over the local transport. !bug * **Ensure core types are available for apply over local transport** [#puppetlabsGH-3310](puppetlabsGH-3310) Previously when using `bundled-ruby: true` config on the local transport core types were not available during catalog application. This commit makes the core types available by loading the bundled bolt module content shipped with bolt packages if it is present on the target.
After some discussion on internal slack i think the best path forward for now is to go with the approach in a094b30 as it avoids having to do any additional pluginsync and should be a cheap and effective way to ensure core types will be available during catalog application over the local transport with the default |
When using the local transport defaulting to
bundled-ruby: true
it is assumed an agent is on the target. Generally when apuppet-agent
feature is applied to a target this is assumed to be the case. This provides the core types and providers when an catalog is applied. We explicitly do not want to pluginsync all the core types and providers when we do not have to. This commit updates the apply_catalog task to take advantage of the fact that bolt packages have the core types in them. This works by sending as a task parameter the install location of bundled module content in bolt packages. After unpacking the plugins sent to the apply_catalog task, if the task finds any modules shipped with bolt, it will add those to theLOAD_PATH
. This allows us to continue to avoid excessive pluginsync and also support the bundled-ruby config over the local transport.