Hotfix/fix start event issue #701

[mrtrkmn]

Closes #701

• Checking available frontends before actually starting to spin up resources
• Remove CreateEventFromDB function, add SaveToDB.
• CreateEventFromDB function was not performing what it says.

[Mikkelhost]

When you are saying checking for available frontends? Does it check if it has been configured?

I have been experiencing that the webclient displays all files which ends with .ova

The expected behavior i would like is that the webclient only displays the vm's that has been configured in frontends.yml

Would be nice to edit this grpc call to only return configured vm's:

func (d *daemon) ListFrontends(ctx context.Context, req *pb.Empty) (*pb.ListFrontendsResponse, error) {
    var respList []*pb.ListFrontendsResponse_Frontend

    err := filepath.Walk(d.conf.ConfFiles.OvaDir, func(path string, info os.FileInfo, err error) error {
        if filepath.Ext(path) == ".ova" {
            relativePath, err := filepath.Rel(d.conf.ConfFiles.OvaDir, path)
            if err != nil {
                return err
            }
            parts := strings.Split(relativePath, ".")
            image := filepath.Join(parts[:len(parts)-1]...)

            ic := d.frontends.GetFrontends(image)[0]
            respList = append(respList, &pb.ListFrontendsResponse_Frontend{
                Image:    image,
                Size:     info.Size(),
                MemoryMB: int64(ic.MemoryMB),
                Cpu:      float32(ic.CPU),
            })
        }
        return nil
    })
    if err != nil {
        return nil, err
    }

    return &pb.ListFrontendsResponse{Frontends: respList}, nil
}

[mrtrkmn]

When you are saying checking for available frontends? Does it check if it has been configured?

I have been experiencing that the webclient displays all files which ends with .ova

The expected behavior i would like is that the webclient only displays the vm's that has been configured in frontends.yml

Yes, from webclient perspective there might not be a problem, however I encouraged the problem when using CLI, in case it has been used, the platform will crash, and before crash it will save the event to database. In order to prevent such scenarios, it would be ideal to check existing frontends which are known by the platform, otherwise a typo in frontend name would crash the platform.

Also, there was a function called CreateEventFromDB, that function was misleading by its name, I removed it and add SaveToDatabase function which is more proper in code readability as well.

[mrtrkmn]

Hmm, configured VMs are actually managed through frontends.yml file instead of the folder which includes .ova file, to make it more proper we can move that yml file to store as well. With path option in store and add more checks before starting to import ova file such as, check if OVA is legit, if OVA file exists and so on. What do you think about it ?

[Mikkelhost]

From my experience, if you try to use a vm that has not been configured in the frontends.yml file it tries to start the vm with 0 memory. Vbox comes back with an error on that one which in turn makes haaukins crash with a nil pointer de-reference panic. Either i would suggest to only try and start the vm if the vm has been configured in the frontends.yml or i would make the function that sets the vm memory or whatever, check if it is 0. If it is 0, it should just default to 4GB or some other default value.

[mrtrkmn]

From my experience, if you try to use a vm that has not been configured in the frontends.yml file it tries to start the vm with 0 memory. Vbox comes back with an error on that one which in turn makes haaukins crash with a nil pointer de-reference panic. Either i would suggest to only try and start the vm if the vm has been configured in the frontends.yml or i would make the function that sets the vm memory or whatever, check if it is 0. If it is 0, it should just default to 4GB or some other default value.

GetAllAvailableFrontends function gets infromation from the frontends.yml file, so from already configured place. However, as you suggested it would be ideal to set default memory if memory is not set correctly, also check if ova file exists.

[Mikkelhost]

From my experience, if you try to use a vm that has not been configured in the frontends.yml file it tries to start the vm with 0 memory. Vbox comes back with an error on that one which in turn makes haaukins crash with a nil pointer de-reference panic. Either i would suggest to only try and start the vm if the vm has been configured in the frontends.yml or i would make the function that sets the vm memory or whatever, check if it is 0. If it is 0, it should just default to 4GB or some other default value.

GetAllAvailableFrontends function gets infromation from the frontends.yml file, so from already configured place. However, as you suggested it would be ideal to set default memory if memory is not set correctly, also check if ova file exists.

Yes!

[mrtrkmn]

Roger, I am going to add them.

[mrtrkmn]

On top of changes done earlier, default values for frontend image is added and checked during creating an event with last commit. What are your feedback on this PR @Lanestolen and @Mikkelhost ?

[mrtrkmn]

I am thinking to merge these changes @Mikkelhost and @Lanestolen. Just for your information.

[Lanestolen]

If @Mikkelhost approves it is fine with me

[Mikkelhost]

Looks good to me, i agree on the points stated by @Lanestolen Making assumptions is a nono.

I would just state, that instead of creating an event with a vm which the user did not mean to use, i would respond with an error instead to the user. Lets take an example:

1. I would like to create an event with 100 capacity and 100 availability like we do for the DDC.
2. I somehow select a vm which is not available.
3. I start the whole event which at times easily takes 30 minutes with this capacity.
4. I discover the vm is wrong, so i now have to shut down the event (30 min - 60 min) to create a new one with the right vm.

Hence i would much rather have it give me an error if i do not specify the right vm or in general if i specify any wrong parameter :)

[mrtrkmn]

Looks good to me, i agree on the points stated by @Lanestolen Making assumptions is a nono.

I would just state, that instead of creating an event with a vm which the user did not mean to use, i would respond with an error instead to the user. Lets take an example:

1. I would like to create an event with 100 capacity and 100 availability like we do for the DDC.
2. I somehow select a vm which is not available.
3. I start the whole event which at times easily takes 30 minutes with this capacity.
4. I discover the vm is wrong, so i now have to shut down the event (30 min - 60 min) to create a new one with the right vm.

Hence i would much rather have it give me an error if i do not specify the right vm or in general if i specify any wrong parameter :)

Roger, will look into into !

[mrtrkmn]

The thing is that, it is not possible to create event with non-existing image through webclient interface, since it only lists ova files which are exist. However, it does not mean, it cannot be created, either using CLI or basic curl requests it is possible for this reason last commit checks image path if does not exists, returns an error.

@Mikkelhost

[Mikkelhost]

The thing is that, it is not possible to create event with non-existing image through webclient interface, since it only lists ova files which are exist. However, it does not mean, it cannot be created, either using CLI or basic curl requests it is possible for this reason last commit checks image path if does not exists, returns an error.

@Mikkelhost

This will also cancel event creation right? It was mainly a concern of user friendliness, all i want to happen is if i try to create an event, and i use values which i expect to work, then i would rather get an error and have to try again than it creating an event for me with some default values :)

[mrtrkmn]

This will also cancel event creation right?

Yes, it is just returning error without initializing anything

[Mikkelhost]

Then i think it's looking good :)