bugfix: create fifo is not exist #1766

Ace-Tang · 2018-07-23T01:49:19Z

Signed-off-by: Ace-Tang aceapril@126.com

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

fuweid

Could you mind to provide more information about this, like bug background, reproduce condition?

Ace-Tang · 2018-07-23T02:06:50Z

time="2018-07-22T14:14:48.348606775+08:00" level=error msg="failed to recover container: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd,  failed to get task: error creating fifo /run/containerd/fifo/079814706/38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd-stdin: no such file or directory"

pouchd重启后，fifo文件不见, 容器recovery失败, 触发原因

task exit channel异常退出，把io关掉了，就删了fifo，不是每次异常退出都会到close io

time="2018-07-22T14:11:09.434073949+08:00" level=info msg="the task has quit, id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd, err: rpc error: code = Canceled desc = context canceled, exitcode: 255, time: 0001-01-01 00:00:00 +0000 UTC"
ime="2018-07-22T14:11:09.438201368+08:00" level=error msg="failed to delete task, container id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd: grpc: the client connection is closing: failed precondition"
time="2018-07-22T14:11:09.454099927+08:00" level=error msg="failed to delete container, container id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd: grpc: the client connection is closing: failed precondition"

time="2018-07-22T14:11:09.455584777+08:00" level=info msg="finished to subscribe io, backend: jsonfile, id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd"
time="2018-07-22T14:11:09.455650528+08:00" level=info msg="finished to subscribe io, backend: jsonfile, id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd"

time="2018-07-22T14:11:09.458266373+08:00" level=info msg="close containerio backend: jsonfile, id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd"
time="2018-07-22T14:11:09.458777444+08:00" level=info msg="close containerio backend: jsonfile, id: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd"

[root@e69g11497.et15sqa /home/huamin.thm]
#ls /run/containerd/fifo/079814706
ls: cannot access /run/containerd/fifo/079814706: No such file or directory

fuweid · 2018-07-23T02:09:33Z

time="2018-07-22T14:14:48.348606775+08:00" level=error msg="failed to recover container: 38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd, failed to get task: error creating fifo /run/containerd/fifo/079814706/38891960ac3dc472f1c5f59a0c5a00f134c93adf04a65e6972dfc10669ddfcfd-stdin: no such file or director

See, you can put the main message as the comment for the function, right? 😄

Ace-Tang · 2018-07-23T02:19:16Z

You can see the fifo has a CREATE flag, why we not create directory if the directory not exist. I do not think the add function is only to fix bug, but make sense for the process.

codecov-io · 2018-07-23T02:22:06Z

Codecov Report

Merging #1766 into master will increase coverage by 0.03%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1766      +/-   ##
==========================================
+ Coverage   56.32%   56.35%   +0.03%     
==========================================
  Files         200      200              
  Lines       15657    15659       +2     
==========================================
+ Hits         8819     8825       +6     
+ Misses       5742     5741       -1     
+ Partials     1096     1093       -3

Flag	Coverage Δ
#critest	`33.54% <0%> (+0.06%)`	⬆️
#integrationtest	`37.63% <0%> (-0.06%)`	⬇️
#unittest	`20.32% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
daemon/containerio/cio.go	`62.5% <0%> (-1%)`	⬇️
ctrd/image.go	`78.71% <0%> (-0.5%)`	⬇️
daemon/mgr/container.go	`53.46% <0%> (+0.15%)`	⬆️
cri/v1alpha2/cri.go	`66.13% <0%> (+0.17%)`	⬆️
ctrd/container.go	`50.17% <0%> (+1.37%)`	⬆️

Signed-off-by: Ace-Tang <aceapril@126.com>

fuweid · 2018-07-23T04:52:51Z

daemon/containerio/cio.go

@@ -116,6 +116,12 @@ func copyIO(fifos *containerdio.FIFOSet, ioset *ioSet, tty bool) (_ *wgCloser, e
 		}
 	}()

+	// if fifos directory is not exist, create fifo will fails,
+	// also in case of fifo directory lost in container recovery process.
+	if _, err := os.Stat(fifos.Dir); err != nil && os.IsNotExist(err) {


if the error is not 404 not found, should we return it directly?

At first , 404 cause by http, then, we deal with non-exist file error to make fifo can be created successful.

404 is metaphor. I means that if the os.IsNotExist(err) is false, should we return err directly?

No, I means we only need to deal this kind error, and not check Mkdir's return.

But the Mkdir maybe fail, right? And if os.IsNotExist(err) is false, I don't think it's necessary to execute the following statements. how do you think?

cc @allencloud

We do not care about if mkdir' return, since if it fails , it means other problem, here we just deal with the problem the directory is not exist, so we also not deal with other error besides os.IsNotExist(err).

If other problem exist, fifo create will return. I do not want return error in the add funtion.

fuweid

The fifo will catch error if the mkdir fails to create dir. It's nit, but LGTM

pouchrobot added the size/XS label Jul 23, 2018

Ace-Tang changed the title ~~create fifo is not exist~~ bugfix: create fifo is not exist Jul 23, 2018

pouchrobot added the kind/bug This is bug report for project label Jul 23, 2018

fuweid suggested changes Jul 23, 2018

View reviewed changes

bugfix: create fifo directory is not exist

a40e6e8

Signed-off-by: Ace-Tang <aceapril@126.com>

fuweid reviewed Jul 23, 2018

View reviewed changes

fuweid approved these changes Jul 24, 2018

View reviewed changes

fuweid merged commit afc7a21 into AliyunContainerService:master Jul 24, 2018

pouchrobot mentioned this pull request Jul 26, 2018

Weekly Report of PouchContainer from 2018-07-19 to 2018-07-26 #1907

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix: create fifo is not exist #1766

bugfix: create fifo is not exist #1766

Ace-Tang commented Jul 23, 2018

fuweid left a comment

Ace-Tang commented Jul 23, 2018 •

edited by allencloud

Loading

fuweid commented Jul 23, 2018

Ace-Tang commented Jul 23, 2018 •

edited

Loading

codecov-io commented Jul 23, 2018 •

edited

Loading

fuweid Jul 23, 2018 •

edited

Loading

Ace-Tang Jul 23, 2018

fuweid Jul 23, 2018

Ace-Tang Jul 23, 2018

fuweid Jul 23, 2018

Ace-Tang Jul 23, 2018 •

edited

Loading

Ace-Tang Jul 23, 2018

fuweid left a comment

bugfix: create fifo is not exist #1766

bugfix: create fifo is not exist #1766

Conversation

Ace-Tang commented Jul 23, 2018

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

fuweid left a comment

Choose a reason for hiding this comment

Ace-Tang commented Jul 23, 2018 • edited by allencloud Loading

fuweid commented Jul 23, 2018

Ace-Tang commented Jul 23, 2018 • edited Loading

codecov-io commented Jul 23, 2018 • edited Loading

Codecov Report

fuweid Jul 23, 2018 • edited Loading

Choose a reason for hiding this comment

Ace-Tang Jul 23, 2018

Choose a reason for hiding this comment

fuweid Jul 23, 2018

Choose a reason for hiding this comment

Ace-Tang Jul 23, 2018

Choose a reason for hiding this comment

fuweid Jul 23, 2018

Choose a reason for hiding this comment

Ace-Tang Jul 23, 2018 • edited Loading

Choose a reason for hiding this comment

Ace-Tang Jul 23, 2018

Choose a reason for hiding this comment

fuweid left a comment

Choose a reason for hiding this comment

Ace-Tang commented Jul 23, 2018 •

edited by allencloud

Loading

Ace-Tang commented Jul 23, 2018 •

edited

Loading

codecov-io commented Jul 23, 2018 •

edited

Loading

fuweid Jul 23, 2018 •

edited

Loading

Ace-Tang Jul 23, 2018 •

edited

Loading