Pass through specific annotation to OCI runtime spec #37262
Conversation
8462ce8 to
52d67b3
Compare
| for k, v := range c.Config.Labels { | ||
| logrus.Info(k) | ||
| if oldValue, ok := s.Annotations[k]; ok { | ||
| return nil, fmt.Errorf("Key %q already exisit in Annotations, the new value will be ignored,"+ |
| } | ||
| } | ||
| } | ||
|
|
||
| return &s, nil |
There was a problem hiding this comment.
seems the code base of this RP is out of date
There was a problem hiding this comment.
I check the mainline and I think it's up to date... ^_^
Before this patch, docker daemon doesn't pass any annotations to OCI runtime spec.However annotations can be useful espacially for Runtime (Virtual Machine): kata/runv/gvisor and so on. It's not good enough to pass all labels as annotations for there will be some labels hold just for images.So we just need to paas two kinds of annotations from upper cluster like kubernets: First is user specific annotations which with prefix of `annotations.` The other is labels for docker used which is marked as internal. Signed-off-by: Haomin <caihaomin@huawei.com>
52d67b3 to
e3e292f
Compare
|
ping @justincormack @thaJeztah @AkihiroSuda What do you think about the new way to pass through |
|
Still hesitant to conflate labels and annotations through the same API I want to bring this up again in a maintainers meeting, but that won't be this week due to DockerCon |
|
@thaJeztah Thx very much！ Passing |
| @@ -34,6 +34,21 @@ var ( | |||
| deviceCgroupRuleRegex = regexp.MustCompile("^([acb]) ([0-9]+|\\*):([0-9]+|\\*) ([rwm]{1,3})$") | |||
| ) | |||
|
|
|||
| // These values are copy from k8s.io/kubernets/pkg/kubelet/dockershim types | |||
There was a problem hiding this comment.
nit: s/copy/copied/ or s/copy/taken/
| s.Annotations[key] = v | ||
| } | ||
|
|
||
| // interl labels for docker |
| logrus.Info(k) | ||
| if oldValue, ok := s.Annotations[k]; ok { | ||
| return nil, fmt.Errorf("Key %q already exisit in Annotations, the new value will be ignored,"+ | ||
| "oldvalue is %q, new value is %q", k, oldValue, v) |
There was a problem hiding this comment.
nit: s/oldvalue/old value/
|
I think it would be great to have a generic way to pass configuration down to a runtime, however it should be explicit rather than implicit (maybe some daemon level config, maybe a dedicated field). Because this doesn't quite seem like the right approach and this has gone stale, I'm going to close, but this is not an outright rejection of the idea behind this. |
- What I did
Before this patch, docker daemon doesn't pass any annotations
to OCI runtime spec.However annotations can be useful espacially
for Runtime (Virtual Machine): kata/runv/gvisor and so on.
It's not good enough to pass all labels as annotations for there
will be some labels hold just for images.So we just need to paas two
kinds of annotations from upper cluster like kubernets:
First is user specific annotations which with prefix of
annotations.The other is labels for docker used which is marked as internal.
Signed-off-by: Haomin caihaomin@huawei.com
- How I did it
@miaoyq, I pick up from #36181, and look into what
kuberneteshas done withannotations.We try to separate
labelinto different types and decide which can be paas through toruntime.- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)