Refactor service/daemon code to be usable outside of Ubic framework

[berekuk]

One possible plan for refactoring: https://gist.github.com/b7a2ab5ae45df5847682

Alternatively, we can move all Ubic::Daemon code to Daemon::Control as patches and then port Ubic::Daemon to use Daemon::Control.

[berekuk]

Ok, new plan which ties everything together:

1. Implement service wrappers mechanism.
   Something like:

Ubic::Service::Wrapper::LSB->new->wrap(
    Ubic::Service::Wrapper::WithLocks->new->wrap(
        Ubic::Service::Wrapper::WithPersistentStatuses->wrap(
            $service # SimpleDaemon, for example
        )
    )
);

The point is so separate service-tree features (code from Ubic and Ubic::Cmd) and services further.

So we will have pure services with start/stop/status methods, and then we will have complete services which update persistent status, support LSB methods like force_reload, and maybe even print to terminal themselves.

I know the example looks complicated and over-engineered, but we can add syntax sugar later.
(Wrappers need to be objects instead of classes because we're going to parameterize Ubic::Multiservice::WithWrappers class which wraps the whole multiservice. So, ->wrap instead of ->new($service)).

We can't use roles, though. There isn't any roles implementation which can be applied to objects instead of classes without causing memory leaks. Because perl doesn't have anonymous classes.
This is ok, though, because Ubic::Service method list is fixed and short. So it won't be too hard to delegate everything manually.

2. Refactor Ubic and Ubic::Cmd to use wrappers.
   Some of wrappers which we will need are listed above.
   We'll also need to figure out how to refactor credentials stuff without breaking things. (By now, services define user and group methods, but don't use them themselves, relying on Ubic.pm code instead).

3. Separate Ubic::Service::* and some wrappers to the different distribution and write syntax-sugar layer.
   Syntax-sugar layer is just another wrapper which turns the service into init script by adding the proper logging, exiting with LSB codes and implementing ->run method.

[berekuk]

Wrappers will be useful outside of the scope of this task too, of course.
Some of the ideas for their application can be found at http://www.slideshare.net/berekuk/ubic-yapc-2012, slide 25.

[berekuk]

There isn't any roles implementation which can be applied to objects instead of classes without causing memory leaks. Because perl doesn't have anonymous classes.

I was wrong.
Moose's apply_all_roles and Role::Tiny's apply_roles_to_object reuse the existing class names.

[shadowcat-mst]

it's called a decorator, not a wrapper, usually. plus I'd make ->new do the wrapping, otherwise ->wrap is going to have to produce a new object. nothing wrong with FooWrapper->new(service => $service, other_option => 'SPOON');

[berekuk]

Yeah, I know what decorators are. The reason I'm not using this word is because I still can go with roles instead of delegation.

The reason I chose ->new($options)->wrap($service) is because of this paragraph:

Wrappers need to be objects instead of classes because we're going to parameterize Ubic::Multiservice::WithWrappers class which wraps the whole multiservice. So, ->wrap instead of ->new($service).

(Multiservice is an object representing the subtree of services, and wrapped multiservice should apply given wrapper to each subservice someone asks from it. Though we can avoid two objects per wrapper if we parameterize multiservice with class name and options hashref.)

What puzzles me more is the painful decorators vs roles choice.

Decorators

If I go with decorators, they'll have to delegate each Ubic::Service method, and there are many.
And what if I decide to add the new command to Ubic::Service base class later?
What's worse, services are allowed to add custom commands (https://metacpan.org/module/Ubic::Service#CUSTOM-COMMAND-METHODS).
They are currently implemented with just two custom_commands and do_custom_command methods, but I want to replace this with method attributes in the future.

I could implement some kind of infrastructure which delegates every method tagged in some way (with method attributes?), but I really don't want to accidentally reinvent the new object system here :)
Or I could go with AUTOLOAD, but I think that's a bad idea.

Roles

With roles, there are different problems.

This is how I imagine the service configuration should look like, whatever we choose for implementation:

