ceph-daemon: do not pass -it unless it is an interactive shell by liewegas · Pull Request #31181 · ceph/ceph

liewegas · 2019-10-27T17:35:45Z

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs

src/ceph-daemon

tchaikov · 2019-10-28T04:46:32Z

186/186 Test  #14: test_objectstore_memstore.sh ............***Timeout 3600.02 sec

jenkins test make check

tchaikov · 2019-10-28T06:08:58Z

jenkins test make check

liewegas · 2019-10-28T14:06:49Z

the LANG=C was unconditoinally set in the shell_cmd() method. updated to be consistent!

mgfritch · 2019-10-28T17:42:26Z

src/ceph-daemon

+            '--env', 'LANG=C',
+        ]
+    c = get_container(args.fsid, daemon_type, daemon_id,
+                      podman_args=podman_args)


get_container needs a podman_args cmd

diff --git a/src/ceph-daemon b/src/ceph-daemon index 94563545b7..369f55624e 100755 --- a/src/ceph-daemon +++ b/src/ceph-daemon @@ -365,8 +365,7 @@ def get_container_mounts(fsid, daemon_type, daemon_id): return mounts -def get_container(fsid, daemon_type, daemon_id, privileged=False): - podman_args = [] +def get_container(fsid, daemon_type, daemon_id, privileged=False, podman_args=[]): if daemon_type == 'osd' or privileged: podman_args += ['--privileged'] return CephContainer(

Note that in python you should not use an object like a list as a default argument - it does not work as you might expect: https://stackoverflow.com/a/366430

weird. updated! (and ran test_ceph_daemon this time)

mgfritch · 2019-10-29T00:59:33Z

src/ceph-daemon

-        podman_args=['--privileged'],
+        podman_args=podman_args,
        volume_mounts=mounts)
    return subprocess.call(c.shell_cmd(args.command))


Minor fixup for shell cmd when no args are passed -

Suggested change

return subprocess.call(c.shell_cmd(args.command))

return subprocess.call(c.shell_cmd(command))

# ./ceph-daemon shell Traceback (most recent call last): File "./ceph-daemon", line 1683, in <module> r = args.func() File "./ceph-daemon", line 1145, in command_shell return subprocess.call(c.shell_cmd(args.command)) File "./ceph-daemon", line 714, in shell_cmd ] + cmd[1:] IndexError: list index out of rang

sigh, fixed. I want to do something like

diff --git a/qa/standalone/test_ceph_daemon.sh b/qa/standalone/test_ceph_daemon.sh index 69ecb242646..af17925f1c1 100755 --- a/qa/standalone/test_ceph_daemon.sh +++ b/qa/standalone/test_ceph_daemon.sh @@ -133,6 +133,7 @@ $SUDO $CEPH_DAEMON unit --fsid $FSID --name mon.a -- is-enabled ## shell $SUDO $CEPH_DAEMON --image $IMAGE shell -- true $SUDO $CEPH_DAEMON --image $IMAGE shell --fsid $FSID -- test -d /var/log/ceph +echo 'whoami ; exit' | $SUDO $CEPH_DAEMON --image $IMAGE shell ## enter expect_false $SUDO $CEPH_DAEMON enter @@ -142,6 +143,8 @@ $SUDO $CEPH_DAEMON enter --fsid $FSID --name mon.a -- pidof ceph-mon expect_false $SUDO $CEPH_DAEMON enter --fsid $FSID --name mgr.x -- pidof ceph-mon $SUDO $CEPH_DAEMON enter --fsid $FSID --name mgr.x -- pidof ceph-mgr +echo 'pidof ceph-mgr ; exit' | $SUDO $CEPH_DAEMON enter --fsid $FSID --name mgr.x + ## ceph-volume $SUDO $CEPH_DAEMON --image $IMAGE ceph-volume --fsid $FSID -- inventory --format=json \ | jq '.[]'

but i have a feeling this needs to use something like expect since there is maybe no tty?

Or maybe remove the -t option since a pty is not in use?

diff --git a/src/ceph-daemon b/src/ceph-daemon index 949af8ffb2..c698ae152b 100755 --- a/src/ceph-daemon +++ b/src/ceph-daemon @@ -1138,7 +1138,7 @@ def command_shell(): else: command = ['bash'] podman_args += [ - '-it', + '-i', '--env', 'LANG=C', ] c = CephContainer( @@ -1159,7 +1159,7 @@ def command_enter(): else: command = ['bash'] podman_args += [ - '-it', + '-i', '--env', 'LANG=C', ] c = get_container(args.fsid, daemon_type, daemon_id,

Otherwise, we get errors like 2019-10-26T17:29:42.004 INFO:tasks.workunit.client.0.mira109.stderr:+ sudo /usr/sbin/ceph-daemon shell -- ceph -v 2019-10-26T17:29:42.004 INFO:tasks.workunit.client.0.mira109.stderr:+ grep 'ceph version' 2019-10-26T17:29:42.149 INFO:tasks.workunit.client.0.mira109.stderr:the input device is not a TTY Fixes: https://tracker.ceph.com/issues/42499 Signed-off-by: Sage Weil <sage@redhat.com>

* refs/pull/31181/head: ceph-daemon: only pass podman -it if need an interactive shell Reviewed-by: Sebastian Wagner <swagner@suse.com> Reviewed-by: Michael Fritch <mfritch@suse.com>

liewegas added bug-fix orchestrator labels Oct 27, 2019

tchaikov reviewed Oct 28, 2019

View reviewed changes

src/ceph-daemon Outdated Show resolved Hide resolved

liewegas force-pushed the wip-cd-tty branch from 39a7166 to 81edbe5 Compare October 28, 2019 14:06

mgfritch reviewed Oct 28, 2019

View reviewed changes

liewegas force-pushed the wip-cd-tty branch from 81edbe5 to f9dced6 Compare October 28, 2019 18:21

mgfritch requested changes Oct 29, 2019

View reviewed changes

liewegas force-pushed the wip-cd-tty branch from f9dced6 to e7b556f Compare October 29, 2019 02:52

tchaikov added the wip-kefu-testing label Oct 29, 2019

liewegas force-pushed the wip-cd-tty branch 2 times, most recently from 1450af2 to e177212 Compare October 29, 2019 12:29

sebastian-philipp approved these changes Oct 29, 2019

View reviewed changes

liewegas force-pushed the wip-cd-tty branch from e177212 to 8c7dbec Compare October 29, 2019 14:41

mgfritch approved these changes Oct 29, 2019

View reviewed changes

liewegas added a commit that referenced this pull request Oct 29, 2019

Merge PR #31181 into master

3c0df44

* refs/pull/31181/head: ceph-daemon: only pass podman -it if need an interactive shell Reviewed-by: Sebastian Wagner <swagner@suse.com> Reviewed-by: Michael Fritch <mfritch@suse.com>

liewegas merged commit 8c7dbec into ceph:master Oct 29, 2019

liewegas deleted the wip-cd-tty branch October 29, 2019 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ceph-daemon: do not pass -it unless it is an interactive shell#31181

ceph-daemon: do not pass -it unless it is an interactive shell#31181
liewegas merged 1 commit intoceph:masterfrom
liewegas:wip-cd-tty

liewegas commented Oct 27, 2019

Uh oh!

Uh oh!

tchaikov commented Oct 28, 2019

Uh oh!

tchaikov commented Oct 28, 2019

Uh oh!

liewegas commented Oct 28, 2019 via email

Uh oh!

mgfritch Oct 28, 2019

Uh oh!

jdurgin Oct 28, 2019

Uh oh!

liewegas Oct 28, 2019

Uh oh!

mgfritch Oct 29, 2019

Uh oh!

mgfritch Oct 29, 2019 •

edited

Loading

Uh oh!

liewegas Oct 29, 2019

Uh oh!

mgfritch Oct 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	return subprocess.call(c.shell_cmd(args.command))
	return subprocess.call(c.shell_cmd(command))

Conversation

liewegas commented Oct 27, 2019

Checklist

Uh oh!

Uh oh!

tchaikov commented Oct 28, 2019

Uh oh!

tchaikov commented Oct 28, 2019

Uh oh!

liewegas commented Oct 28, 2019 via email

Uh oh!

mgfritch Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

jdurgin Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

liewegas Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

mgfritch Oct 29, 2019

Choose a reason for hiding this comment

Uh oh!

mgfritch Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liewegas Oct 29, 2019

Choose a reason for hiding this comment

Uh oh!

mgfritch Oct 29, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mgfritch Oct 29, 2019 •

edited

Loading