Add chef_version and ohai_version to database tables

[lamont-granquist]

This is the task to extract the chef_version and ohai_version out of the uploaded cookbook metadata and to get them into a table.

Displaying the data in the UI comes later (and I actually don't super care about it as a priority)

Getting the chef_version and ohai_version into /universe also comes later (this is what I want to implement on top of this issue).

[robbkidd]

@lamont-granquist Can you add some examples of the various forms these versions may come in via cookbook metadata? I recall something about strings or arrays of strings.

[lamont-granquist]

PR that added them to chef/chef: chef/chef#4081

[lamont-granquist]

Here's an example of the sub-JSON:

https://github.com/chef/chef/blob/a75ce7ce4c5ee9565f536430244a37bc33cd4e59/lib/chef/cookbook/metadata.rb#L526

[nellshamrell]

@lamont-granquist: what am I looking for? I see that a cookbook itself can have multiple chef versions and multiple ohai versions, but I believe a cookbook VERSION can only have one chef version and one ohai version. Is there something that needs to change?

[nellshamrell]

OH wait a minute. It looks like people can specify that a cookbook works with multiple versions of Chef in their metadata.rb. Therefore, I believe the chef_version and ohai_version attributes should be on the Cookbook object, rather than the CookbookVersion object and should allow arrays of strings. Any objections to this?

[lamont-granquist]

i think i ninja answered you?

also note that they key will be "ohai_versions" and "chef_versions" in the plural.

[nellshamrell]

Right. So it looks like the changes that need to be made are:

1. Rename ohai_version and chef_version to ohai_versions and chef_versions
2. Move ohai_versions and chef_versions to be attributes on the Cookbook object, rather than the CookbookVersion object
3. Add in the ability to handle an array of strings.

Is this correct?

[lamont-granquist]

I'm not sure what Cookbook and CookbookVersion mean to you? Each version of a cookbook will have different chef_versions and ohai_versions.

[lamont-granquist]

So for a 'foo' cookbook:

foo 0.0.0 may not have anything specified
foo 1.0.0 may add something like chef_versions: [ [ "~> 11.14.0" ] ]
foo 2.0.0 may move to something like chef_versions: [ [ "~> 11.18.0" ], [ "~> 12.3.0" ]]

[nellshamrell]

Got it!

[robbkidd]

My read is they should stay on CookbookVersion. A new version of a cookbook could make use of an ohai or chef feature that requires different ohai/chef versions than a previous cookbook release.

[nellshamrell]

Understood.

[robbkidd]

Or... what Lamont said.

[lamont-granquist]

It should look something like how you handle the 'depends' field most likely?

only its going to contain an array-of-arrays...

[nellshamrell]

Does it really need to be an array of arrays? (I know that's what is done on the Chef Server, but hear me out). Or would this be acceptable:

Cookbook Name: MyCookbook
Cookbook Versions:
MyCookbookVersion1
* chef_versions = ['12.4.1','12.3.1']
* ohai_versions = ['8.0.0'],['8.1.0']
MyCookbookVersion2
* chef_versions = ['12.5.1','12.4.1']
* ohai_versions = ['8.0.1'],['8.1.2']

So I would add these columns to CookbookVersion - chef_versions and ohai_versions.

Is there a need for the array of arrays that I am not seeing?

[lamont-granquist]

Its not a version really its a version constraint. Simple version constraints rendered as strings may be "~> 12.0" or "= 1.2.3", but a more complicated constraint is ">= 12.4.3", "<= 13.0.0".

So you could have something gnarly like:

"chef_versions": [
  [ ">= 11.14.5", "< 12.0.0" ],
  [ ">= 12.4.2", "< 13.0.0" ]
]

The way to read that is that the inner array elements are joined by "&&" while the outer array is joined by "||" operators.

So that constraint would be appropriate if there was some bug that was fixed in chef or some new feature introduced in 11.14.5 and 12.4.2 which the cookbook depended upon, and it was expected that it worked for any future version of 11.x or 12.x, but couldn't be guaranteed to work on some future 13.x

The ~> operator also doesn't work here since ~> 11.14.5 means >= 11.14.5, < 11.15.0

[robbkidd]

Can the same thing not be accomplished with:

"chef_versions": [
  ">= 11.14.5",
  "< 12.0.0",
  ">= 12.4.2",
  "< 13.0.0",
]

Like, "hey, dependency resolver, solve for all."

[lamont-granquist]

If you join those with '&&' that has no solution. If you join those with '||' then everything passes.

[nellshamrell]

I see your point and it looks like we do indeed need an array of arrays or some other data structure. Will return to this in the morning :)

[robbkidd]

A-ha. Paired upper and lower bounds, pick one. I see.

[lamont-granquist]

The only real alternative is something like:

"chef_versions": [
  ">= 11.14.5, < 12.0.0",
  ">= 12.4.2, < 13.0.0"
]

But you're just joining and splitting on ',' all the time if you go down that route--there might conceivably be a parser bug in there somewhere as well, although I think it would require something perverse like allowing version numbers with commas in them...

[robbkidd]

It is possible to store the array of arrays data structure as given in the metadata. We could use a JSON column in the database. As @nellshamrell says, in the morning. Fresh light of day and all.

[lamont-granquist]

Yep, that sounds like a good approach. I don't particularly like the idea of rendering them back and forwards as strings because at some point we may wind up with mutually incompatible JSON formats running around the API...