opserver / Opserver

Stack Exchange's Monitoring System
https://opserver.github.io/Opserver/
MIT License
4.5k stars 827 forks source link

Added functionality to start and stop services via wmi. #298

Closed KennethScott closed 6 years ago

KennethScott commented 6 years ago

Hey @NickCraver - 'Cleaning up these comments in the interest of saving time. I'd gotten a little long-winded thinking through this. :) Anyway, I think the way I finally wound up approaching this makes sense (at least to me), but I'll definitely be interested to hear your feedback. I've been running it for a little while now and it seems to be working pretty well. Let me know what you think. Thanks-

KennethScott commented 6 years ago

Hey @NickCraver - 'just wanted to check-in and see if you've had a chance to take a look at this. Is it still something you're interested in? Thanks again-

KennethScott commented 6 years ago

That all makes sense, but the one thing I'm struggling with is if I remove the virtual method from the DashboardDataProvider and instead change WmiDataProvider to just implement the new interface, I'm not sure how to call it from NodeService. That would look like:

public Task<bool> Update(Action action)
{
    Node.DataProvider.ControlServiceAsync(Node, Id, action);
}

But Node.DataProvider here is a DashboardDataProvider (as NodeService isn't WMI-specific), so there's no method.

Maybe I misunderstood?

NickCraver commented 6 years ago

@KennethScott it'd look like this:

public Task<bool> Update(Action action) =>
    (Node.DataProvider as IServiceControlProvider)?.UpdateServiceAsync(Node, Id, action) ?? false;

If the provider isn't a IServiceControlProvider, it'll return false, as it's unable to perform that action...and the UI shouldn't be showing the ability to do this in the first place due to other checks anyway :)

KennethScott commented 6 years ago

Ah, OK. I wasn't sure if you were expecting to have to cast it or not. Thanks for the quick response. I'm going to try to get this polished up and committed this weekend.

NickCraver commented 6 years ago

@KennethScott <3 Much appreciated!

KennethScott commented 6 years ago

Well crap, I thought I'd resolved the merge conflict on the Opserver.Core.csproj file but apparently I didn't do it correctly. Trying to figure out the best way to resolve this... I didn't realize you'd converted the proj file until after I'd committed and sync'ed. I'm going to call it a night, but I will pick back up tomorrow.

KennethScott commented 6 years ago

OK, maybe that got it.

BTW, I went ahead and implemented returning the actual error message from the service action. When I was testing I happened to pick a service that I couldn't get to Stop successfully and at first I couldn't figure out why. It was a bit frustrating since I was just returning success=false and a generic error message from the attempted action. After I realized I was trying to stop a service that had some dependencies, I thought it'd probably be a good idea to return the actual error message as well as just a success/failure boolean. Let me know what you think.

NickCraver commented 6 years ago

@KennethScott looking awesome - merging this in before some more .NET Core work :) Thanks so much!