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.

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

set masterfd_stdout before regsitering ctrl_cb #71

Merged
merged 1 commit into from
Sep 13, 2019

Conversation

haircommander
Copy link
Collaborator

exec with conmon ran into issues because the manager attempted to resize the window before stdout was configured. Thus, we got the ctrl_cb from the caller, but had no where to send the ioctl.

Fix this by registering the callback after we set stdout.

Signed-off-by: Peter Hunt [email protected]

exec with conmon ran into issues because the manager attempted to resize the window before stdout was configured. Thus, we got the ctrl_cb from the caller, but had no where to send the ioctl.

Fix this by registering the callback after we set stdout.

Signed-off-by: Peter Hunt <[email protected]>
@haircommander
Copy link
Collaborator Author

partial fix for containers/podman#3903

@rhatdan
Copy link
Member

rhatdan commented Sep 12, 2019

LGTM
@vrothberg @giuseppe @mheon PTAL

* if we didn't set it here, we'd risk attempting to run ioctl on
* a negative fd, and fail to resize the window */
g_unix_fd_add(terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL);

Copy link
Member

Choose a reason for hiding this comment

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

🥇 on an excellent comment

@TomSweeneyRedHat
Copy link
Member

LGTM assuming happy tests

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@vrothberg vrothberg merged commit 35437e4 into containers:master Sep 13, 2019
apyrgio pushed a commit to freedomofpress/maint-dangerzone-conmon that referenced this pull request Jan 24, 2024
Squashed commit of the following:

commit 60b42f2
Author: Peter Hunt <[email protected]>
Date:   Mon Jan 6 10:52:09 2020 -0500

    bump to version 2.0.9

    Signed-off-by: Peter Hunt <[email protected]>

commit 1560392
Author: Mrunal Patel <[email protected]>
Date:   Sat Jan 4 13:04:58 2020 -0800

    Add TimedOutMessage to config to share with go code

    Signed-off-by: Mrunal Patel <[email protected]>

commit b17d81b
Author: Riccardo Schirone <[email protected]>
Date:   Fri Dec 20 17:38:42 2019 +0100

    Fixes format string to limit the size of the string to 10 characters

    %10s just ensures that when printing, at least 10 characters are
    written. However, in this case we probably want *at most* 10 characters
    to be written.

    Signed-off-by: Riccardo Schirone <[email protected]>

commit 3a11e26
Author: Riccardo Schirone <[email protected]>
Date:   Fri Dec 20 18:20:28 2019 +0100

    g_file_{get,set}_contents return a gboolean to indicate failure, use it

    Signed-off-by: Riccardo Schirone <[email protected]>

commit c2e2e67
Merge: fe2ebfd 0c66b66
Author: Daniel J Walsh <[email protected]>
Date:   Fri Dec 13 14:35:09 2019 -0500

    Merge pull request containers#105 from haircommander/bump-2.0.8

    Bump 2.0.8

commit 0c66b66
Author: Peter Hunt <[email protected]>
Date:   Fri Dec 13 14:22:13 2019 -0500

    bump to 2.0.9-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit 5ca7df2
Author: Peter Hunt <[email protected]>
Date:   Fri Dec 13 14:22:00 2019 -0500

    bump to 2.0.8

    Signed-off-by: Peter Hunt <[email protected]>

commit fe2ebfd
Merge: c8f7443 5bbd452
Author: Giuseppe Scrivano <[email protected]>
Date:   Fri Dec 13 20:20:44 2019 +0100

    Merge pull request containers#104 from haircommander/revert-oom

    Revert "conmon: check oom counter on cgroup v1"

commit 5bbd452
Author: Peter Hunt <[email protected]>
Date:   Fri Dec 13 12:09:27 2019 -0500

    Revert "conmon: check oom counter on cgroup v1"

    This reverts commit 4100fb2.

    We found issues where the reading of the memory control file was racing with the cgroup manager's write. while we want to prevent false positives, not having false negatives is more important. Revert this commit until we find a more complete solution.

    Signed-off-by: Peter Hunt <[email protected]>

commit c8f7443
Merge: 036ff29 7d8faa7
Author: Mrunal Patel <[email protected]>
Date:   Thu Dec 12 15:13:27 2019 -0800

    Merge pull request containers#102 from haircommander/cgroup_v2_persist

    persist oom files on cgroup v2

commit 7d8faa7
Author: Peter Hunt <[email protected]>
Date:   Thu Dec 12 11:04:52 2019 -0500

    persist oom files on cgroup v2

    Signed-off-by: Peter Hunt <[email protected]>

commit 036ff29
Author: Peter Hunt <[email protected]>
Date:   Thu Dec 12 13:56:07 2019 -0500

    bump to v2.0.8-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit d532cae
Author: Peter Hunt <[email protected]>
Date:   Thu Dec 12 13:55:53 2019 -0500

    bump to v2.0.7

    Signed-off-by: Peter Hunt <[email protected]>

commit faccd40
Merge: 0760156 7d4c6be
Author: Mrunal Patel <[email protected]>
Date:   Thu Dec 12 10:49:47 2019 -0800

    Merge pull request containers#100 from giuseppe/splice

    conmon: use splice(2) to copy from stdin

commit 0760156
Merge: 4100fb2 dc23660
Author: Mrunal Patel <[email protected]>
Date:   Thu Dec 12 10:48:58 2019 -0800

    Merge pull request containers#101 from haircommander/bump-2.0.7

    Bump 2.0.7

commit dc23660
Author: Peter Hunt <[email protected]>
Date:   Thu Dec 12 12:59:36 2019 -0500

    bump to version 2.0.8-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit c35049c
Author: Peter Hunt <[email protected]>
Date:   Thu Dec 12 12:59:18 2019 -0500

    bump to version 2.0.7

    Signed-off-by: Peter Hunt <[email protected]>

commit 7d4c6be
Author: Giuseppe Scrivano <[email protected]>
Date:   Thu Dec 12 15:24:43 2019 +0100

    conmon: use splice(2) to copy from stdin

    the first advantage is that it is much faster as data is not copied
    from the kernel to the userspace and the back to the kernel, and the
    second advantage is that we don't block on writ'ing on the fd all the
    data we've buffered.

    Closes: containers#99

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit 4100fb2
Author: Giuseppe Scrivano <[email protected]>
Date:   Thu Dec 12 12:14:05 2019 +0100

    conmon: check oom counter on cgroup v1

    when running on cgroup v1, also check whether the OOM counter is > 0
    before writing the "oom" file.

    Previously, conmon was detecting an OOM every time an event fd was
    triggered.

    Unfortunately that is not enough, as an event on the eventfd is also
    triggered when the cgroup is deleted.  That could cause a race
    condition where conmon sees the event from the cgroup deletion before
    the SIGCHLD+SIGUSR1.  That could happen for example when systemd
    CollectMode is set for failed units.

    Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1782601

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit 95ed45a
Merge: 8ba9575 82ef9c7
Author: Giuseppe Scrivano <[email protected]>
Date:   Thu Dec 12 12:21:59 2019 +0100

    Merge pull request containers#97 from mrunalp/persist_oom_file

    Persist the oom file when --persist-dir is specified

commit 82ef9c7
Author: Mrunal Patel <[email protected]>
Date:   Wed Dec 11 14:49:23 2019 -0800

    Persist the oom file when --persist-dir is specified

    Signed-off-by: Mrunal Patel <[email protected]>

commit 8ba9575
Author: Peter Hunt <[email protected]>
Date:   Wed Dec 11 09:04:43 2019 -0500

    bump to v2.0.7-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit 29c3367
Author: Peter Hunt <[email protected]>
Date:   Wed Dec 11 09:04:25 2019 -0500

    bump to v2.0.6

    Signed-off-by: Peter Hunt <[email protected]>

commit ba14d9c
Author: Giuseppe Scrivano <[email protected]>
Date:   Wed Dec 11 14:05:44 2019 +0100

    conmon: kill the process group on timeout

    when the timeout is expired, instead of killing the process, make sure
    to kill the entire process group.

    It is not safe as using pid cgroup or a pid namespace, which we cannot
    use as the script must run inside of the container pid namespace, but
    it addresses most common cases like bash forking a new process.

    Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1774184

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit bc9e976
Author: Peter Hunt <[email protected]>
Date:   Tue Dec 10 12:48:56 2019 -0500

    bump to v2.0.6-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit f91c170
Author: Peter Hunt <[email protected]>
Date:   Tue Dec 10 12:48:39 2019 -0500

    bump to v2.0.5

    Signed-off-by: Peter Hunt <[email protected]>

commit c792503
Author: Mrunal Patel <[email protected]>
Date:   Mon Dec 9 11:23:02 2019 -0800

    Write exit file to container bundle directory

    Write the exit file under the container bundle directory so
    it is available even when we restart the node. Also, keeping
    it scoped there makes the cleanup easier.

    Signed-off-by: Mrunal Patel <[email protected]>

commit fd5ac47
Merge: df8c6aa c6835ed
Author: Daniel J Walsh <[email protected]>
Date:   Mon Dec 9 12:27:55 2019 -0500

    Merge pull request containers#91 from haircommander/version-2.0.4

    bump to version 2.0.4

commit c6835ed
Author: Peter Hunt <[email protected]>
Date:   Mon Dec 9 11:04:44 2019 -0500

    bump to version 2.0.5-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit d930e38
Author: Peter Hunt <[email protected]>
Date:   Mon Dec 9 11:04:10 2019 -0500

    bump to version 2.0.4

    Signed-off-by: Peter Hunt <[email protected]>

commit df8c6aa
Author: Giuseppe Scrivano <[email protected]>
Date:   Mon Dec 2 16:41:03 2019 +0100

    conmon: fix tight loop on OOM

    inotify fails with EINVAL when trying to read an event with a too
    small buffer.

    Increase the size to "sizeof(struct inotify_event) + NAME_MAX + 1",
    that is enough to read at least one event.

    Closes: containers#87

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit 42bce45
Merge: f6d23b5 5ae9942
Author: Daniel J Walsh <[email protected]>
Date:   Fri Nov 29 06:34:52 2019 -0500

    Merge pull request containers#89 from DaanDeMeyer/reduce-error-impact

    Initialize the endpoint for attach before calling fork.

commit 5ae9942
Author: Daan De Meyer <[email protected]>
Date:   Wed Nov 27 21:24:16 2019 +0100

    Initialize the endpoint for attach before calling fork.

    This way, when an error occurs in the attach endpoint logic, conmon can
    exit immediately without having to clean up the child process.

commit f6d23b5
Merge: 098fcce 951052b
Author: Daniel J Walsh <[email protected]>
Date:   Mon Nov 11 15:48:46 2019 -0500

    Merge pull request containers#83 from haircommander/bump-2.0.4

    Bump to 2.0.3

commit 951052b
Author: Peter Hunt <[email protected]>
Date:   Mon Nov 11 10:16:39 2019 -0500

    version 2.0.4-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit eb5fa88
Author: Peter Hunt <[email protected]>
Date:   Mon Nov 11 10:16:23 2019 -0500

    version 2.0.3

    Signed-off-by: Peter Hunt <[email protected]>

commit 098fcce
Merge: 002da25 496e8e4
Author: Giuseppe Scrivano <[email protected]>
Date:   Mon Nov 11 16:09:42 2019 +0100

    Merge pull request containers#82 from haircommander/try_other_pipe

     Handle window resize events in a different fifo

commit 496e8e4
Author: Peter Hunt <[email protected]>
Date:   Thu Nov 7 22:01:48 2019 -0500

    config: include the two ctrl events

    Rather than having a comment saying "this should stay in sync", let's define a variable and import it there, so we're guarenteed to stay in sync

    Signed-off-by: Peter Hunt <[email protected]>

