Skip to content

mgr/dashboard: vstart: Fix /dev/tty No such device or address #31195

Merged
batrick merged 1 commit intoceph:masterfrom
votdev:issue_42487_dev_tty
Oct 30, 2019
Merged

mgr/dashboard: vstart: Fix /dev/tty No such device or address #31195
batrick merged 1 commit intoceph:masterfrom
votdev:issue_42487_dev_tty

Conversation

@votdev
Copy link
Member

@votdev votdev commented Oct 28, 2019 •
edited
Loading

Running on Jenkins slave or as a daemon there will be no writable console.

Fixes: https://tracker.ceph.com/issues/42487

Signed-off-by: Volker Theile vtheile@suse.com

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

@votdev votdev force-pushed the issue_42487_dev_tty branch from 7c21f4b to b74cef6 Compare October 28, 2019 13:12
@votdev
Copy link
Member Author

votdev commented Oct 28, 2019

@callithea Can you run a Dashboard E2E test including this patch to validate if it fixes the issue?

@callithea
Copy link
Member

callithea commented Oct 29, 2019

jenkins test dashboard

@callithea callithea changed the title vstart: Check if /dev/tty is a character device dashboard: vstart: Check if /dev/tty is a character device Oct 29, 2019
@callithea
Copy link
Member

callithea commented Oct 29, 2019

jenkins test dashboard

@callithea callithea changed the title dashboard: vstart: Check if /dev/tty is a character device vstart: Check if /dev/tty is a character device Oct 29, 2019
@votdev votdev changed the title vstart: Check if /dev/tty is a character device mgr/dashboard: vstart: Check if /dev/tty is a character device Oct 29, 2019
@votdev votdev force-pushed the issue_42487_dev_tty branch 3 times, most recently from d2f911d to 18777c0 Compare October 29, 2019 10:05
@votdev votdev changed the title mgr/dashboard: vstart: Check if /dev/tty is a character device mgr/dashboard: vstart: Fix /dev/tty No such device or address Oct 29, 2019
@votdev votdev added the DNM label Oct 29, 2019
@votdev votdev changed the title mgr/dashboard: vstart: Fix /dev/tty No such device or address [WIP] mgr/dashboard: vstart: Fix /dev/tty No such device or address Oct 29, 2019
@votdev votdev force-pushed the issue_42487_dev_tty branch 5 times, most recently from 19a5df0 to f4a3757 Compare October 29, 2019 15:14
@votdev votdev changed the title [WIP] mgr/dashboard: vstart: Fix /dev/tty No such device or address mgr/dashboard: vstart: Fix /dev/tty No such device or address Oct 29, 2019
@votdev votdev removed the DNM label Oct 29, 2019
@votdev votdev force-pushed the issue_42487_dev_tty branch from f4a3757 to ef51615 Compare October 29, 2019 23:20
@tchaikov tchaikov mentioned this pull request Oct 30, 2019
Merged
epuertat
epuertat reviewed Oct 30, 2019
View reviewed changes
Copy link
Member

@epuertat epuertat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I tried a different approach (just fixing the checking and leaving everything else unmodified). I hit this issue when running vstart in a non-interactive container environment.

epuertat
epuertat reviewed Oct 30, 2019
View reviewed changes
src/vstart.sh Outdated
debug() {
if [ -w /dev/tty -a ! -t 2 ]; then
"$@" | tee /dev/tty >&2
if [ -w /dev/stderr -a ! -t 2 ]; then
Copy link
Member

@epuertat epuertat Oct 30, 2019

Choose a reason for hiding this comment

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

I tried again and now it really works (although I'm not sure if the resulting behaviour is the same as what @batrick expected when he made this change):

Suggested change
if [ -w /dev/stderr -a ! -t 2 ]; then
if [ -w /dev/tty -a -t 1 ]; then

Copy link
Member Author

@votdev votdev Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

You're right, the ! -t 2 does not make sense when using /dev/stderr. Our problem is that Jenkins does not have a /dev/tty device when our tests scripts are executed.

Don't know whether your changes let the thing work as wanted by @batrick. Maybe he needs to adapt the code.

Copy link
Member

@epuertat epuertat Oct 30, 2019

Choose a reason for hiding this comment

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

The -t 1 basically skips tty in non-interactive shells (my suggestion was to change just that, and leave the remaining tty stuff).

@epuertat epuertat mentioned this pull request Oct 30, 2019
Merged
3 tasks
This was referenced Oct 30, 2019
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@votdev
Copy link
Member Author

votdev commented Oct 30, 2019

@batrick Can you please review this PR. This issue is blocking the Dashboard team from running E2E tests and merging PR's.

batrick
batrick requested changes Oct 30, 2019
View reviewed changes
src/vstart.sh Outdated
"$@" | tee /dev/tty >&2
if [ -w /dev/stderr -a ! -t 2 ]; then
"$@" | tee /dev/stderr
else
Copy link
Member

@batrick batrick Oct 30, 2019

Choose a reason for hiding this comment

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

I'm not sure what jenkins is doing here to make /dev/tty visible but not writeable. In any case, to unblock your testing, just make debug() unconditionally call "$@" > &2

Copy link
Member Author

@votdev votdev Oct 30, 2019

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@epuertat epuertat Oct 30, 2019

Choose a reason for hiding this comment

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

Not only Jenkins. With bash running in a detached container (docker daemon) it fails in the same way.

@votdev
vstart: Fix /dev/tty No such device or address
4385ce4 
Running on Jenkins slave or as a daemon there will be no writable console.

Fixes: https://tracker.ceph.com/issues/42487

Signed-off-by: Volker Theile <vtheile@suse.com>
@votdev votdev force-pushed the issue_42487_dev_tty branch from ef51615 to 4385ce4 Compare October 30, 2019 14:52
batrick added a commit that referenced this pull request Oct 30, 2019
@batrick
Merge PR #31195 into master
25f4a03 
* refs/pull/31195/head:
	vstart: Fix /dev/tty No such device or address

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick batrick merged commit 4385ce4 into ceph:master Oct 30, 2019
@callithea callithea mentioned this pull request Oct 31, 2019
Closed
@votdev votdev deleted the issue_42487_dev_tty branch October 31, 2019 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@epuertat epuertat epuertat left review comments

@batrick batrick batrick approved these changes

@LenzGr LenzGr Awaiting requested review from LenzGr

@callithea callithea Awaiting requested review from callithea

Assignees

No one assigned

Labels

bug-fix dashboard needs-qa wip-kefu-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@votdev @callithea @batrick @epuertat @tchaikov