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

Ubuntu environment on WSL is not detected #18

Closed
zhubonan opened this issue Feb 12, 2021 · 13 comments · Fixed by #25
Closed

Ubuntu environment on WSL is not detected #18

zhubonan opened this issue Feb 12, 2021 · 13 comments · Fixed by #25

Comments

@zhubonan
Copy link

Related to aiidateam/aiida-core#4748. The the sudo is not only needed for quicksetup but also other things like verdi profile delete etc.

Because WSL does not have version keys containing Ubuntu, the use of sudo can not be set.

Perhaps there the package can use an environmental variable to force the use of sudo a workaround.

platform.version gives

@ltalirz
Copy link
Member

ltalirz commented Feb 12, 2021

thanks @zhubonan for the report!

I would propose:

  • try_sudo should become a parameter of the constructor
    def __init__(self,
  • Your cases should be taken care of in the DEFAULT (if there is a way to identify WSL from this?)
    If not we may indeed think about alternate ways... I'd like to avoid environment variables (makes this package harder to use; it work "out of the box") but if there is no other way...

Sounds good?

I'd be happy to review and accept a PR on this - otherwise it might take me a bit longer.

It would be even better if there was a way of testing WSL on github actions, but perhaps that's not easily possible... can you check?

@greschd
Copy link
Member

greschd commented Feb 12, 2021

For detecting that we're on WSL, checking for "microsoft" (any capitalization) in the release attribute of uname (and, probably, also checking that we're on Linux) seems like a reasonable (although not bulletproof) way:

In [6]: from platform import uname

In [7]: uname()
Out[7]: uname_result(system='Linux', node='A-DOGRES-082219', release='4.19.128-microsoft-standard', version='#1 SMP Tue Jun 23 12:58:10 UTC 2020', machine='x86_64', processor='x86_64')

Some discussions:
microsoft/WSL#423
microsoft/WSL#4071
Found via this page:
https://www.scivision.dev/python-detect-wsl/

If it's just about detecting Ubuntu, platform.dist() seems to work also on WSL:

In [7]: platform.dist()
Out[7]: ('Ubuntu', '18.04', 'bionic')

@greschd
Copy link
Member

greschd commented Feb 12, 2021

What's the reason for trying sudo only on Ubuntu, by the way? How's the behavior different in other Linuxes?

pgsu/pgsu/__init__.py

Lines 29 to 31 in c75a1f4

# By default, try "sudo" only on Ubuntu
DEFAULT_TRY_SUDO = platform.system(
) == 'Linux' and 'Ubuntu' in platform.version()

@ltalirz
Copy link
Member

ltalirz commented Feb 12, 2021

Thanks a lot for the helpful info @greschd !

What's the reason for trying sudo only on Ubuntu, by the way? How's the behavior different in other Linuxes?

No good reason other than that I did not know the default postgres setup on other linux distros and did not want to scare users with trying sudo unnecessarily.

Happy to add any other distros, where the setup is known to be the same (postgres system user, with paswordless psql into becoming postgres superuser), or different (let me know the details).

@greschd
Copy link
Member

greschd commented Feb 12, 2021

Ah yeah, that makes sense -- the default user etc. can definitely vary between distros.

My naive guess would be that most Ubuntu-based (or maybe even Debian-based) distros would work the same, but it probably makes sense to test before adding them.

In that case, probably just replacing 'Ubuntu' in platform.version() with 'Ubuntu' in platform.dist()[0] should do the trick.

@zhubonan
Copy link
Author

Thanks for looking into this, for me the platform.dist() give this:

('debian', 'buster/sid', '')

so no mentioning of Ubuntu. The same goes for both WSL1 and WSL2. I am on 18.04 though, it may very different versions? @greschd

The other complication is WSL is not always Ubuntu, other Linux distributions can be run as well like openSUSU....

Perhaps the check can be simply whether this is a sudo in the path and if the system is Linux?

>>> from subprocess import check_output
>>> check_output(['which', 'sudo'])
b'/usr/bin/sudo\n'
>>>

@zhubonan
Copy link
Author

platform.dist()

Maybe it depends on python versions, mine on 3.7 says the function is deprecated since 3.5 and it is gone in 3.8.

>>> platform.dist()
__main__:1: DeprecationWarning: dist() and linux_distribution() functions are deprecated in Python 3.5
('debian', 'buster/sid', '')

@greschd
Copy link
Member

greschd commented Feb 16, 2021

