Skip to content

Conversation

@Ace-Tang
Copy link
Contributor

if pouch update a container get error, restore container
config, aviod container record error config.

Fixes: #1512

Signed-off-by: Ace-Tang [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Fixes: #1512

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

if pouch update a container get error, restore container
config, aviod container record error config.

Fixes: #1512

Signed-off-by: Ace-Tang <[email protected]>
@pouchrobot pouchrobot added kind/bug This is bug report for project size/S labels Jun 12, 2018
@codecov-io
Copy link

Codecov Report

Merging #1513 into master will decrease coverage by 0.08%.
The diff coverage is 35.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1513      +/-   ##
==========================================
- Coverage   40.56%   40.48%   -0.09%     
==========================================
  Files         254      254              
  Lines       16535    16547      +12     
==========================================
- Hits         6708     6699       -9     
- Misses       8965     8980      +15     
- Partials      862      868       +6
Impacted Files Coverage Δ
daemon/mgr/container.go 48.8% <35.29%> (-0.49%) ⬇️
ctrd/watch.go 75% <0%> (-3.13%) ⬇️
ctrd/image.go 77.71% <0%> (-1.72%) ⬇️
ctrd/container.go 49.45% <0%> (-0.73%) ⬇️
daemon/mgr/network.go 68.94% <0%> (-0.49%) ⬇️

@allencloud allencloud added the priority/P1 this is high priority that all maintainers should stop to handle this issue label Jun 12, 2018
@allencloud
Copy link
Collaborator

allencloud commented Jun 12, 2018

I think this issue is quite severe, and we need to fix this ASAP.
assign to @HusterWan

@HusterWan
Copy link
Contributor

since this problem is severe, i will first merge it.

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jun 12, 2018
@HusterWan HusterWan merged commit 8545de4 into AliyunContainerService:master Jun 12, 2018
@Ace-Tang Ace-Tang deleted the fix_update_resource branch June 12, 2018 09:26
return err
}

restore := false
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am wondering if we need the new variable restore. Could we just decide whether to rollback via the nullable of err? @Ace-Tang @HusterWan

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 LGTM one maintainer or community participant agrees to merge the pull reuqest. priority/P1 this is high priority that all maintainers should stop to handle this issue size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: pouch update fail forever when one update command fail

5 participants