module: Ubic::Service::SimpleDaemon
options:
    bin: /usr/sbin/nginx
wrappers:    # wrappers? middlewares? decorators? features? plugins? (arrrrgh, I'll just call them wrappers by now)
    Ubic::Wrapper::PortCheck:
        port: 80
        timeout: 3
        url: /ping
    Ubic::Wrapper::MemoryCheck:
        limit: 512M     # if limit is exceeded, watchdog will restart the service

(I chose YAML for this example, but the similar thing can be done in perl)
So, I want options to be wrapper's properties. I think it would even be useful to apply some wrapper twice with different parameters! (I'm trying to reinvent monit here and I'm aware of this.)

So, issue 1: I need parameterized roles, and I can't use Moose.
Should look at Moo and check if parameterized roles are possible to implement as MooX::...

Issue 2: wrappers application order can be important. I'm pretty sure both Moose and Moo will do-what-I-expect if I apply roles one-by-one, but relying on roles ordering is still considered a sin, right?

Issue 3: of course roles and decorators are totally different in their behavior when the base class calls its own method. I'm still not sure which behavior I need, because I didn't wrote any code yet, but decorators look saner to me in this aspect.

PS: Whatever I choose, final implementation should be easy to use for those who want to write a new wrapper.

[shadowcat-mst]

your argument wrt multiservice is incoherent; I think you're imagining problems that don't exist depending on the implementation.

building a decorator is easy given (handles => 'NameOfRole')

Package::Variant gives you a parameterized role like thing, but simply asserting "I need parameterized roles" isn't useful. I think "jam everything onto the service object" is likely the wrong answer here, and decorators built -using- roles is going to be rather nicer.

equally, I don't really see how PortCheck or MemoryCheck is actually a wrapper; the locking thing is but they aren't - don't try and smash those two concepts together, it'll only make you cry.

[berekuk]

your argument wrt multiservice is incoherent

Yes, you're right. There are many ways to do it. Passing sub {} is another.
On the other hand, my version is closer to the way Plack middlewares work.
I'll have to think more about trade-offs here.

decorators built -using- roles is going to be rather nicer.

Yes! That's what I thought too after writing my last comment.
So, decorators it is, and all decorator classes should include role which adds handles for delegating all necessary methods.

equally, I don't really see how PortCheck or MemoryCheck is actually a wrapper

But I don't agree with this one. How else can I implement these features? Invent some kind of status check stack?
Here's a poorly thought-out list of other wrapper ideas, to show the scope of problems I'm trying to solve:

• adding custom commands like pid, bin, exec (run without daemonizing), logrotate, etc.; some of them could rely on base service object having particular roles;
• notifications system, via http requests, sockets or zeromq (see http://circus.readthedocs.org/en/latest/index.html, for example);
• note that MemoryCheck wrapper could be applied to the whole service tree by local sysadm who's got tons of poorly implemented memory-leaking services on his hands.

I agree that *Check examples is just a matter of adding around to status method. But I don't see why they shouldn't be just a particular case of a more generic solution.

Thank you for your comments, it's all much cleaner now :)

[berekuk]

@shadowcat-mst Damn, it's possible that you're right about Checks not being wrappers after all.
Service's start method should check for status in a loop itself (Ubic::Service::Skeleton base class handles this stuff currently).
So it'll be either "apply CheckLoop wrapper after everything else", or "don't use delegation for this".
I guess the only way to find out which way works better it to shup up and start coding :)

[shadowcat-mst]

the crucial one is that MemoryCheck is actually an instruction to the watchdog to load a particular plugin, which seems to me to not be the same class of problem.

if PortCheck is "how to test if the service is up" then maybe that one -is-, but in that case PortCheck and MemoryCheck shouldn't have such similar names

[berekuk]

Oh right. I'm always forgetting about this one.
The thing is, the idea that the same status check is used in starting loop, in 'ubic status' CLI command and in watchdog greatly simplifies things.
But it is a simplification, and it did bite us in the past.
Separate 'check' method would solve this, but it's outside of scope of this task.