@zhubonan hmm, I get the same behavior when using the conda Python. Seems like it depends on how Python was compiled.. maybe the command was deprecated because it ended up being unreliable.

There seem to be a few different ways of determining the distro, not sure which is the most commonly available: https://linuxhandbook.com/check-linux-version/

So, in general, I think this is less of a WSL problem and more of a Linux distro problem. As you mentioned, different distros or even non-Linux OSes can run on WSL; our problem here comes mostly from that. I think we should overall rely more on how the system is set up, and less on hard-coding "known good" distro names.

@ltalirz
Copy link
Member

ltalirz commented Feb 16, 2021

I think we should overall rely more on how the system is set up, and less on hard-coding "known good" distro names.

Open to suggestions on how to do this

@zhubonan
Copy link
Author

@greschd Ahh I see, yes I am using python from anaconda!
@ltalirz How about we test if there is a sudo in the $PATH, and if so we use it? I don't see any drawback of trying to run sudo when other ways fail, as long as the user is given clear explanation that AiiDA is asking the password for setting up the postgres database.

@ltalirz
Copy link
Member

ltalirz commented Mar 1, 2021

How about we test if there is a sudo in the $PATH

This is already the case

if _sudo_exists():

I don't see any drawback of trying to run sudo when other ways fail, as long as the user is given clear explanation that AiiDA is asking the password for setting up the postgres database.

Some users get (understandably) rather suspicious if any script asks for a sudo password...
on the other hand, we're already doing this for ubuntu, so it's not really something new.

Perhaps the best heuristic to decide whether the sudo approach should be tried is whether the postgres system user exists:

import pwd

try:
    pwd.getpwnam('postgres')
except KeyError:
    print('User postgres does not exist.')

My question then is:

  1. what is the postgresql setting in your WSL setup?
    Do you have a postgres system user with connection setting peer for local?
    Can you paste the postgresql server configuration?
    Is there a way for us to test it on CI?

  2. What was the error message when quicksetup didn't work?
    Can it be improved?
    There will always be cases for which autotedection of the postgres setup doesn't work, but one can still use verdi quicksetup (you can specify just the variables you need to provide). We should make sure that this is clearly spelled out.

@zhubonan
Copy link
Author

zhubonan commented Mar 9, 2021

Thanks for looking into this. I think it makes sense to use postgres user as a heuristic. As long as the purposed of asking for sudo password is communicated to the user I think it would be fine.

what is the postgresql setting in your WSL setup?

I am not quite sure what you refers to - it should be identical to the Ubuntu setup. I just install it with apt.

Do you have a postgres system user with connection setting peer for local?

Yes, there is a postgres users added when the package is installed by apt.

Can you paste the postgresql server configuration?

Please see the attached acharive - I just zipped the whole /etc/postgresql/10/main/ folder.
postgresql.tar.gz

Is there a way for us to test it on CI?

I am not sure, but any update-to-date Windows 10 should be able to run WSL1. WSL2 requires a HyperV so I am not sure if that is possible.

What was the error message when quicksetup didn't work?
Can it be improved?

Please provide PostgreSQL connection info:
postgres host []: localhost
postgres port [5432]:
postgres super user [postgres]:
database [template1]:
postgres password of postgres []:
Unable to autodetect postgres setup.
Critical: failed to determine the PostgreSQL setup

with pgsu == 0.1.0 and aiida-core==1.5.2, there is no password for postgres users so I left it blank.

@ltalirz
Copy link
Member

ltalirz commented Mar 9, 2021

Thanks @zhubonan !

For the record, the postgresql configuration on WSL is indeed identical to Ubuntu, i.e. there is a postgresql system user with peer authentication. Excerpt from pg_hba.conf:

# Database administrative login by Unix domain socket
local   all             postgres                                peer

# TYPE  DATABASE        USER            ADDRESS                 METHOD

# "local" is for Unix domain socket connections only
local   all             all                                     peer
# IPv4 local connections:
host    all             all             127.0.0.1/32            md5
# IPv6 local connections:
host    all             all             ::1/128                 md5

Note to self: While peer authentication is, in principle, available for all unix users, a corresponding postgresql database user is only created for the postgres system user - i.e. when you try psql as a random system user, you get

ubuntu@qmobile:~$ psql
psql: FATAL:  role "ubuntu" does not exist

Since the available method for host-based login is md5 (password), I think that indeed the only way to connect as the postgresql superuser is to become the postgres system user.

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 a pull request may close this issue.

3 participants