commit 1ed4f02
Author: Peter Hunt <[email protected]>
Date:   Tue Nov 5 15:25:00 2019 -0500

    Handle window resize events in a different fifo

    The terminal_ctrl_fd was being overloaded with events. CRI-O wants to reopen the log files at any time, and podman wants to not drop window resize events on exec.

    But we can have both.

    Introduce winsz_fd_*, a special fifo pair for window resize events. The terminal_ctrl passes window resize events to the write side, and the read side can process them (after masterfd_stdout is initialized, so we have somewhere to write to and we don't drop events).

    Signed-off-by: Peter Hunt <[email protected]>

commit 002da25
Merge: 19a1d5c 8d66f44
Author: Daniel J Walsh <[email protected]>
Date:   Tue Nov 5 09:13:28 2019 -0500

    Merge pull request containers#81 from giuseppe/opt-tag

    src: add new option --log-tag

commit 8d66f44
Author: Giuseppe Scrivano <[email protected]>
Date:   Tue Nov 5 09:59:07 2019 +0100

    src: add new option --log-tag

    it specifies an additional tag to use for logging messages.

    When using journald, it adds a tag "CONTAINER_TAG".

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit 19a1d5c
Author: Sascha Grunert <[email protected]>
Date:   Tue Oct 15 13:08:12 2019 +0200

    Add 'trace' log level support

    The trace log level is currently not supported by conmon, but seems to be
    a valid level for container runtimes like CRI-O and podman. This results
    that the container currently won't start if the log level is set to
    'trace'. Beside this, we now case insensitively compare the log level
    string to make it more fail safe.

    Signed-off-by: Sascha Grunert <[email protected]>

commit bc758d8
Merge: e37aa48 cb4d0e3
Author: Giuseppe Scrivano <[email protected]>
Date:   Fri Oct 11 08:23:02 2019 +0200

    Merge pull request containers#77 from haircommander/v2.0.2-bump

    bump to 2.0.2

commit cb4d0e3
Author: Peter Hunt <[email protected]>
Date:   Thu Oct 10 08:42:17 2019 -0400

    version 2.0.3-dev

    Signed-off-by: Peter Hunt <[email protected]>

commit 65fe022
Author: Peter Hunt <[email protected]>
Date:   Thu Oct 10 08:41:45 2019 -0400

    version 2.0.2

    Signed-off-by: Peter Hunt <[email protected]>

commit e37aa48
Merge: 0e888a9 067c0a5
Author: Daniel J Walsh <[email protected]>
Date:   Thu Oct 10 06:42:21 2019 -0400

    Merge pull request containers#76 from giuseppe/fix-podman-hang

    conmon: close the sync pipe before exit command

commit 067c0a5
Author: Giuseppe Scrivano <[email protected]>
Date:   Wed Oct 9 17:41:06 2019 +0200

    conmon: close the sync pipe before exit command

    before calling the exit command be sure the sync pipe is closed so
    that the waiting process can stop waiting.

    It solves an issue with Podman where the cleanup process could hang
    since the container is already locked by the podman process waiting
    on the sync pipe.  The reproducer here:

    podman --runtime /bin/false run --rm  alpine true

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit 0e888a9
Author: Sascha Grunert <[email protected]>
Date:   Mon Oct 7 11:45:45 2019 +0200

    Add static binary build to Cirrus CI

    Signed-off-by: Sascha Grunert <[email protected]>

commit 422ce21
Merge: 7286971 d7c90bd
Author: Daniel J Walsh <[email protected]>
Date:   Wed Sep 18 07:34:52 2019 -0400

    Merge pull request containers#74 from haircommander/conn_sock_config

    Add CONN_SOCK_BUF_SIZE to config

commit d7c90bd
Author: Peter Hunt <[email protected]>
Date:   Tue Sep 17 15:04:35 2019 -0400

    Add CONN_SOCK_BUF_SIZE to config

    instead of having a note that says to sync the value

    Signed-off-by: Peter Hunt <[email protected]>

commit 7286971
Author: Valentin Rothberg <[email protected]>
Date:   Fri Sep 13 16:20:48 2019 +0200

    bump to v2.0.2-dev

    Signed-off-by: Valentin Rothberg <[email protected]>

commit 4dc8bcf
Author: Valentin Rothberg <[email protected]>
Date:   Fri Sep 13 16:20:28 2019 +0200

    v2.0.1

    * set masterfd_stdout before regsitering ctrl_cb
    * Add static & containerized targets to Makefile.
    * Makefile: don't fail if clean is called without a build
    * Fix compilation with clang
    * conmon: propagate error code from the container

    Signed-off-by: Valentin Rothberg <[email protected]>

commit 35437e4
Merge: 4d36cb6 9e1e1ff
Author: Valentin Rothberg <[email protected]>
Date:   Fri Sep 13 10:11:09 2019 +0200

    Merge pull request containers#71 from haircommander/stdout-wait-ctrl

    set masterfd_stdout before regsitering ctrl_cb

commit 9e1e1ff
Author: Peter Hunt <[email protected]>
Date:   Thu Sep 12 15:59:20 2019 -0400

    set masterfd_stdout before regsitering ctrl_cb

    exec with conmon ran into issues because the manager attempted to resize the window before stdout was configured. Thus, we got the ctrl_cb from the caller, but had no where to send the ioctl.

    Fix this by registering the callback after we set stdout.

    Signed-off-by: Peter Hunt <[email protected]>

commit 4d36cb6
Merge: fac48d0 58ce137
Author: Daniel J Walsh <[email protected]>
Date:   Wed Sep 4 16:06:00 2019 -0400

    Merge pull request containers#69 from mgoltzsche/static-build

    Add static & dockerized targets to Makefile.

commit 58ce137
Author: Max Goltzsche <[email protected]>
Date:   Tue Sep 3 21:57:38 2019 +0200

    Add static & containerized targets to Makefile.

    Signed-off-by: Max Goltzsche <[email protected]>

commit fac48d0
Merge: 8a56404 6ef63df
Author: Daniel J Walsh <[email protected]>
Date:   Sat Aug 31 06:25:52 2019 -0400

    Merge pull request containers#68 from agners/master

    Makefile: don't fail if clean is called without a build

commit 6ef63df
Author: Stefan Agner <[email protected]>
Date:   Fri Aug 30 17:49:22 2019 +0200

    Makefile: don't fail if clean is called without a build

    Do not fail when calling the clean target without building first.

    Signed-off-by: Stefan Agner <[email protected]>

commit 8a56404
Merge: af8e5a6 6d4f502
Author: Giuseppe Scrivano <[email protected]>
Date:   Tue Aug 20 16:33:23 2019 +0200

    Merge pull request containers#66 from openSUSE/clang-fix

    Fix compilation with clang

commit 6d4f502
Author: Sascha Grunert <[email protected]>
Date:   Tue Aug 20 15:27:38 2019 +0200

    Fix compilation with clang

    This fixes the missing field initializers error when compiling with
    clang:

    ```
    src/conmon.c:119:7: error: missing field 'short_name' initializer
            {NULL}};
                 ^
    ```

    Signed-off-by: Sascha Grunert <[email protected]>

commit af8e5a6
Merge: a1cdfff 74da3fb
Author: Daniel J Walsh <[email protected]>
Date:   Mon Aug 19 10:25:01 2019 -0400

    Merge pull request containers#65 from giuseppe/conmon-return-exit-code-container

    conmon: propagate error code from the container

commit 74da3fb
Author: Giuseppe Scrivano <[email protected]>
Date:   Mon Aug 19 13:34:13 2019 +0200

    conmon: propagate error code from the container

    at exit propagate the return code from the watched container.  This is
    useful when running in systemd so it can properly detect a failure and
    restart the container if necessary.

    Signed-off-by: Giuseppe Scrivano <[email protected]>

commit a1cdfff
Author: Peter Hunt <[email protected]>
Date:   Fri Aug 9 09:50:56 2019 -0400

    v2.0.1-dev

    Signed-off-by: Peter Hunt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants