RFC-037: add chef_version and ohai_version metadata #4081
Conversation
most of our metadata is the form of: key "value" but chef_version can be: chef_version ">= 12.3.1", "< 13.0.0" so we need a splat in method messing and need to shovel it into an array. relates to chef/chef#4081
lamont-granquist
force-pushed
the
lcg/chef-version
branch
from
63fdd7c
to
19875bc
Compare
|
Tested on hosted and this works as designed.
Because of that latter point, we still need to do this: chef_version ">= 12.0" if respond_to?(:chef_version)So that old versions of knife can upload the cookbook. We might want to change knife so that unknown metadata is ignored (which means a method_missing that warns and then no-ops? mild sigh...), but that's out of scope for this. |
|
@chef/client-core ready for review |
| private | ||
|
|
||
| def gem_dep_matches?(what, version, *deps) | ||
| return true unless deps.length > 0 | ||
| deps.each do |dep| |
There was a problem hiding this comment.
I don't think this is the logic we want. What you have here is equiv to deps.any? {|dep| dep.match?(what, version) }. I think we want all? there.
There was a problem hiding this comment.
Ahh, I think I see how this works. Each "row" is loaded into a single dependency object so that covers the all?, and then we want an any? between each of those? That seems maybe a bit overcomplicated but I can dig it. I would still rewrite this as an any? for clarity.
There was a problem hiding this comment.
Heh, I think this was the last bit of code I wrote before demoing it on the last day of the summit. Its very obviously a reimplementation of #any? so I can certainly fix that...
There was a problem hiding this comment.
And yeah, this is the code that handles the "we backported a feature" use case.
|
Code and tests look good. Need to figure out the travis issue and add the docs you meant to write, then 👍 |
|
travis was timing out on something unrelated, rebooting the test fixed it. |
|
Added docs, discovered that the ...to_hash and ...from_hash methods were acting on Arrays in the process so cosmetically fixed those names... |
| # @param version_args [Array<String>] Version constraint in String form | ||
| # @return [Array<Gem::Dependency>] Current chef_versions array | ||
| def chef_version(*version_args) | ||
| @chef_versions << Gem::Dependency.new('chef', *version_args) |
There was a problem hiding this comment.
The fact that this returns a value makes it sound like you expect it to be used as a reader too? Calling chef_versions() would shift on an empty dependency, maybe add unless version_args.empty? here?
|
👍 |
|
👍 Thanks @lamont-granquist! |
|
What about chef-shell? People still use that, right? Seems like we should validate there too so you get the same behavior. |
|
Hmmm? The point of testing solo is because there's a solo-specific block in the |
|
Turns out I did an integration test for the solo-side of that conditional, but forgot the client-side. |
|
👍 |
lamont-granquist
force-pushed
the
lcg/chef-version
branch
from
3e3a0be
to
a75ce7c
Compare
|
I'll push the CHANGELOG after meeting.... |
RFC-037: add chef_version and ohai_version metadata
This allows knife to upload cookbooks containing chef_version and ohai_version. When the chef-client downloads cookbooks with constraints it checks them and throws an exception if the constraints do not match. The knife command will upload cookbooks which do not match the version constraint (this is deliberate in the case where knife workstation versions differ from chef-client production versions -- injecting the production chef-client version into knife to prevent uploads is out-of-scope).
Also out of scope - including these into /universe, teaching berkshelf and the policyfile depsolver to avoid them, injecting production chef-version values into policyfiles, teaching the cookbook_versions endpoint to filter based on chef_version, etc, etc, etc, etc, etc.... This is only step number one.
This PR does go slightly beyond the RFC and includes the ability to not only pin via a simple constraint like '~> 12.3' but also to allow multiple pins to handle features that were back-ported:
That will be satisfied by a chef version that is 11.x >= 11.20.0, or 12.x >= 12.6.0, or >= 13.