Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bugfix: move IO part behind to ensure execConfig result assignment before IO closes #1340

Merged
merged 1 commit into from
May 17, 2018
Merged

bugfix: move IO part behind to ensure execConfig result assignment before IO closes #1340

merged 1 commit into from
May 17, 2018

Conversation

ZouRui89
Copy link
Contributor

Signed-off-by: Zou Rui [email protected]

Ⅰ. Describe what this PR did

Right now flaky test happens once in a while because getExecConfig does not always get the execConfig result before IO is pipe closed.
This pr changed the sequence inside execExitedAndRelease, which pouchd will call to execute exit. By moving IO close operation behind the execConfig value assignment, the result getting is ensured.

Ⅱ. Does this pull request fix one issue?

fixes #1337

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added kind/bug This is bug report for project size/S labels May 16, 2018
YaoZengzeng
YaoZengzeng reviewed May 17, 2018
View reviewed changes
daemon/mgr/container.go Outdated
// close io
io.Close()
mgr.IOs.Remove(id)
}
// TODO: GC invalid mgr.ExecProcess.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rm this comment, we already have GC.

YaoZengzeng
YaoZengzeng reviewed May 17, 2018
View reviewed changes
daemon/mgr/container.go Outdated
// close io
io.Close()
mgr.IOs.Remove(id)
}
// TODO: GC invalid mgr.ExecProcess.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rm this comment, we already have GC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why comment the same thing twice? LOL :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I did it only once 😢

@ZouRui89
Copy link
Contributor Author

@YaoZengzeng Updated!

@YaoZengzeng
Copy link
Contributor

LGTM

@YaoZengzeng YaoZengzeng merged commit 521ca7c into AliyunContainerService:master May 17, 2018
@ZouRui89 ZouRui89 deleted the race_off branch May 17, 2018 02:40
@pouchrobot pouchrobot mentioned this pull request May 17, 2018
Closed
starnop added a commit to starnop/pouch that referenced this pull request May 20, 2018
@starnop
Squashed commit of the following:
6c6650d 
commit 75aab2b
Merge: 5c8993b 2f2e9c1
Author: Starnop <[email protected]>
Date:   Sat May 19 21:51:30 2018 +0800

    Merge branch 'cri-compatibility' of github.com:Starnop/pouch into cri-compatibility

commit 2f2e9c1
Merge: bd9c4fd acbd19a
Author: Starnop <[email protected]>
Date:   Fri May 18 15:32:31 2018 +0800

    Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility

commit acbd19a
Merge: 6cef601 8e862f9
Author: Yao Zengzeng <[email protected]>
Date:   Fri May 18 15:27:01 2018 +0800

    Merge pull request AliyunContainerService#1351 from fuweid/feature_allow_use_image_by_digest_id

    feature: allow use image by digest id

commit 6cef601
Merge: 4423313 caf45ec
Author: Wei Fu <[email protected]>
Date:   Fri May 18 15:10:30 2018 +0800

    Merge pull request AliyunContainerService#1350 from Ace-Tang/version

    build: generate version information at build time

commit caf45ec
Author: Ace-Tang <[email protected]>
Date:   Fri May 18 13:31:25 2018 +0800

    build: generate version information at build time

    auto-generate git commit, build time at build time,
    fix go-version hard code.

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

commit 8e862f9
Author: Wei Fu <[email protected]>
Date:   Fri May 18 13:50:42 2018 +0800

    feature: allow use image by digest id

    Basically, the user can use sha256:xyz to inspect image or run
    container.

    Signed-off-by: Wei Fu <[email protected]>

commit 4423313
Merge: 8410064 f67cf08
Author: Allen Sun <[email protected]>
Date:   Fri May 18 11:44:39 2018 +0800

    Merge pull request AliyunContainerService#1326 from shaloulcy/core_volume

    feature: remote volume driver

commit 8410064
Merge: 8af60a9 852d4f4
Author: Wei Fu <[email protected]>
Date:   Fri May 18 10:39:15 2018 +0800

    Merge pull request AliyunContainerService#1339 from rhinoceros/master

    docs: Update INSTALLATION.md apt-key fingerprint BE2F475F

commit f67cf08
Author: Eric Li <[email protected]>
Date:   Mon May 14 12:19:46 2018 +0800

    feature: remote volume driver

    Signed-off-by: Eric Li <[email protected]>

commit bd9c4fd
Merge: 8d01e30 521ca7c
Author: Starnop <[email protected]>
Date:   Thu May 17 11:11:40 2018 +0800

    Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility

commit 8af60a9
Merge: ecbe3b6 13a17b5
Author: Allen Sun <[email protected]>
Date:   Fri May 18 09:43:40 2018 +0800

    Merge pull request AliyunContainerService#1349 from pouchrobot/auto-doc-2018-05-18

    docs: auto generate pouch cli/api docs via code

commit 13a17b5
Author: pouchrobot <[email protected]>
Date:   Fri May 18 01:28:51 2018 +0000

    docs: auto generate pouch cli docs via code

    Signed-off-by: pouchrobot <[email protected]>

commit 5c8993b
Merge: 8d01e30 521ca7c
Author: Starnop <[email protected]>
Date:   Thu May 17 11:11:40 2018 +0800

    Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility

commit ecbe3b6
Merge: 3374daf 7fc11df
Author: Wei Fu <[email protected]>
Date:   Thu May 17 18:53:20 2018 +0800

    Merge pull request AliyunContainerService#1227 from Ace-Tang/full_spec_params

    feature: add pidslimit implement

commit 7fc11df
Author: Ace-Tang <[email protected]>
Date:   Fri May 11 16:25:32 2018 +0800

    feature: add pidslimit implement

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

commit 3374daf
Merge: 89a5ae5 fd8e0f7
Author: Allen Sun <[email protected]>
Date:   Thu May 17 13:44:52 2018 +0800

    Merge pull request AliyunContainerService#1341 from Letty5411/0517-assertfix

    test: enhance cli related tests

commit 89a5ae5
Merge: 521ca7c 8469cea
Author: Wei Fu <[email protected]>
Date:   Thu May 17 13:17:47 2018 +0800

    Merge pull request AliyunContainerService#1331 from Letty5411/0516-doc

    doc: update test.md about how to run test

commit fd8e0f7
Author: letty <[email protected]>
Date:   Thu May 17 11:23:16 2018 +0800

    test: enhance test

    Signed-off-by: letty <[email protected]>

commit 521ca7c
Merge: 19c956b 6b2a8b4
Author: Yao Zengzeng <[email protected]>
Date:   Thu May 17 10:39:06 2018 +0800

    Merge pull request AliyunContainerService#1340 from ZouRui89/race_off

    bugfix: move IO part behind to ensure execConfig result assignment before IO closes

commit 6b2a8b4
Author: Zou Rui <[email protected]>
Date:   Wed May 16 18:03:27 2018 +0800

    bugfix: move IO part behind to ensure execConfig result assignment before IO closes

    Signed-off-by: Zou Rui <[email protected]>

commit 8d01e30
Merge: 9339a70 19c956b
Author: Starnop <[email protected]>
Date:   Wed May 16 12:57:14 2018 +0800

    Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility

commit 852d4f4
Author: rhinoceros.xn <[email protected]>
Date:   Wed May 16 17:37:51 2018 +0800

    Update INSTALLATION.md

    $ apt-key fingerprint BE2F475F
    pub   4096R/BE2F475F 2018-02-28
          Key fingerprint = F443 EDD0 4A58 7E8B F645  9C40 CF68 F84A BE2F 475F
    uid                  opsx-admin <[email protected]>

commit 8469cea
Author: letty <[email protected]>
Date:   Wed May 16 10:08:32 2018 +0800

    doc: update test.md about how to run test

    Signed-off-by: letty <[email protected]>

commit 9339a70
Author: Starnop <[email protected]>
Date:   Tue May 15 17:35:53 2018 +0800

    cri com and up
@pouchrobot pouchrobot mentioned this pull request May 24, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YaoZengzeng YaoZengzeng YaoZengzeng left review comments

Assignees
No one assigned
Labels
kind/bug This is bug report for project size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[flaky test] CRI test failed when the pr is just doc-related
3 participants
@ZouRui89 @YaoZengzeng @pouchrobot