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

doc:fix poststart doc #1259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

doc:fix poststart doc #1259

wants to merge 1 commit into from
+2 −4

Conversation

ningmingxiao
Copy link

@ningmingxiao ningmingxiao commented Jul 13, 2024 •
edited
Loading

I find poststart is in runc create stage

kolyshkin
kolyshkin requested changes Jul 13, 2024
View reviewed changes
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

This change is obviously wrong, or perhaps I'm missing something here?

Looking into runc sources, I see that poststart is run right after we start init, but before we tell it to continue. This is kind of what the current doc says.

@ningmingxiao
Copy link
Author

ningmingxiao commented Jul 13, 2024 •
edited
Loading

This change is obviously wrong, or perhaps I'm missing something here?

Looking into runc sources, I see that poststart is run right after we start init, but before we tell it to continue. This is kind of what the current doc says.

what does "user-specified process" means ?

{
    "ociVersion": "1.0.2-dev",
    "process": {
            "terminal": false,
            "user": {
                    "uid": 0,
                    "gid": 0
            },
            "args": [
                    "sh"
            ],
....

does it refer to this run custom process ? In runc start

@utam0k
Copy link
Member

utam0k commented Jul 14, 2024

what does "user-specified process" means ?

It means a container process. I've double-checked runc's code, and I found a poststart hook is kicked after a container process started.

https://github.com/opencontainers/runc/blob/3778ae603c706494fd1e2c2faf83b406e38d687d/libcontainer/container_linux.go#L350-L352

https://github.com/opencontainers/runc/blob/3778ae603c706494fd1e2c2faf83b406e38d687d/libcontainer/container_linux.go#L362-L367

@utam0k
Copy link
Member

utam0k commented Jul 14, 2024 •
edited
Loading

Feel free to re-open if there is anything you want to discuss. Thanks for your contribution 🙏

@utam0k utam0k closed this Jul 14, 2024
@ningmingxiao
Copy link
Author

ningmingxiao commented Jul 14, 2024 •
edited by utam0k
Loading

config.json

{
    "ociVersion": "1.0.2-dev",
    "process": {
            "terminal": false,
            "user": {
                    "uid": 0,
                    "gid": 0
            },
            "args": [
                    "sh"
            ],
            "env": [
                    "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
                    "TERM=xterm"
            ],
            "cwd": "/",
            "capabilities": {
                    "bounding": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ],
                    "effective": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ],
                    "permitted": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ],
                    "ambient": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ]
            },
            "rlimits": [
                    {
                            "type": "RLIMIT_NOFILE",
                            "hard": 1024,
                            "soft": 1024
                    }
            ],
            "noNewPrivileges": true
    },
    "root": {
            "path": "rootfs",
            "readonly": false
    },
    "hostname": "runc",
    "mounts": [
            {
                    "destination": "/proc",
                    "type": "proc",
                    "source": "proc"
            },
            {
                    "destination": "/dev",
                    "type": "tmpfs",
                    "source": "tmpfs",
                    "options": [
                            "nosuid",
                            "strictatime",
                            "mode=755",
                            "size=65536k"
                    ]
            },
            {
                    "destination": "/dev/pts",
                    "type": "devpts",
                    "source": "devpts",
                    "options": [
                            "nosuid",
                            "noexec",
                            "newinstance",
                            "ptmxmode=0666",
                            "mode=0620"
                    ]
            },
            {
                    "destination": "/dev/shm",
                    "type": "tmpfs",
                    "source": "shm",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev",
                            "mode=1777",
                            "size=65536k"
                    ]
            },
            {
                    "destination": "/dev/mqueue",
                    "type": "mqueue",
                    "source": "mqueue",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev"
                    ]
            },
            {
                    "destination": "/sys",
                    "type": "sysfs",
                    "source": "sysfs",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev",
                            "ro"
                    ]
            },
            {
                    "destination": "/sys/fs/cgroup",
                    "type": "cgroup",
                    "source": "cgroup",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev",
                            "relatime",
                            "ro"
                    ]
            }
    ],
    "hooks": {
            "Poststart": [
                {
                    "path": "/usr/bin/sleep",
                    "args":  ["/usr/bin/sleep","10"]
                }
            ]
    },
    "linux": {
            "resources": {
                    "devices": [
                            {
                                    "allow": false,
                                    "access": "rwm"
                            }
                    ]
            },
            "uidMappings": [
                    {
                            "containerID": 0,
                            "hostID": 1000,
                            "size": 1
                    }
            ],
            "gidMappings": [
                    {
                            "containerID": 0,
                            "hostID": 1000,
                            "size": 1
                    }
            ],
            "namespaces": [
                    {
                            "type": "pid"
                    },
                    {
                            "type": "network"
                    },
                    {
                            "type": "ipc"
                    },
                    {
                            "type": "uts"
                    },
                    {
                            "type": "mount"
                    }
            ],
            "maskedPaths": [
                    "/proc/acpi",
                    "/proc/asound",
                    "/proc/kcore",
                    "/proc/keys",
                    "/proc/latency_stats",
                    "/proc/timer_list",
                    "/proc/timer_stats",
                    "/proc/sched_debug",
                    "/sys/firmware",
                    "/proc/scsi"
            ],
            "readonlyPaths": [
                    "/proc/bus",
                    "/proc/fs",
                    "/proc/irq",
                    "/proc/sys",
                    "/proc/sysrq-trigger"
            ]
    }
}

I set

            "Poststart": [
                {
                    "path": "/usr/bin/sleep",
                    "args":  ["/usr/bin/sleep","10"]
                }

[root@localhost mycontainer]# time runc create test0001

real    0m10.199s
user    0m0.010s
sys     0m0.035s

that means user-specified process doesn't run. (user-specified process will run in runc start stage). @utam0k
does user-specified process means following code ?

            "args": [
                    "sh"
            ],

@cyphar cyphar mentioned this pull request Jul 15, 2024
Closed
@cyphar cyphar reopened this Jul 15, 2024
@cyphar
Copy link
Member

cyphar commented Jul 15, 2024 •
edited
Loading

@utam0k process.start doesn't actually start the process, that's actually the implementation of runc create. runc start calls container.Exec. (Yes this is confusing, this happened because when we added runc create we changed the semantics of runc start and it seems we forgot to rename the internal functions properly.)

I think this is a runc bug.

@kolyshkin

Looking into runc sources, I see that poststart is run right after we start init, but before we tell it to continue. This is kind of what the current doc says.

That is what we are doing, but that's not what the spec says should happen AFAICS. The spec says that we should run poststart after the user-specified process is started (so during runc start after we signal the exec FIFO to start, not runc create as we are currently doing).

cyphar
cyphar requested changes Jul 15, 2024
View reviewed changes
Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

If runc's behaviour doesn't match the spec then we need to fix runc.

@cyphar cyphar mentioned this pull request Jul 15, 2024
Open
@utam0k
Copy link
Member

utam0k commented Jul 15, 2024

@utam0k process.start doesn't actually start the process, that's actually the implementation of runc create. runc start calls container.Exec. (Yes this is confusing, this happened because when we added runc create we changed the semantics of runc start and it seems we forgot to rename the internal functions properly.)

@cyphar @ningmingxiao I revised the runc's code and I found my misunderstanding. Thanks for pointing out it to me 🙏

If runc's behaviour doesn't match the spec then we need to fix runc.

+1

@utam0k
Copy link
Member

utam0k commented Jul 15, 2024

I wonder if we should check other runtimes ...

@ningmingxiao
Copy link
Author

I wonder if we should check other runtimes ...

crun have same problem

@utam0k
Copy link
Member

utam0k commented Jul 15, 2024

I wonder if we should check other runtimes ...

crun have same problem

Thanks!

cc: @giuseppe

@utam0k
Copy link
Member

utam0k commented Jul 15, 2024

I wonder if we should check other runtimes ...

crun have same problem

@ningmingxiao
youki doesn't have the problem, doesn't it?

$ time sudo ./youki create -b postStart-bug test
sudo ./youki create -b postStart-bug test  0.00s user 0.01s system 17% cpu 0.047 total

https://github.com/containers/youki/blob/9ec0cb2a7716615f259cfe9726cd9ca71ba507d6/crates/libcontainer/src/container/container_start.rs#L71-L78

@utam0k
Copy link
Member

utam0k commented Jul 15, 2024

I wonder if we should check other runtimes ...

crun have same problem

@ningmingxiao
I couldn't reproduce it with crun. How did you check it?

$ time sudo crun create -b postStart-bug test
sudo crun create -b postStart-bug test  0.00s user 0.01s system 23% cpu 0.035 total

$ crun -v
crun version 1.15
commit: e6eacaf4034e84185fd8780ac9262bbf57082278
rundir: /run/user/1000/crun
spec: 1.0.0
+SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL

@ningmingxiao
Copy link
Author 

{
    "ociVersion": "1.0.2-dev",
    "process": {
            "terminal": false,
            "user": {
                    "uid": 0,
                    "gid": 0
            },
            "args": [
                    "sh"
            ],
            "env": [
                    "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
                    "TERM=xterm"
            ],
            "cwd": "/",
            "capabilities": {
                    "bounding": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ],
                    "effective": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ],
                    "permitted": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ],
                    "ambient": [
                            "CAP_AUDIT_WRITE",
                            "CAP_KILL",
                            "CAP_NET_BIND_SERVICE"
                    ]
            },
            "rlimits": [
                    {
                            "type": "RLIMIT_NOFILE",
                            "hard": 1024,
                            "soft": 1024
                    }
            ],
            "noNewPrivileges": true
    },
    "root": {
            "path": "rootfs",
            "readonly": false
    },
    "hostname": "runc",
    "mounts": [
            {
                    "destination": "/proc",
                    "type": "proc",
                    "source": "proc"
            },
            {
                    "destination": "/dev",
                    "type": "tmpfs",
                    "source": "tmpfs",
                    "options": [
                            "nosuid",
                            "strictatime",
                            "mode=755",
                            "size=65536k"
                    ]
            },
            {
                    "destination": "/dev/pts",
                    "type": "devpts",
                    "source": "devpts",
                    "options": [
                            "nosuid",
                            "noexec",
                            "newinstance",
                            "ptmxmode=0666",
                            "mode=0620"
                    ]
            },
            {
                    "destination": "/dev/shm",
                    "type": "tmpfs",
                    "source": "shm",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev",
                            "mode=1777",
                            "size=65536k"
                    ]
            },
            {
                    "destination": "/dev/mqueue",
                    "type": "mqueue",
                    "source": "mqueue",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev"
                    ]
            },
            {
                    "destination": "/sys",
                    "type": "sysfs",
                    "source": "sysfs",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev",
                            "ro"
                    ]
            },
            {
                    "destination": "/sys/fs/cgroup",
                    "type": "cgroup",
                    "source": "cgroup",
                    "options": [
                            "nosuid",
                            "noexec",
                            "nodev",
                            "relatime",
                            "ro"
                    ]
            }
    ],
    "hooks": {
            "Poststart": [
                {
                    "path": "/usr/bin/sleep",
                    "args":  ["/usr/bin/sleep","10"]
                }
            ]
    },
    "linux": {
            "resources": {
                    "devices": [
                            {
                                    "allow": false,
                                    "access": "rwm"
                            }
                    ]
            },
            "uidMappings": [
                    {
                            "containerID": 0,
                            "hostID": 1000,
                            "size": 1
                    }
            ],
            "gidMappings": [
                    {
                            "containerID": 0,
                            "hostID": 1000,
                            "size": 1
                    }
            ],
            "namespaces": [
                    {
                            "type": "pid"
                    },
                    {
                            "type": "network"
                    },
                    {
                            "type": "ipc"
                    },
                    {
                            "type": "uts"
                    },
                    {
                            "type": "mount"
                    }
            ],
            "maskedPaths": [
                    "/proc/acpi",
                    "/proc/asound",
                    "/proc/kcore",
                    "/proc/keys",
                    "/proc/latency_stats",
                    "/proc/timer_list",
                    "/proc/timer_stats",
                    "/proc/sched_debug",
                    "/sys/firmware",
                    "/proc/scsi"
            ],
            "readonlyPaths": [
                    "/proc/bus",
                    "/proc/fs",
                    "/proc/irq",
                    "/proc/sys",
                    "/proc/sysrq-trigger"
            ]
    }
}

I recheck crun it is ok.

@lifubang
Copy link
Member

opencontainers/runc#4348 (comment)

Could you please help to check this in crun and youki? Thanks.

@utam0k
Copy link
Member

utam0k commented Jul 19, 2024

opencontainers/runc#4348 (comment)

Could you please help to check this in crun and youki? Thanks.

As far as my investigation, the latest crun and youki couldn't reproduce this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyphar cyphar cyphar requested changes

@kolyshkin kolyshkin kolyshkin requested changes

@AkihiroSuda AkihiroSuda Awaiting requested review from AkihiroSuda AkihiroSuda is a code owner

@crosbymichael crosbymichael Awaiting requested review from crosbymichael crosbymichael is a code owner

@dqminh dqminh Awaiting requested review from dqminh dqminh is a code owner

@giuseppe giuseppe Awaiting requested review from giuseppe giuseppe is a code owner

@hqhq hqhq Awaiting requested review from hqhq hqhq is a code owner

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@thaJeztah thaJeztah Awaiting requested review from thaJeztah thaJeztah is a code owner

@tianon tianon Awaiting requested review from tianon tianon is a code owner

@utam0k utam0k Awaiting requested review from utam0k utam0k is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ningmingxiao @utam0k @cyphar @lifubang @kolyshkin