diff --git a/engine/hatchery/openstack/get.go b/engine/hatchery/openstack/get.go index aca47ef178..65988dc510 100644 --- a/engine/hatchery/openstack/get.go +++ b/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] } } diff --git a/engine/hatchery/openstack/init.go b/engine/hatchery/openstack/init.go index b338525ba0..20b4acebba 100644 --- a/engine/hatchery/openstack/init.go +++ b/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 { diff --git a/engine/hatchery/openstack/init_test.go b/engine/hatchery/openstack/init_test.go new file mode 100644 index 0000000000..578f9cd635 --- /dev/null +++ b/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) +} diff --git a/engine/hatchery/openstack/openstack.go b/engine/hatchery/openstack/openstack.go index 6289116b31..c0fa6b3ad8 100644 --- a/engine/hatchery/openstack/openstack.go +++ b/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 }) diff --git a/engine/hatchery/openstack/openstack_test.go b/engine/hatchery/openstack/openstack_test.go index 0eb2d78676..a267925ce6 100644 --- a/engine/hatchery/openstack/openstack_test.go +++ b/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) } diff --git a/engine/hatchery/openstack/spawn.go b/engine/hatchery/openstack/spawn.go index 2f1e5a7e26..833b1cd1b9 100644 --- a/engine/hatchery/openstack/spawn.go +++ b/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 +} diff --git a/engine/hatchery/openstack/spawn_test.go b/engine/hatchery/openstack/spawn_test.go new file mode 100644 index 0000000000..41a11d4a27 --- /dev/null +++ b/engine/hatchery/openstack/spawn_test.go @@ -0,0 +1,163 @@ +package openstack + +import ( + "context" + "testing" + + "github.com/gophercloud/gophercloud/openstack/compute/v2/flavors" + "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ovh/cds/sdk" + "github.com/ovh/cds/sdk/log" +) + +func TestHatcheryOpenstack_checkSpawnLimits_MaxWorker(t *testing.T) { + log.SetLogger(t) + + h := &HatcheryOpenstack{} + h.Config.Provision.MaxWorker = 3 + h.flavors = []flavors.Flavor{ + {Name: "my-flavor", VCPUs: 2}, + } + + m := sdk.Model{ + ID: 1, + Name: "my-model", + Group: &sdk.Group{ID: 1, Name: "my-group"}, + ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "my-flavor"}, + } + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 16}}, + } + + err := h.checkSpawnLimits(context.TODO(), m) + require.NoError(t, err) + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 16}}, + {Flavor: map[string]interface{}{"vcpus": 32}}, + } + + err = h.checkSpawnLimits(context.TODO(), m) + require.Error(t, err) + assert.Contains(t, err.Error(), "MaxWorker") +} + +func TestHatcheryOpenstack_checkSpawnLimits_MaxCPUs(t *testing.T) { + log.SetLogger(t) + + h := &HatcheryOpenstack{} + h.Config.Provision.MaxWorker = 10 + h.Config.MaxCPUs = 6 + h.flavors = []flavors.Flavor{ + {Name: "my-flavor", VCPUs: 2}, + } + + m := sdk.Model{ + ID: 1, + Name: "my-model", + Group: &sdk.Group{ID: 1, Name: "my-group"}, + ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "my-flavor"}, + } + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 2}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + } + + err := h.checkSpawnLimits(context.TODO(), m) + require.NoError(t, err) + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 2}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + } + + err = h.checkSpawnLimits(context.TODO(), m) + require.Error(t, err) + assert.Contains(t, err.Error(), "MaxCPUs") +} + +func TestHatcheryOpenstack_checkSpawnLimits_CountSmallerFlavorToKeep(t *testing.T) { + log.SetLogger(t) + + h := &HatcheryOpenstack{} + h.Config.Provision.MaxWorker = 10 + h.Config.MaxCPUs = 30 + h.Config.CountSmallerFlavorToKeep = 2 + h.flavors = []flavors.Flavor{ + {ID: "1", Name: "b2-7", VCPUs: 2}, + {ID: "3", Name: "b2-30", VCPUs: 8}, + {ID: "2", Name: "b2-15", VCPUs: 4}, + } + + m1 := sdk.Model{ + ID: 1, + Name: "my-model-1", + Group: &sdk.Group{ID: 1, Name: "my-group"}, + ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "b2-7"}, + } + m2 := sdk.Model{ + ID: 2, + Name: "my-model-2", + Group: &sdk.Group{ID: 1, Name: "my-group"}, + ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "b2-15"}, + } + m3 := sdk.Model{ + ID: 3, + Name: "my-model-3", + Group: &sdk.Group{ID: 1, Name: "my-group"}, + ModelVirtualMachine: sdk.ModelVirtualMachine{Flavor: "b2-30"}, + } + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 8}}, + } + + err := h.checkSpawnLimits(context.TODO(), m3) + require.NoError(t, err, "22 CPUs left (30-8) should be enough to start 8 CPUs flavor (8+4*2=16)") + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 8}}, + } + + err = h.checkSpawnLimits(context.TODO(), m3) + require.Error(t, err, "14 CPUs left (30-8*2) should be not be enough to start 8 CPUs flavor (8+4*2=16)") + assert.Contains(t, err.Error(), "CountSmallerFlavorToKeep") + + err = h.checkSpawnLimits(context.TODO(), m2) + require.NoError(t, err, "14 CPUs left (30-8*2) should be enough to start 4 CPUs flavor (4+2*2=8)") + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 4}}, + {Flavor: map[string]interface{}{"vcpus": 4}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + } + + err = h.checkSpawnLimits(context.TODO(), m1) + require.NoError(t, err, "2 CPUs left (30-8*2-4*2-2*2) should be enough to start the smallest flavor with 2 CPUs") + + lservers.list = []servers.Server{ + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 8}}, + {Flavor: map[string]interface{}{"vcpus": 4}}, + {Flavor: map[string]interface{}{"vcpus": 4}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + {Flavor: map[string]interface{}{"vcpus": 2}}, + } + + err = h.checkSpawnLimits(context.TODO(), m1) + require.Error(t, err, "0 CPUs left to start new flavor") + assert.Contains(t, err.Error(), "CountSmallerFlavorToKeep") +} diff --git a/go.mod b/go.mod index 167e985439..3c1cc786b0 100644 --- a/go.mod +++ b/go.mod @@ -209,7 +209,7 @@ require ( gopkg.in/stomp.v1 v1.0.1 // indirect gopkg.in/vmihailenco/msgpack.v2 v2.9.1 // indirect gopkg.in/yaml.v2 v2.2.4 - gotest.tools v2.1.0+incompatible // indirect + gotest.tools v2.1.0+incompatible k8s.io/api v0.0.0-20181204000039-89a74a8d264d k8s.io/apimachinery v0.0.0-20190223094358-dcb391cde5ca k8s.io/client-go v10.0.0+incompatible