pufferpanel / pufferd

The PufferPanel daemon
https://pufferd.pufferpanel.com
Apache License 2.0
46 stars 15 forks source link

First uninstall function attempt #28

Closed Pandry closed 7 years ago

Pandry commented 7 years ago

I did try...

LordRalex commented 7 years ago

Aside from the comments, I like that you covered most of the points I was looking at.

A few things though we probably should also remove is the /etc/pufferd directory where configs and assorted data is as well.

You also followed the golang style (betting you just followed how install worked, but still), which has the OS specific code, good job :)

Pandry commented 7 years ago

How do you suggest to remove the "/etc/pufferd"? I wouldn't use a remove by the golang itself (since it could leave a trace) I would instead use a "rm -rf"

LordRalex commented 7 years ago

Please also make sure this builds, not entirely sure if this can build (do not rely on the CI server for now)

Pandry commented 7 years ago

Okay, I think all errors get fixed, if anything is okay I would proceed to test in a VM

Pandry commented 7 years ago

TODO: Read data from config file to delete data Windows version Test

Pandry commented 7 years ago

At the moment I'm not at the computer, so I did not see how the program recognize the OS... Can you tell mW where's the snippet which recognize the OS?

LordRalex commented 7 years ago

You do not directly code for it.

Golang will use the file name to determine if it should include that file for a build for a specific OS (the _linux and _windows), and only include those that matter for that OS.

This means that you need a uninstall_windows.go file which implements the publicly used function StartProcess and does whatever it needs.

https://github.com/PufferPanel/pufferd/blob/master/pufferd.go#L141 is an example, which we implement differently for Windows and Linux (https://github.com/PufferPanel/pufferd/blob/master/install/service_linux.go)

As long as the function exists for both OSes, Golang will build it and you don't have to code for it directly.

Pandry commented 7 years ago

Ooh, Awsome! It wasn't on the ebook I read :P

Pandry commented 7 years ago

I started thinking about how to modify the function... At the moment I would love to have a sample of the config file (since I'm not able to host a PufferPanel right now) And I was thinking about passing the config as an argument (if possible) And meanwhile I would try to code the windows version (And is the Windows config file different?)

LordRalex commented 7 years ago

You do not need to worry about the config format or location, we handle that using the --config option already

You have access to the config.Get functions, which ask for a "key" and will give you the value from the config if it exists

https://github.com/PufferPanel/pufferd/tree/master/data contains the 2 configs that we by default use, including their keys and values.

Pandry commented 7 years ago

Awsome, thanks!

Pandry commented 7 years ago

I have a question... Should I remove the folders in the config only (so leaving the /var/lib/pufferd folder in case of default path) or I should actually try to see if the path contains a "pufferd" path and delete it?

"serverfolder": "/var/lib/pufferd/servers",
"templatefolder": "/var/lib/pufferd/templates",
"datafolder": "/etc/pufferd"
LordRalex commented 7 years ago

Delete the folder in the config is the way I would do it.

I have plans on redoing that config, so that the /var/lib/pufferd folder isn't broken up that way.

Pandry commented 7 years ago

I guess everything's alright now, can I test if it work?

LordRalex commented 7 years ago

You should always be trying to test as you go.

If you need help building it though, I can alter the build server to help prepare builds for you, but note it doesn't log if there's an error (which I believe your code might)

Pandry commented 7 years ago

I should have fixed anything, today I'll try to compile the program and test it

Pandry commented 7 years ago

Okay, obviously the "uninstall" var caused a conflict with the "uninstall" package, so i renamed it to "uninstaller"

Pandry commented 7 years ago

I'm stuck since if a config file is not found, I would like to ask if delete the default config path (in the default config), so I would like to improve the "GetOrDefault" in the config package to make it return the default var in the default config, can I do it?

Pandry commented 7 years ago

Okay, I tested it and it works both in Debian and Fedora Server