Skip to content

Commit c3e1288

Browse files
jokemanfirejokemanfire
committed
Change fifo to pipe in shim like go shim
Signed-off-by: jokemanfire <[email protected]>
1 parent e69120e commit c3e1288

File tree

5 files changed

+90
-21
lines changed

5 files changed

+90
-21
lines changed

‎crates/runc-shim/src/common.rs

+24-13
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ use std::{
2929

3030
use containerd_shim::{
3131
api::{ExecProcessRequest, Options},
32-
io_error, other, other_error,
33-
util::IntoOption,
34-
Error,
32+
io_error, other, other_error, Error,
3533
};
36-
use log::{debug, warn};
34+
use log::{debug, info, warn};
3735
use nix::{
3836
cmsg_space,
3937
sys::{
@@ -43,7 +41,7 @@ use nix::{
4341
};
4442
use oci_spec::runtime::{LinuxNamespaceType, Spec};
4543
use runc::{
46-
io::{Io, NullIo, FIFO},
44+
io::{IOOption, Io, NullIo, PipedIo},
4745
options::GlobalOpts,
4846
Runc, Spawner,
4947
};
@@ -76,8 +74,8 @@ pub struct ProcessIO {
7674

7775
pub fn create_io(
7876
id: &str,
79-
_io_uid: u32,
80-
_io_gid: u32,
77+
io_uid: u32,
78+
io_gid: u32,
8179
stdio: &Stdio,
8280
) -> containerd_shim::Result<ProcessIO> {
8381
let mut pio = ProcessIO::default();
@@ -100,19 +98,32 @@ pub fn create_io(
10098

10199
if scheme == FIFO_SCHEME {
102100
debug!(
103-
"create named pipe io for container {}, stdin: {}, stdout: {}, stderr: {}",
101+
"create pipe io for container {}, stdin: {}, stdout: {}, stderr: {}",
104102
id,
105103
stdio.stdin.as_str(),
106104
stdio.stdout.as_str(),
107105
stdio.stderr.as_str()
108106
);
109-
let io = FIFO {
110-
stdin: stdio.stdin.to_string().none_if(|x| x.is_empty()),
111-
stdout: stdio.stdout.to_string().none_if(|x| x.is_empty()),
112-
stderr: stdio.stderr.to_string().none_if(|x| x.is_empty()),
107+
108+
// let io = FIFO {
109+
// stdin: stdio.stdin.to_string().none_if(|x| x.is_empty()),
110+
// stdout: stdio.stdout.to_string().none_if(|x| x.is_empty()),
111+
// stderr: stdio.stderr.to_string().none_if(|x| x.is_empty()),
112+
// };
113+
// pio.copy = false;
114+
115+
if stdio.stdin.is_empty() {
116+
debug!("stdin is empty");
117+
}
118+
let opts = IOOption {
119+
open_stdin: !stdio.stdin.is_empty(),
120+
open_stdout: !stdio.stdout.is_empty(),
121+
open_stderr: !stdio.stderr.is_empty(),
113122
};
123+
let io = PipedIo::new(io_uid, io_gid, &opts).unwrap();
124+
pio.copy = true;
125+
114126
pio.io = Some(Arc::new(io));
115-
pio.copy = false;
116127
}
117128
Ok(pio)
118129
}

‎crates/runc-shim/src/processes.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use tokio::{
3737
sync::oneshot::{channel, Receiver, Sender},
3838
};
3939

40-
use crate::io::Stdio;
40+
use crate::{common::ProcessIO, io::Stdio};
4141

4242
#[async_trait]
4343
pub trait Process {
@@ -71,6 +71,7 @@ pub struct ProcessTemplate<S> {
7171
pub state: Status,
7272
pub id: String,
7373
pub stdio: Stdio,
74+
pub io: Option<Arc<ProcessIO>>,
7475
pub pid: i32,
7576
pub exit_code: i32,
7677
pub exited_at: Option<OffsetDateTime>,
@@ -86,6 +87,7 @@ impl<S> ProcessTemplate<S> {
8687
state: Status::CREATED,
8788
id: id.to_string(),
8889
stdio,
90+
io: None,
8991
pid: 0,
9092
exit_code: 0,
9193
exited_at: None,

‎crates/runc-shim/src/runc.rs

+38-5
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,10 @@ impl RuncFactory {
163163
(Some(s), None)
164164
} else {
165165
let pio = create_io(&id, opts.io_uid, opts.io_gid, stdio)?;
166-
create_opts.io = pio.io.as_ref().cloned();
167-
(None, Some(pio))
166+
let ref_pio = Arc::new(pio);
167+
create_opts.io = ref_pio.io.clone();
168+
init.io = Some(ref_pio.clone());
169+
(None, Some(ref_pio))
168170
};
169171

170172
let resp = init
@@ -178,6 +180,22 @@ impl RuncFactory {
178180
}
179181
return Err(runtime_error(bundle, e, "OCI runtime create failed").await);
180182
}
183+
if !init.stdio.stdin.is_empty() {
184+
let stdin_clone = init.stdio.stdin.clone();
185+
let stdin_w = init.stdin.clone();
186+
// Open the write side in advance to make sure read side will not block,
187+
// open it in another thread otherwise it will block too.
188+
tokio::spawn(async move {
189+
if let Ok(stdin_w_file) = OpenOptions::new()
190+
.write(true)
191+
.open(stdin_clone.as_str())
192+
.await
193+
{
194+
let mut lock_guard = stdin_w.lock().unwrap();
195+
*lock_guard = Some(stdin_w_file);
196+
}
197+
});
198+
}
181199
copy_io_or_console(init, socket, pio, init.lifecycle.exit_signal.clone()).await?;
182200
let pid = read_file_to_str(pid_path).await?.parse::<i32>()?;
183201
init.pid = pid;
@@ -232,6 +250,7 @@ impl ProcessFactory<ExecProcess> for RuncExecFactory {
232250
stderr: req.stderr.to_string(),
233251
terminal: req.terminal,
234252
},
253+
io: None,
235254
pid: 0,
236255
exit_code: 0,
237256
exited_at: None,
@@ -394,8 +413,10 @@ impl ProcessLifecycle<ExecProcess> for RuncExecLifecycle {
394413
(Some(s), None)
395414
} else {
396415
let pio = create_io(&p.id, self.io_uid, self.io_gid, &p.stdio)?;
397-
exec_opts.io = pio.io.as_ref().cloned();
398-
(None, Some(pio))
416+
let ref_pio = Arc::new(pio);
417+
exec_opts.io = ref_pio.io.clone();
418+
p.io = Some(ref_pio.clone());
419+
(None, Some(ref_pio))
399420
};
400421
//TODO checkpoint support
401422
let exec_result = self
@@ -457,6 +478,15 @@ impl ProcessLifecycle<ExecProcess> for RuncExecLifecycle {
457478

458479
async fn delete(&self, p: &mut ExecProcess) -> Result<()> {
459480
self.exit_signal.signal();
481+
//close pipe read
482+
if !p.stdio.is_null() {
483+
if let Some(c) = p.io.clone() {
484+
if let Some(io) = c.io.clone() {
485+
io.close_all_sid();
486+
}
487+
}
488+
}
489+
debug!("Do close io complete");
460490
let exec_pid_path = Path::new(self.bundle.as_str()).join(format!("{}.pid", p.id));
461491
remove_file(exec_pid_path).await.unwrap_or_default();
462492
Ok(())
@@ -568,6 +598,7 @@ pub async fn copy_io(pio: &ProcessIO, stdio: &Stdio, exit_signal: Arc<ExitSignal
568598
stdout,
569599
exit_signal.clone(),
570600
Some(move || {
601+
debug!("stdout exit.....................");
571602
drop(stdout_r);
572603
}),
573604
);
@@ -594,6 +625,7 @@ pub async fn copy_io(pio: &ProcessIO, stdio: &Stdio, exit_signal: Arc<ExitSignal
594625
stderr,
595626
exit_signal,
596627
Some(move || {
628+
debug!("stderr exit.....................");
597629
drop(stderr_r);
598630
}),
599631
);
@@ -632,7 +664,7 @@ where
632664
async fn copy_io_or_console<P>(
633665
p: &mut ProcessTemplate<P>,
634666
socket: Option<ConsoleSocket>,
635-
pio: Option<ProcessIO>,
667+
pio: Option<Arc<ProcessIO>>,
636668
exit_signal: Arc<ExitSignal>,
637669
) -> Result<()> {
638670
if p.stdio.terminal {
@@ -670,6 +702,7 @@ impl Spawner for ShimExecutor {
670702
}
671703
};
672704
let pid = child.id().unwrap();
705+
673706
let (stdout, stderr, exit_code) = tokio::join!(
674707
read_std(child.stdout),
675708
read_std(child.stderr),

‎crates/runc/src/io.rs

+23-1
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ pub trait Io: Debug + Send + Sync {
7575

7676
/// Only close write side (should be stdout/err "from" runc process)
7777
fn close_after_start(&self);
78+
79+
/// Close read side
80+
fn close_all_sid(&self);
7881
}
7982

8083
#[derive(Debug, Clone)]
@@ -227,7 +230,7 @@ impl Io for PipedIo {
227230

228231
if let Some(p) = self.stderr.as_ref() {
229232
let pw = p.wr.try_clone()?;
230-
cmd.stdout(pw);
233+
cmd.stderr(pw);
231234
}
232235

233236
Ok(())
@@ -242,6 +245,17 @@ impl Io for PipedIo {
242245
nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stderr: {}", e));
243246
}
244247
}
248+
249+
fn close_all_sid(&self) {
250+
if let Some(p) = self.stdout.as_ref() {
251+
debug!("close pipe read from stdout");
252+
nix::unistd::close(p.rd.as_raw_fd()).unwrap_or_else(|e| debug!("close stdout: {}", e));
253+
}
254+
if let Some(p) = self.stderr.as_ref() {
255+
debug!("close pipe read from stderr");
256+
nix::unistd::close(p.rd.as_raw_fd()).unwrap_or_else(|e| debug!("close stderr: {}", e));
257+
}
258+
}
245259
}
246260

247261
/// IO driver to direct output/error messages to /dev/null.
@@ -273,6 +287,8 @@ impl Io for NullIo {
273287
let mut m = self.dev_null.lock().unwrap();
274288
let _ = m.take();
275289
}
290+
291+
fn close_all_sid(&self) {}
276292
}
277293

278294
/// Io driver based on Stdio::inherited(), to direct outputs/errors to stdio.
@@ -296,6 +312,8 @@ impl Io for InheritedStdIo {
296312
}
297313

298314
fn close_after_start(&self) {}
315+
316+
fn close_all_sid(&self) {}
299317
}
300318

301319
/// Io driver based on Stdio::piped(), to capture outputs/errors from runC.
@@ -319,6 +337,8 @@ impl Io for PipedStdIo {
319337
}
320338

321339
fn close_after_start(&self) {}
340+
341+
fn close_all_sid(&self) {}
322342
}
323343

324344
/// FIFO for the scenario that set FIFO for command Io.
@@ -353,6 +373,8 @@ impl Io for FIFO {
353373
}
354374

355375
fn close_after_start(&self) {}
376+
377+
fn close_all_sid(&self) {}
356378
}
357379

358380
#[cfg(test)]

‎crates/runc/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,6 @@ impl Runc {
383383
Ok(())
384384
});
385385
}
386-
387386
let (status, pid, stdout, stderr) = self.spawner.execute(cmd).await?;
388387
if status.success() {
389388
let output = if combined_output {
@@ -425,6 +424,7 @@ impl Runc {
425424
}
426425
args.push(id.to_string());
427426
let mut cmd = self.command(&args)?;
427+
428428
match opts {
429429
Some(CreateOpts { io: Some(io), .. }) => {
430430
io.set(&mut cmd).map_err(Error::UnavailableIO)?;
@@ -618,6 +618,7 @@ impl Spawner for DefaultExecutor {
618618
let mut cmd = cmd;
619619
let child = cmd.spawn().map_err(Error::ProcessSpawnFailed)?;
620620
let pid = child.id().unwrap();
621+
621622
let result = child
622623
.wait_with_output()
623624
.await

0 commit comments

Comments
 (0)