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

[Docker] Config.isDocker is not right in some Docker version / Linux distribution #84

Closed
4 tasks done
JasLin opened this issue Nov 9, 2016 · 9 comments
Closed
4 tasks done
Labels

Comments

@JasLin
Copy link
Contributor

JasLin commented Nov 9, 2016

Updated by @zixia

There's so many difference between Docker container and the Linux distribution.

It seems that we have to list all the docker /proc/self/cgroup files by ourselves...

  1. Ubuntu 16.04
    1:name=systemd:/init.scope

  2. Docker version 1.9.1-circleci-cp-workaround, build 517b158(LXC Container in CircleCI)
    2:cpuset:/user/1001.user/13034.session/lxc/box2176/lxc/5cd9c2d7f4a7be56db6b79afdf65376bc882c0fa98dd187cda3bf61082041c09

  3. Debian 8@VPS of Google Cloud:
    1:name=systemd:/system.slice/ssh.service

  4. Docker version 1.12.1, build 23cf638
    1:name=systemd:/docker/3dcbb75a8e932d16a2443afadbe6f02ff242d2665cc43e3f2c7926ae8fdd0633

See Also:

THE FOLLOWING IS THE ORIGNAL ISSUE POST


Run npm run doctor or wechaty run doctor(for docker user), paste output here

one ubuntu 16.04 (not a docker):

> [email protected] doctor /home/user1/apps/wechaty
> ts-node bin/doctor


#### Wechaty Doctor

1. Wechaty version: #git[4c5f713 fix & enhance docker entrypoint help & command parse, with unit tests]
2. Linux x64 version 4.4.0-45-generic memory 378/3919 MB
3. Docker: true
4. Node version: v7.0.0

Expected behavior

3. Docker: true

this expected to be false

Actual behavior

function isWechatyDocker() {
  /**
   * Continuous Integration System
   */
  const isCi = require('is-ci')
  if (isCi) {
    return false
  }

  /**
   * Cloud9 IDE
   */
  const c9 = Object.keys(process.env)
                  .filter(k => /^C9_/.test(k))
                  .length
  if (c9 > 7 && process.env['C9_PORT']) {
    return false
  }

  const cgroup = '/proc/1/cgroup'
  try       { fs.statSync(cgroup).isFile() }
  catch (e) { return false }

  // http://stackoverflow.com/a/20624315/1123955
  const line = execSync(`sort -n ${cgroup} 2>/dev/null | head -1`)
                .toString()
                .replace(/\n$/, '')

  // instead of `/`, docker will end with a container id
  if (/\/$/.test(line)) {
    return false
  }

  return true
}

but on ubuntu 16.04 ,wen run cat /proc/1/cgroup, it's returns not the same as the others version linux.

~$ cat /proc/1/cgroup
11:cpuset:/
10:perf_event:/
9:memory:/init.scope
8:pids:/init.scope
7:hugetlb:/
6:cpu,cpuacct:/init.scope
5:net_cls,net_prio:/
4:blkio:/init.scope
3:devices:/init.scope
2:freezer:/
1:name=systemd:/init.scope
~$ sort -n /proc/1/cgroup 2>/dev/null | head -1
1:name=systemd:/init.scope

it's no end with '/' , and method return true finnally

Steps to reproduce the behavior (and fixes, if any)

Paste the full output logs here with WECHATY_LOG=silly set

@huan
Copy link
Member

huan commented Nov 9, 2016

Hmm... the isDocker() is a dirty & quick function...

We need to rewrite it. Do you have any good idea?

@JasLin
Copy link
Contributor Author

JasLin commented Nov 9, 2016

:( i have no idea about this, i am googling it, docker upgrade day by day, i seems there is no regular way help to do this.

i had checked cat /proc/1/cgroup in ubuntu 14.04, and 14.06, it has difference string pattern.
but in docker ,i seems cat /proc/1/group always contain keyword 'docker' ?

@JasLin
Copy link
Contributor Author

JasLin commented Nov 9, 2016

@huan
Copy link
Member

huan commented Nov 10, 2016

@JasLin could you please confirm that the latest version on master resolve this issue? thanks

@huan huan added the bug label Nov 10, 2016
@huan huan changed the title Config.isWechatyDocker is not compatible with ubuntu 16.04 [Docker] Config.isDocker is not right in some Docker version / Linux distribution Nov 10, 2016
@JasLin
Copy link
Contributor Author

JasLin commented Nov 10, 2016

@zixia the issue has gone.
run npm run doctor with the latest code on ubuntu 16.04 and 14.04, it got the right result .

@huan
Copy link
Member

huan commented Nov 10, 2016

great.

Could you help to paste the output of following two commands under ubuntu 16 for me? Because I need them to be set as test fixture:

  1. cat /proc/self/cgroup
  2. cat /proc/1/cgroup

appreciated.

@huan
Copy link
Member

huan commented Nov 10, 2016

ah, not necessary anymore.

I found this npm module: https://www.npmjs.com/package/is-docker

it's a pity, haha. because I'm just writing my own one... and had already finished here:
https://github.com/wechaty/is-docker

It seems good, let's use it.

@JasLin
Copy link
Contributor Author

JasLin commented Nov 10, 2016

@zixia

it's pretty!
have tested this new patch ,it works as well

@huan
Copy link
Member

huan commented Nov 10, 2016

ok, then this issue closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants