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

Can't run sudo takyctl systemd --user bluetack #79

Open
vctibor opened this issue Jan 27, 2023 · 12 comments
Open

Can't run sudo takyctl systemd --user bluetack #79

vctibor opened this issue Jan 27, 2023 · 12 comments
Assignees
Labels
bug Something isn't working fix-committed

Comments

@vctibor
Copy link

vctibor commented Jan 27, 2023

Attempts to run sudo takyctl systemd --user bluetack end with following error:

Building systemd services
systemd failed: expected str, bytes or os.PathLike object, not NoneType
Unhandled exception:
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/taky/cli/__main__.py", line 51, in main
    ret = commands[args.command](args)
  File "/usr/local/lib/python3.10/dist-packages/taky/cli/systemd_cmd.py", line 166, in systemd
    site_path = os.path.dirname(config.get("taky", "cfg_path"))
  File "/usr/lib/python3.10/posixpath.py", line 152, in dirname
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType
@vctibor
Copy link
Author

vctibor commented Jan 27, 2023

systemd function is trying to read cfg_path option from config, although it's never added. Butt even if I add this option manually, it fails to read it.

@gncnpk
Copy link

gncnpk commented Jan 28, 2023

It looks like systemd_cmd.py is calling load_config with args.cfg_file which is None since no directory is passed, and it is also setting explicit to True which sets cfg_path to args.cfg_file (which is None), easy fix is to change Explicit in systemd_cmd.py to False on line 144 and manually add the option cfg_path into taky.conf and set that to the config file path.

@cniles
Copy link

cniles commented Feb 3, 2023

@gncnpk would you say this is a bug? Hit this following instructions in getting started using ubuntu.

@gncnpk
Copy link

gncnpk commented Feb 3, 2023

@cniles Yes, I'll take a look after work and see what I can do to fix it and make a PR

@tkuester
Copy link
Owner

tkuester commented Feb 4, 2023

Oh dear. This is one of the ugly parts of taky, the setup script. I really should have this fixed, since it's what's in the documentation! @gncnpk Do you think you will submit a PR? I'd love to have more contributors to the codebase!

@tkuester tkuester self-assigned this Feb 5, 2023
@tkuester tkuester added the bug Something isn't working label Feb 5, 2023
@gncnpk
Copy link

gncnpk commented Feb 5, 2023

Hi @tkuester, I would be glad to submit a PR! How/what should the setup script exactly do/check for?

@tkuester
Copy link
Owner

tkuester commented Feb 5, 2023

First, it should be noted that the configuration system is terribly broken, and in desperate need of overhaul!

I was able to recreate the bug by deleting all previous installs and configuration files, and then running:

$ sudo takyctl setup --user "$(whoami)" --public-ip 192.168.1.100
$ sudo takyctl systemd --user "$(whoami)"

If I remember correctly, cfg_path is simply the location of the config file... which I don't really use anywhere else but the systemd command. It seems like config.py should be setting cfg_path, but isn't. I think all that needs to be done is remove the check to see if explicit == True on line 136.

@gncnpk
Copy link

gncnpk commented Feb 5, 2023

Gotcha, will do that now, as a quick fix for new users attempting to use taky, and later on we can work together on overhauling the configuration system.

@wuhei
Copy link

wuhei commented Feb 12, 2023

Hoping not to disturb with my noob nonsense, here's how I fixed it (using Debian 10 but I assume it's worht trying on other distros as well):

vim /usr/local/lib/python3.9/site-packages/taky/config.py

edited line 138 as follows:

        ret_config.set("taky", "cfg_path", "/etc/taky/taky.conf")

@Jachimo
Copy link

Jachimo commented Mar 3, 2023

Hello, all! Not trying to pile on, but I just ran into this issue and I'm unsure what the recommended workaround is. Should I make the change as @wuhei suggests above? Or should I pull down the latest version from Github instead? (And if I should do the latter, is it going to create a problem to install the Github version on top of the released pip installed version?) I just don't want to dig myself into a dependency-hell hole that's going to be not-fun to fix later.

For completeness, this is what I am seeing, on a brand-new install of Ubuntu 22.04 (which comes with Python 3.10), with taky installed against the system Python (no venv or anything):

$ sudo takyctl systemd --user tak
Building systemd services
systemd failed: expected str, bytes or os.PathLike object, not NoneType
Unhandled exception:
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/taky/cli/__main__.py", line 51, in main
    ret = commands[args.command](args)
  File "/usr/local/lib/python3.10/dist-packages/taky/cli/systemd_cmd.py", line 166, in systemd
    site_path = os.path.dirname(config.get("taky", "cfg_path"))
  File "/usr/lib/python3.10/posixpath.py", line 152, in dirname
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType

tkuester added a commit that referenced this issue Jul 30, 2023
Fixes #79. cfg_path was a terrible decision, and really shouldn't exist.
The idea was for the config object to keep track of where it was loaded
from -- but I'm abusing the ever loving stuffing out of it, and it
should be removed in a future release.

For the time being, however, I'm moving this line within the conditional
to ensure that cfg_path is only set when we are directly reading a file
on the disk. It's pedantic, but hopefully this will prevent any further
undefined behavior until I can revamp the config system.
@tkuester
Copy link
Owner

Hey everyone! I've pushed the fix to the next branch, and I hope to merge this in soon. If you're able to test this, that'd be great!

To install the next branch, run:

sudo python3 -m pip install git+https://github.com/tkuester/taky@next

@jasonmhite
Copy link

@tkuester Just tested your next branch fix and it appears to work fine. Debian 12 (container on top of Proxmox) with system wide install.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix-committed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants