Skip to content

Conversation

@knightXun
Copy link
Contributor

Ⅰ. Describe what this PR did

Bugfix: pouch exec with env will not replace old envs.
This pr is to fix it.
Add a test for this situation.

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov
Copy link

codecov bot commented Sep 25, 2018

Codecov Report

Merging #2275 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2275      +/-   ##
==========================================
- Coverage    66.8%   66.77%   -0.03%     
==========================================
  Files         208      208              
  Lines       16934    16934              
==========================================
- Hits        11312    11308       -4     
- Misses       4260     4262       +2     
- Partials     1362     1364       +2
Flag Coverage Δ
#criv1alpha1test 32.63% <100%> (+0.12%) ⬆️
#criv1alpha2test 36.14% <100%> (+0.16%) ⬆️
#integrationtest 39.49% <100%> (-0.05%) ⬇️
#nodee2etest 33.4% <100%> (-0.03%) ⬇️
#unittest 23.75% <0%> (ø) ⬆️
Impacted Files Coverage Δ
daemon/mgr/container_exec.go 67.28% <100%> (ø) ⬆️
apis/server/utils.go 61.9% <0%> (-4.77%) ⬇️
pkg/meta/store.go 62.5% <0%> (-1.57%) ⬇️
daemon/mgr/container_utils.go 83.13% <0%> (-1.21%) ⬇️
cri/v1alpha2/cri_wrapper.go 60% <0%> (-1.21%) ⬇️
ctrd/container.go 59.28% <0%> (-0.48%) ⬇️
cri/v1alpha2/cri.go 66.76% <0%> (-0.3%) ⬇️
daemon/mgr/container.go 57.39% <0%> (ø) ⬆️
cri/v1alpha1/cri.go 61.88% <0%> (+0.33%) ⬆️
daemon/containerio/container_io.go 77.04% <0%> (+1.09%) ⬆️
... and 2 more

@pouchrobot pouchrobot added kind/bug This is bug report for project size/S labels Sep 25, 2018
@pouchrobot
Copy link
Collaborator

@knightXun Thanks for your contribution. 🍻
Please sign off in each of your commits.

@knightXun
Copy link
Contributor Author

/cc @fuweid @allencloud

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 36d308e into AliyunContainerService:master Sep 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug This is bug report for project size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants