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

Ze/labvm status job #58

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from
Open

Ze/labvm status job #58

wants to merge 23 commits into from

Conversation

WarriorAchilles
Copy link
Contributor

Implemented service and job to check the status of vms every minute.

CSLabs.Api/Services/TestVmConnectionService.cs Outdated Show resolved Hide resolved
CSLabs.Api/Services/TestVmConnectionService.cs Outdated Show resolved Hide resolved
Cliftonz
Cliftonz previously approved these changes Jan 28, 2022
@WarriorAchilles WarriorAchilles marked this pull request as ready for review January 28, 2022 18:36
Comment on lines 20 to 25
using var context = scope.ServiceProvider.GetService<DefaultContext>();

// Do job
var connectionService = new TestVmConnectionService(context);
await connectionService.TestLabVmConnection();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the following to the ServiceProvider.ProvideAppServices method:

 services.AddScoped<TestVmConnectionService>();

Then rewrite this section to handle the dependency injection:

Suggested change
using var context = scope.ServiceProvider.GetService<DefaultContext>();
// Do job
var connectionService = new TestVmConnectionService(context);
await connectionService.TestLabVmConnection();
var connectionService = scope.ServiceProvider.GetService<TestVmConnectionService>();
await connectionService.TestLabVmConnection();

Then anything in the constructor of the TestVmConnectionService will be automatically injected.

Copy link
Contributor

@jasekiw jasekiw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WarriorAchilles
To make the code more testable and separate concerns, do the following:

Create an InjectedAsyncJob class.

using System;
using FluentScheduler;
using Microsoft.Extensions.DependencyInjection;

namespace CSLabs.Api.Jobs
{
    public class InjectedAsyncJob<T> : IJob where T : AsyncJob
    {
        private IServiceProvider _provider;

        public InjectedAsyncJob(IServiceProvider provider) => _provider = provider;

        public void Execute()
        {
            using var scope = _provider.CreateScope();
            scope.ServiceProvider.GetService<T>().Execute();
        }
    }
}

Change the VmStatusJob to the following:

using System.Threading.Tasks;
using CSLabs.Api.Services;

namespace CSLabs.Api.Jobs
{
    public class VmStatusJob : AsyncJob
    {
        private readonly TestVmConnectionService _service;
        public VmStatusJob(TestVmConnectionService service) =>
            _service = service;
        protected override async Task ExecuteAsync() => await _service.TestLabVmConnection();
    }
}

Change the schedule call to the following:

Schedule(provider.GetService<InjectedAsyncJob<VmStatusJob>>).ToRunEvery(1).Minutes();

Add this to the ServiceProvider:

services.AddTransient<VmStatusJob>();
services.AddTransient<InjectedAsyncJob<VmStatusJob>>();

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication


namespace CSLabs.Api.Proxmox
{
public class ProxmoxDBApi : ProxmoxApi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You havent configured this ProxmoxDBApito be used anywhere but one place, it should be used instead of the Proxmox api since you want to track the start and stop states of the VM when initiated by the UI.

{
await base.ShutdownVm(vmId, timeout);
// Save in the database that the VM is stopped
_context.UserLabVms.Find(vmId).Running = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vm id here is the proxmox vmid so the .Find(vmId) will fail.

}
else
{
// TODO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ses sender is merged, so you should be able to continue with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants