edgedb / edgedb-pkg

EdgeDB Release Packaging Toolkit
1 stars 2 forks source link

Remove service definitions on macOS and linux #17

Closed tailhook closed 4 years ago

tailhook commented 4 years ago

We want edgedb server to be responsive for service files.

This has the downside is that systems which are not managed by edgedb server (i.e. perhaps all production systems) don't have service files out of the box. This isn't a big downside, though, as most production systems are managed by ansible or terraform or similar tools that can generate service files better.

(It's untested, need time to recall how to test this repo)

elprans commented 4 years ago

ansible or terraform or similar tools that can generate service files better

That's questionable. I think we should still ship some service files in the packages, just don't enable them by default. For systemd we can even ship an instanced service file (edgedb-server@.service) and make use of that in edgedb server. We can do a similar thing on macOS also (i.e. ship a plist template and copy/instantiate it as necessary).

tailhook commented 4 years ago

instanced service file (edgedb-server@.service) and make use of that in edgedb server

This doesn't work well, as we also have a --port and --server-options. So we have a per-instance override file anyway. I prefer raw file than multiple pieces that are scattered around and can be broken during some routine update.

We can do a similar thing on macOS also (i.e. ship a plist template and copy/instantiate it as necessary).

Is plist template a thing? It looks like plist files from packages are loaded by OS by itself, so we can't ship "inactive" file as far as I understand.

elprans commented 4 years ago

It looks like plist files from packages are loaded by OS by itself,

No, we load the service explicitly in the "postinstall" script. We can add an env var to skip that similar to the SKIP_BOOTSTRAP thing.

This doesn't work well

I understand, but I don't think removing the service files from the packages for everyone is the best solution. Why can't edgedb server install remove the service files post-install if they're interfering?

tailhook commented 4 years ago

It looks like plist files from packages are loaded by OS by itself,

No, we load the service explicitly in the "postinstall" script. We can add an env var to skip that similar to the SKIP_BOOTSTRAP thing.

Last time we debugged issue with loading a service, you have said that it's done automatically, without any scripting on our end. If it's not, let's do that.

This doesn't work well

I understand, but I don't think removing the service files from the packages for everyone is the best solution. Why can't edgedb server install remove the service files post-install if they're interfering?

Removing sounds weird. The file will appear on every reinstall. We can disable it by (1) shadowing with dev/null symlink (it's normal pattern on systemd) or (2) just assume that user never starts or enables it. But I'm not sure when to do (1), on reinstall? On every start? And (2) is quite fragile, I believe users will forget which way they have started the edgedb and running wrong way will appear as "edgedb loosing data".

elprans commented 4 years ago

you have said that it's done automatically

I meant it's done automatically by the package by default (as opposed to having the user enable it explicitly).

We can disable it by (1) shadowing with dev/null symlink

OK, so is the port conflict the only problem here? Can we simply check if the port is busy and show a nice error message in server init/start?

tailhook commented 4 years ago

you have said that it's done automatically

I meant it's done automatically by the package by default (as opposed to having the user enable it explicitly).

Well, we needed to find launchctl load command, to add an unload before to make package easily upgradable. Currently edgedb server does that, but upgrading package manually doesn't work. Do you say that you know how to fix that?

We can disable it by (1) shadowing with dev/null symlink

OK, so is the port conflict the only problem here? Can we simply check if the port is busy and show a nice error message in server init/start?

This is the only technical problem. And yes, we can check the port.

But I think users will mess up this very often because they can start both systemctl start edgedb-server and edgedb server start and both will work, but with the different underlying data storage.

ambv commented 4 years ago

I personally think it's fine to drop launchd and systemd integration now that we have edgedb server. If those new commands end up being insufficient, we can restore this functionality in a future release.

tailhook commented 4 years ago

Also, the problem might be when two version of packages are installed. So we have two system services competing for port 5656

elprans commented 4 years ago

now that we have edgedb server

The problem is that we don't yet have equivalent functionality in edgedb server, because system instances are not implemented yet (at least they weren't when I checked yesterday).

tailhook commented 4 years ago

now that we have edgedb server

The problem is that we don't yet have equivalent functionality in edgedb server, because system instances are not implemented yet (at least they weren't when I checked yesterday).

Yes system instances aren't implemented yet.

ambv commented 4 years ago

As discussed on Slack and in the weekly meeting, we need to include this for alpha4 to unblock upgrades.

elprans commented 4 years ago

Note: this breaks the test-systemd target, which we're not testing with GHA, and which should probably also be removed

elprans commented 4 years ago

Also, @ambv we need to remove the bits from macos/test.sh that expect the service to get started on installation.