test: unit test func

engine/hatchery/openstack/get.go

@@ -39,7 +39,7 @@ func (h *HatcheryOpenstack) getSmallerFlavorThan(flavor flavors.Flavor) flavors.
 	var smaller *flavors.Flavor
 	for i := range h.flavors {
 		// If the flavor is not the given one and need less CPUs its
-		if h.flavors[i].ID != flavor.ID && h.flavors[i].VCPUs < flavor.VCPUs && (smaller == nil || smaller.VCPUs > h.flavors[i].VCPUs) {
+		if h.flavors[i].ID != flavor.ID && h.flavors[i].VCPUs < flavor.VCPUs && (smaller == nil || smaller.VCPUs < h.flavors[i].VCPUs) {
 			smaller = &h.flavors[i]
 		}
 	}

engine/hatchery/openstack/init.go

@@ -61,14 +61,41 @@ func (h *HatcheryOpenstack) initFlavors() error {
 	if err != nil {
 		return sdk.WithStack(fmt.Errorf("initFlavors> error on flavors.ListDetail: %v", err))
 	}
+
 	lflavors, err := flavors.ExtractFlavors(all)
 	if err != nil {
 		return sdk.WithStack(fmt.Errorf("initFlavors> error on flavors.ExtractFlavors: %v", err))
 	}
-	h.flavors = lflavors
+
+	h.flavors = h.filterAllowedFlavors(lflavors)
+
 	return nil
 }
 
+func (h HatcheryOpenstack) filterAllowedFlavors(allFlavors []flavors.Flavor) []flavors.Flavor {
+	// If allowed flavors are given in configuration we should check that given flavor is part of the list.
+	if len(h.Config.AllowedFlavors) == 0 {
+		return allFlavors
+	}
+
+	filteredFlavors := make([]flavors.Flavor, 0, len(allFlavors))
+	for i := range allFlavors {
+		var allowed bool
+		for j := range h.Config.AllowedFlavors {
+			if h.Config.AllowedFlavors[j] == allFlavors[i].Name {
+				allowed = true
+				break
+			}
+		}
+		if !allowed {
+			log.Debug("initFlavors> flavor '%s' is not allowed", allFlavors[i].Name)
+			continue
+		}
+		filteredFlavors = append(filteredFlavors, allFlavors[i])
+	}
+	return filteredFlavors
+}
+
 func (h *HatcheryOpenstack) initNetworks() error {
 	all, err := tenantnetworks.List(h.openstackClient).AllPages()
 	if err != nil {

engine/hatchery/openstack/init_test.go

@@ -0,0 +1,41 @@
+package openstack
+
+import (
+	"testing"
+
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/ovh/cds/sdk/log"
+)
+
+func TestHatcheryOpenstack_initFlavors(t *testing.T) {
+	log.SetLogger(t)
+
+	h := &HatcheryOpenstack{}
+
+	allFlavors := []flavors.Flavor{
+		{Name: "b2-7", VCPUs: 2},
+		{Name: "b2-15", VCPUs: 4},
+		{Name: "b2-30", VCPUs: 8},
+		{Name: "b2-60", VCPUs: 16},
+		{Name: "b2-120", VCPUs: 32},
+	}
+
+	filteredFlavors := h.filterAllowedFlavors(allFlavors)
+	require.Len(t, filteredFlavors, 5, "no filter as allowed flavor list is empty in config")
+
+	h.Config.AllowedFlavors = []string{"b2-15", "b2-60"}
+
+	filteredFlavors = h.filterAllowedFlavors(allFlavors)
+	require.Len(t, filteredFlavors, 2)
+	assert.Equal(t, "b2-15", filteredFlavors[0].Name)
+	assert.Equal(t, "b2-60", filteredFlavors[1].Name)
+
+	h.Config.AllowedFlavors = []string{"s1-4", "b2-15"}
+
+	filteredFlavors = h.filterAllowedFlavors(allFlavors)
+	require.Len(t, filteredFlavors, 1)
+	assert.Equal(t, "b2-15", filteredFlavors[0].Name)
+}

engine/hatchery/openstack/openstack.go

@@ -169,28 +169,13 @@ func (h *HatcheryOpenstack) WorkerModelsEnabled() ([]sdk.Model, error) {
 			continue
 		}
 
-		// If allowed flavors are given in configuration we should check that given flavor is part of the list.
-		if h.Config.AllowedFlavors != nil && len(h.Config.AllowedFlavors) > 0 {
-			var allowed bool
-			for j := range h.Config.AllowedFlavors {
-				if h.Config.AllowedFlavors[j] == allModels[i].ModelVirtualMachine.Flavor {
-					allowed = true
-					break
-				}
-			}
-			if !allowed {
-				log.Debug("WorkerModelsEnabled> model %s/%s is not usable because flavor '%s' is not allowed", allModels[i].Group.Name, allModels[i].Name, allModels[i].ModelVirtualMachine.Flavor)
-				continue
-			}
-		}
-
 		filteredModels = append(filteredModels, allModels[i])
 	}
 
 	// Sort models by required CPUs, this will allows to starts job without defined model on the smallest flavor.
 	sort.Slice(filteredModels, func(i, j int) bool {
 		flavorI, _ := h.flavor(filteredModels[i].ModelVirtualMachine.Flavor)
-		flavorJ, _ := h.flavor(filteredModels[i].ModelVirtualMachine.Flavor)
+		flavorJ, _ := h.flavor(filteredModels[j].ModelVirtualMachine.Flavor)
 		return flavorI.VCPUs < flavorJ.VCPUs
 	})
 

engine/hatchery/openstack/openstack_test.go

@@ -4,10 +4,14 @@ import (
 	"context"
 	"testing"
 
+	"github.com/golang/mock/gomock"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ovh/cds/sdk"
+	"github.com/ovh/cds/sdk/cdsclient/mock_cdsclient"
+	"github.com/ovh/cds/sdk/log"
 )
 
 func TestHatcheryOpenstack_CanSpawn(t *testing.T) {
@@ -28,41 +32,60 @@ func TestHatcheryOpenstack_CanSpawn(t *testing.T) {
 	// no model, hostname prerequisite, canSpawn must be false: hostname can't be managed by openstack hatchery
 	canSpawn = h.CanSpawn(context.TODO(), nil, 1, []sdk.Requirement{{Type: sdk.HostnameRequirement, Value: "localhost"}})
 	require.False(t, canSpawn)
+}
 
-	flavors := []flavors.Flavor{
-		{Name: "b2-7"},
-	}
-	h.flavors = flavors
+func TestHatcheryOpenstack_WorkerModelsEnabled(t *testing.T) {
+	log.SetLogger(t)
 
-	m := &sdk.Model{
-		ID:   1,
-		Name: "my-model",
-		Group: &sdk.Group{
-			ID:   1,
-			Name: "mygroup",
-		},
-		ModelVirtualMachine: sdk.ModelVirtualMachine{
-			Flavor: "vps-ssd-3",
-		},
-	}
+	h := &HatcheryOpenstack{}
 
-	// model with a unknowned flavor
-	canSpawn = h.CanSpawn(context.TODO(), m, 1, nil)
-	require.False(t, canSpawn)
+	ctrl := gomock.NewController(t)
+	mockClient := mock_cdsclient.NewMockInterface(ctrl)
+	h.Client = mockClient
+	t.Cleanup(func() { ctrl.Finish() })
+
+	mockClient.EXPECT().WorkerModelsEnabled().DoAndReturn(func() ([]sdk.Model, error) {
+		return []sdk.Model{
+			{
+				ID:    1,
+				Type:  sdk.Docker,
+				Name:  "my-model-1",
+				Group: &sdk.Group{ID: 1, Name: "mygroup"},
+			},
+			{
+				ID:                  2,
+				Type:                sdk.Openstack,
+				Name:                "my-model-2",
+				Group:               &sdk.Group{ID: 1, Name: "mygroup"},
+				ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "b2-120"},
+			},
+			{
+				ID:                  3,
+				Type:                sdk.Openstack,
+				Name:                "my-model-3",
+				Group:               &sdk.Group{ID: 1, Name: "mygroup"},
+				ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "b2-7"},
+			},
+			{
+				ID:                  4,
+				Type:                sdk.Openstack,
+				Name:                "my-model-4",
+				Group:               &sdk.Group{ID: 1, Name: "mygroup"},
+				ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "unknown"},
+			},
+		}, nil
+	})
 
-	m = &sdk.Model{
-		ID:   1,
-		Name: "my-model",
-		Group: &sdk.Group{
-			ID:   1,
-			Name: "mygroup",
-		},
-		ModelVirtualMachine: sdk.ModelVirtualMachine{
-			Flavor: "b2-7",
-		},
+	h.flavors = []flavors.Flavor{
+		{Name: "b2-7", VCPUs: 2},
+		{Name: "b2-30", VCPUs: 16},
+		{Name: "b2-120", VCPUs: 32},
 	}
 
-	// model with a knowned flavor
-	canSpawn = h.CanSpawn(context.TODO(), m, 1, nil)
-	require.True(t, canSpawn)
+	// Only model that match a known flavor should be returned and sorted by CPUs asc
+	ms, err := h.WorkerModelsEnabled()
+	require.NoError(t, err)
+	require.Len(t, ms, 2)
+	assert.Equal(t, "my-model-3", ms[0].Name)
+	assert.Equal(t, "my-model-2", ms[1].Name)
 }

engine/hatchery/openstack/spawn.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+	"github.com/sirupsen/logrus"
 
 	"github.com/ovh/cds/sdk"
 	"github.com/ovh/cds/sdk/hatchery"
@@ -30,9 +31,13 @@ func (h *HatcheryOpenstack) SpawnWorker(ctx context.Context, spawnArgs hatchery.
 		return sdk.WithStack(fmt.Errorf("no job ID and no register"))
 	}
 
-	existingServers := h.getServers(ctx)
-	if len(existingServers) >= h.Configuration().Provision.MaxWorker {
-		log.Debug("MaxWorker limit (%d) reached", h.Configuration().Provision.MaxWorker)
+	if err := h.checkSpawnLimits(ctx, *spawnArgs.Model); err != nil {
+		isErrWithStack := sdk.IsErrorWithStack(err)
+		fields := logrus.Fields{}
+		if isErrWithStack {
+			fields["stack_trace"] = fmt.Sprintf("%+v", err)
+		}
+		log.InfoWithFields(ctx, fields, "%s", err)
 		return nil
 	}
 
@@ -42,33 +47,6 @@ func (h *HatcheryOpenstack) SpawnWorker(ctx context.Context, spawnArgs hatchery.
 		return err
 	}
 
-	// If a max CPUs count is set in configuration we will check that there are enough CPUs available to spawn the model
-	var totalCPUsUsed int
-	if h.Config.MaxCPUs > 0 {
-		for i := range existingServers {
-			totalCPUsUsed += existingServers[i].Flavor["vcpus"].(int)
-		}
-		if totalCPUsUsed+flavor.VCPUs >= h.Config.MaxCPUs {
-			log.Debug("MaxCPUs limit (%d) reached", h.Config.MaxCPUs)
-			return nil
-		}
-	}
-
-	// If the CountSmallerFlavorToKeep is set in config, we should check that there will be enough CPUs to spawn a smaller flavor after this one
-	if h.Config.MaxCPUs > 0 && h.Config.CountSmallerFlavorToKeep > 0 {
-		smallerFlavor := h.getSmallerFlavorThan(flavor)
-		// If same id, means that the requested flavor is the smallest one so we want to start it.
-		if smallerFlavor.ID != flavor.ID {
-			minCPUsNeededToStart := flavor.VCPUs + h.Config.CountSmallerFlavorToKeep*smallerFlavor.VCPUs
-			countCPUsLeft := int(math.Max(.0, float64(h.Config.MaxCPUs-totalCPUsUsed))) // Set zero as min value in case that the limit changed and count of used greater than max count
-			if minCPUsNeededToStart > countCPUsLeft {
-				log.Debug("CountSmallerFlavorToKeep limit reached, can't start model %s/%s with flavor %s that requires %d CPUs. Smaller flavor is %s and need %d CPUs. There are currently %d/%d left CPUs.",
-					spawnArgs.Model.Group.Name, spawnArgs.Model.Name, flavor.Name, flavor.VCPUs, smallerFlavor.Name, smallerFlavor.VCPUs, countCPUsLeft, h.Config.MaxCPUs)
-				return nil
-			}
-		}
-	}
-
 	// Get image ID
 	imageID, err := h.imageID(ctx, spawnArgs.Model.ModelVirtualMachine.Image)
 	if err != nil {
@@ -173,3 +151,46 @@ func (h *HatcheryOpenstack) SpawnWorker(ctx context.Context, spawnArgs hatchery.
 	}
 	return nil
 }
+
+func (h *HatcheryOpenstack) checkSpawnLimits(ctx context.Context, model sdk.Model) error {
+	existingServers := h.getServers(ctx)
+	if len(existingServers) >= h.Configuration().Provision.MaxWorker {
+		return sdk.WithStack(fmt.Errorf("MaxWorker limit (%d) reached", h.Configuration().Provision.MaxWorker))
+	}
+
+	// Get flavor for target model
+	flavor, err := h.flavor(model.ModelVirtualMachine.Flavor)
+	if err != nil {
+		return err
+	}
+
+	// If a max CPUs count is set in configuration we will check that there are enough CPUs available to spawn the model
+	var totalCPUsUsed int
+	if h.Config.MaxCPUs > 0 {
+		for i := range existingServers {
+			totalCPUsUsed += existingServers[i].Flavor["vcpus"].(int)
+		}
+		if totalCPUsUsed+flavor.VCPUs > h.Config.MaxCPUs {
+			return sdk.WithStack(fmt.Errorf("MaxCPUs limit (%d) reached", h.Config.MaxCPUs))
+		}
+	}
+
+	// If the CountSmallerFlavorToKeep is set in config, we should check that there will be enough CPUs to spawn a smaller flavor after this one
+	if h.Config.MaxCPUs > 0 && h.Config.CountSmallerFlavorToKeep > 0 {
+		smallerFlavor := h.getSmallerFlavorThan(flavor)
+		// If same id, means that the requested flavor is the smallest one so we want to start it.
+		log.Debug("checkSpawnLimits> smaller flavor found for %s is %s", flavor.Name, smallerFlavor.Name)
+		if smallerFlavor.ID != flavor.ID {
+			minCPUsNeededToStart := flavor.VCPUs + h.Config.CountSmallerFlavorToKeep*smallerFlavor.VCPUs
+			countCPUsLeft := int(math.Max(.0, float64(h.Config.MaxCPUs-totalCPUsUsed))) // Set zero as min value in case that the limit changed and count of used greater than max count
+			if minCPUsNeededToStart > countCPUsLeft {
+				return sdk.WithStack(fmt.Errorf("CountSmallerFlavorToKeep limit reached, can't start model %s/%s with flavor %s that requires %d CPUs. Smaller flavor is %s and need %d CPUs. There are currently %d/%d left CPUs",
+					model.Group.Name, model.Name, flavor.Name, flavor.VCPUs, smallerFlavor.Name, smallerFlavor.VCPUs, countCPUsLeft, h.Config.MaxCPUs))
+			}
+			log.Debug("checkSpawnLimits> %d/%d CPUs left is enougth to start model %s/%s with flavor %s that require %d CPUs",
+				countCPUsLeft, h.Config.MaxCPUs, model.Group.Name, model.Name, flavor.Name, flavor.VCPUs)
+		}
+	}
+
+	return nil
+}