Conversation
|
I think this could be a better choice than #37262 and it's really useful for runtimes other than runc to do more custom works |
|
there will be a related patch on cli and it already done. But I think we should let the |
|
Looks good, this would be very useful indeed ! |
api/types/container/config.go
Outdated
There was a problem hiding this comment.
I feel like the naming around this is too generic. If we want to provide passthroughs to the OCI runtime then naming/structure should reflect this.
There was a problem hiding this comment.
Do you have something in mind that would be a less generic naming ?
There was a problem hiding this comment.
@cpuguy83 you mean like OciAnnotations or RuntimeAnnotations or other things like these to indicate that this belongs to OCI spec
There was a problem hiding this comment.
Maybe something like RuntimeAnnotations... but I'm curious why these configurations aren't passed down as runtime args?
There was a problem hiding this comment.
Well if we use args, this means everytime a new thing comes up, we'll need to get those changes accepted. Annotations is the most generic way to pass arguments that might not be defined yet.
There was a problem hiding this comment.
Are these annotations unique for each individual container, or for the runtime that's used? (i.e., must each container have different annotations?)
There was a problem hiding this comment.
It's per container since they will be used to differentiate containers from pods.
Signed-off-by: Haomin <caihaomin@huawei.com>
Signed-off-by: Haomin <caihaomin@huawei.com>
ff5ebe7 to
8700ab2
Compare
|
@cpuguy83 config updated~ |
|
Hi All~ Can we get this design done ~~ 'cause it's really important for our vm based container and I think it will be really useful for docker users~ |
|
We discussed this at the maintainers meeting last week without a real conclusion. |
|
Anything new in come ? @cpuguy83 Hah~ |
|
ping docker maintainers @cpuguy83 @thaJeztah @tonistiigi @justincormack |
|
ping @cpuguy83 ^^ |
|
anything new~ @cpuguy83 PLZ let us know~ it's a really useful option ^^ |
|
@jshachm plz check those failing tests |
cpuguy83
left a comment
There was a problem hiding this comment.
I think this seems fine, but feel that the field needs be to moved to HostConfig.
Thanks for your patience on this. I wound up reviewing a similar (in goals) PR to this and pretty much came to the conclusion that this would be the best way forward, then low and behold I found this PR already exists.
Let me know if you still want to get this in.
| MacAddress string `json:",omitempty"` // Mac Address of the container | ||
| OnBuild []string // ONBUILD metadata that were defined on the image Dockerfile | ||
| Labels map[string]string // List of labels set to this container | ||
| RuntimeAnnotations map[string]string // List of runtime annotations set to this container |
There was a problem hiding this comment.
This should probably go in HostConfig instead of Config because this becomes part of the image config which does not seem desirable... we already made this mistake with Labels which wound up creating a lot of problems that we really couldn't fix, unfortunately.
|
@cpuguy83 Hah It's just a bit long time~ I still want to get this in. I will rebase first and get it done on the latest master branch... Quit a long time... Hah |
|
API side was implemented (but slightly different) in #45025 |
As talked in #37262, it will be very useful to support
annotationsflag indocker cli.Support
annotationwill bring a lot of benefits not only fordockershimmodel forkubernetesand alsonative docker users will have additional ways to pass there
key-valueto container.- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)