bugfix: use image reference during ContainerStatus #2096
Conversation
If user repush image with the same reference, the image ID will be changed. For now, pouch daemon will remove the old image ID so that CRI fails to fetch the running container. Before upgrade pouch daemon image manager, we use reference to get image instead of id. Signed-off-by: Wei Fu <fhfuwei@163.com>
Codecov Report
@@ Coverage Diff @@
## master #2096 +/- ##
==========================================
- Coverage 65.35% 65.33% -0.02%
==========================================
Files 207 207
Lines 16226 16226
==========================================
- Hits 10605 10602 -3
- Misses 4311 4315 +4
+ Partials 1310 1309 -1
|
|
Could you help to review this? @xiechengsheng |
|
I'm sorry I have no idea what the |
| } | ||
| imageRef := imageInfo.ID |
There was a problem hiding this comment.
Emmm, I think we could directly use imageID variable name here.
There was a problem hiding this comment.
If user has changed Dockerfile and re-builds it with the same reference, the imageID will be changed. This is tricky part that we use reference to GetImage to make sure that ContainerStatus can be successfully executed after imageID was changed. That is why that we should use imageInfo.ID here.
There was a problem hiding this comment.
I know what you mean, what I want to say is should we use imageID := imageInfo.ID here to avoid the misunderstanding?
There was a problem hiding this comment.
CRI defines the imageRef in the protobuf. we just follow the rule here.
|
LGTM |
pouchrobot
added
the
LGTM
Signed-off-by: Wei Fu fhfuwei@163.com
Ⅰ. Describe what this PR did
If user repush image with the same reference, the image ID will be changed. For now, pouch daemon will remove the old image ID so that CRI fails to fetch the running container. Before upgrade pouch daemon image manager, we use reference to get image instead of id.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
NO
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews