Add instance placement scriptlet extension

[victoitor]

Currently, the instance placement scriptlet does not have enough functionality to implement even the default scheduler. Some extra functionality is needed both to be able to reproduce it and to extend it in simple ways.

This was discussed int this linuxcontainers forum post.

The desired extra functionality would be the following:

• get_instances function which would then take optional project and/or location args. We should be able to obtain instance configuration options and cluster node placement from this list.
• get_cluster_members function taking as input a cluster group to return a list of cluster nodes in that cluster group.

[christina-zh]

Im interested in working on this issue, can I be assigned to it please?

[stgraber]

So as mentioned, the goal here is to expose two new functions to instance scriptlets.
Those are defined in:

• internal/server/scriptlet/load/load.go
• internal/server/scriptlet/instance_placement.go
• doc/explanation/clustering.md

This will need API extensions, let's go with one for each function, so:

• instances_scriptlet_get_instances
• instances_scriptlet_get_cluster_members

The functions themselves need to be added to instance_placement.go, you can look at the existing functions there like getInstanceResourcesFunc and getClusterMemberStateFunc.

We'll want get_instances to take optional location and project arguments.
Then it should call GetInstances() from the database with a suitable filter (location/project) and turn the result into an []api.Instance which it then returns.

For get_cluster_members, it should take an optional group argument and use GetNodes() to get the cluster members, then filter by group if needed and turn that into a []api.ClusterMember to return.

As far as commits for this one, it should be a somewhat repetitive:

• api: instances_scriptlet_get_instances
• doc/instances/scriptlet: Add get_instances
• incusd/scriptlet: Add get_instances
• api: instances_scriptlet_get_cluster_members
• doc/instances/scriptlet: Add get_cluster_members
• incusd/scriptlet: Add get_cluster_members

For testing, you'd need an Incus cluster with at least two servers in it.
That can be done pretty easily with a couple of virtual machines, but you can also add the feature without being able to validate it yourself and I can do the validation for you.

[awalvie]

Is this still being worked on? If not can I pick this up? Love the project and would like to start contributing!

[stgraber]

Hey @awalvie, this is currently assigned to a group of student to work on.
Those groups usually get a few easy issues and a more difficult one to work on, this is the case here.

If the group in question somehow can't resolve this one, I'll be happy to assign it to you.

I realize we're a bit short on unassigned issues right now (weird problem to have ;)), but I just opened #741 which may be interesting for you, though a completely different part of Incus :)

[awalvie]

Haha, it's a good problem to have though! I'll check out the linked issue!

[christina-zh]

Hello @stgraber We have a few questions regarding this issue:

1. Should we be adding the API extensions in internal/server/scriptlet/instance_placement.go or internal/version/api.go (or both)?
2. Since get_instances() will be taking optional parameters (location and project), is it correct to call GetInstances() when location and project are not being passed in and call GetInstancesWithFilter() when the parameters are available?
3. In the codebase there's usually a calling object before the call to GetInstances() or GetInstancesWithFilter(). Should we be getting the calling object from conf.GetInstanceServer(remote)?

[stgraber]

Should we be adding the API extensions in internal/server/scriptlet/instance_placement.go or internal/version/api.go (or both)?

The commit api: XYZ should only include the doc/api-extensions.md and internal/version/api.go changes as that's effectively where the API extension gets allocated.

Your changes to instance_placement.go will go into the incusd/scriptlet commit.

In the codebase there's usually a calling object before the call to GetInstances() or GetInstancesWithFilter(). Should we be getting the calling object from conf.GetInstanceServer(remote)?

No, those are client-side functions, you're running from within the server, so in the server, you instead hit the database functions which in this case would be GetInstances() from the DB code.

internal/server/db/cluster/instances.mapper.go:func GetInstances(ctx context.Context, tx *sql.Tx, filters ...InstanceFilter) ([]Instance, error) {

That function takes an InstanceFilter which will let you filter by Project or Location.

[christina-zh]

Thank you for answering our previous questions @stgraber ! We want to ask for clarification on the get_cluster_members function. Since GetNodes() returns a list of NodeInfo, should we be passing that list into GetCandidateMembers(...) to filter offline nodes or is looping through the list and checking for the group enough?

[stgraber]

I think it'd make sense to filter the list based on what's allowed, so indeed limiting it to the list of candidates.

[christina-zh]

Ok that makes sense. Also, since ToAPI(...) takes NodeInfoArgs as the third parameter, should we be setting the fields in the struct similar to how it's being done in instance_placement.go (near the bottom of it)?

[stgraber]

Yeah, that should be fine.