-
Notifications
You must be signed in to change notification settings - Fork 39
vendor: Update runtime-spec package for compatibility with CRI-O #259
Conversation
Global update relying on: - dep ensure -update - dep prune Signed-off-by: Sebastien Boeuf <[email protected]>
6cab732 to
644c851
Compare
| BundlePathKey = "com.github.containers.virtcontainers.pkg.oci.bundle_path" | ||
| ) | ||
|
|
||
| type CompatOCIProcess struct { |
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.
I don't understand why this is being introduced on a vendoring change? It isn't actively being used so can't (shouldn't?) this be handled on a follow-up PR (along with a new test or two)?
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.
@jodh-intel I think @sboeuf is trying to implement a structure that would allow us to be both rc4 and rc5 compatible.
Those compat structures replace the spec.Process ones in the below calls, they are used.
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.
Right, after some digging I found it.
@sboeuf - it would have been helpful to reference this: opencontainers/runtime-spec#675 and/or opencontainers/runtime-spec@37391fb explicitly.
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.
Agreed. In particular, a mention to opencontainers/runtime-spec@37391fb in a comment would be helpful.
@sboeuf, can you quickly do that to make things a little clearer ?
jodh-intel
left a comment
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.
lgtm
| BundlePathKey = "com.github.containers.virtcontainers.pkg.oci.bundle_path" | ||
| ) | ||
|
|
||
| type CompatOCIProcess struct { |
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.
Right, after some digging I found it.
@sboeuf - it would have been helpful to reference this: opencontainers/runtime-spec#675 and/or opencontainers/runtime-spec@37391fb explicitly.
This commit updates the vendored version of the runtime-spec to v1.0.0-rc5. In order to support both Docker and CRI-O (using different runtime-spec versions), it also introduces a new compatibility structure inheriting from the one defined by runtime-spec. Signed-off-by: Sebastien Boeuf <[email protected]>
644c851 to
449497a
Compare
|
@sameo @jodh-intel I have added a short explanation as a comment in each Compat structure. Let me know if that's better ! |
|
@sboeuf One more question: Why do you want to keep the rc4 compatibility ? |
|
@sameo Because the config.json given by Docker is relying on this. |
|
Makes sense. Docker is based on rc4. |
This PR updates the vendored version of the runtime-spec to v1.0.0-rc5. In order to support both Docker and CRI-O (using different runtime-spec versions), it also introduces a new compatibility structure inheriting from the one defined by runtime-spec.