Skip to content

Conversation

@changyaowei
Copy link
Contributor

Ⅰ. Describe what this PR did

Get a valid user and image uid through imageStatus and ListImages CRI

Ⅱ. Does this pull request fix one issue?

fixes #2548

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Modify an exist test case

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov
Copy link

codecov bot commented Dec 11, 2018

Codecov Report

Merging #2549 into master will increase coverage by 0.06%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2549      +/-   ##
==========================================
+ Coverage   69.09%   69.16%   +0.06%     
==========================================
  Files         278      278              
  Lines       18552    18552              
==========================================
+ Hits        12819    12831      +12     
- Misses       4253     4254       +1     
+ Partials     1480     1467      -13
Flag Coverage Δ
#criv1alpha1test 31.22% <0%> (ø) ⬆️
#criv1alpha2test 35.43% <75%> (-0.08%) ⬇️
#integrationtest 40.56% <0%> (-0.02%) ⬇️
#nodee2etest 32.64% <75%> (+0.15%) ⬆️
#unittest 26.8% <75%> (-0.02%) ⬇️
Impacted Files Coverage Δ
cri/v1alpha2/cri_utils.go 89.47% <75%> (-0.29%) ⬇️
pkg/streams/utils.go 82.14% <0%> (-9.53%) ⬇️
ctrd/image.go 76.75% <0%> (-2.2%) ⬇️
cri/v1alpha2/cri.go 69.07% <0%> (+1.1%) ⬆️
daemon/mgr/container_utils.go 85.11% <0%> (+1.19%) ⬆️
pkg/meta/store.go 68.99% <0%> (+1.55%) ⬆️
daemon/logger/jsonfile/utils.go 73.17% <0%> (+1.62%) ⬆️
cri/v1alpha2/cri_wrapper.go 65.59% <0%> (+2.39%) ⬆️
cri/ocicni/cni_manager.go 70.58% <0%> (+11.76%) ⬆️

@CLAassistant
Copy link

CLAassistant commented Dec 11, 2018

CLA assistant check
All committers have signed the CLA.

@pouchrobot
Copy link
Collaborator

We found this is your first time to contribute to Pouch, @changyaowei
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/alibaba/pouch/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

imageUID, username := getUserFromImageUser(image.Config.User)
if imageUID != nil {
uid.Value = *imageUID
if image == nil || image.Config == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@HusterWan
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Dec 11, 2018
Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@changyaowei please use git commit -s --amend --no-edit and repush again. We need your sign off before merge 😄

Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fuweid fuweid merged commit f6c1ec3 into AliyunContainerService:master Dec 11, 2018
@changyaowei changyaowei deleted the image_cri branch December 11, 2018 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM one maintainer or community participant agrees to merge the pull reuqest. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Cannot get a valid user name by CRI ImageStatus or ListImages interface

5 participants