Skip to content

Commit dcda100

Browse files
authored
Merge pull request #3162 from s4ke/44378-moby-fix-constraint-enforcer-generic-resources
Fix HasResource inverted boolean error
2 parents 7eb5046 + d67eec8 commit dcda100

File tree

3 files changed

+155
-1
lines changed

3 files changed

+155
-1
lines changed

api/genericresource/validate.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func HasResource(res *api.GenericResource, resources []*api.GenericResource) boo
6363
return false
6464
}
6565

66-
if res.GetDiscreteResourceSpec().Value < rtype.DiscreteResourceSpec.Value {
66+
if res.GetDiscreteResourceSpec().Value > rtype.DiscreteResourceSpec.Value {
6767
return false
6868
}
6969

api/genericresource/validate_test.go

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package genericresource
2+
3+
import (
4+
"testing"
5+
6+
"github.com/moby/swarmkit/v2/api"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestHasResourceDiscreteMoreThanEnough(t *testing.T) {
11+
required := NewDiscrete("apple", 1)
12+
available := []*api.GenericResource{NewDiscrete("apple", 5)}
13+
14+
assert.True(t, HasResource(required, available))
15+
}
16+
17+
func TestHasResourceDiscreteExactlyEnough(t *testing.T) {
18+
required := NewDiscrete("apple", 5)
19+
available := []*api.GenericResource{NewDiscrete("apple", 5)}
20+
21+
assert.True(t, HasResource(required, available))
22+
}
23+
24+
func TestHasResourceDiscreteNotEnough(t *testing.T) {
25+
required := NewDiscrete("apple", 6)
26+
available := []*api.GenericResource{NewDiscrete("apple", 5)}
27+
28+
assert.False(t, HasResource(required, available))
29+
}

manager/orchestrator/constraintenforcer/constraint_enforcer_test.go

+125
Original file line numberDiff line numberDiff line change
@@ -391,3 +391,128 @@ func TestGenericResourcesPlacementConstraints(t *testing.T) {
391391
task = testutils.WatchTaskUpdate(t, watch)
392392
assert.Equal(t, api.TaskStateRejected, task.Status.State)
393393
}
394+
395+
func TestGenericResourcesPlacementConstraintsDiscrete(t *testing.T) {
396+
node := &api.Node{
397+
ID: "id0",
398+
Spec: api.NodeSpec{
399+
Annotations: api.Annotations{
400+
Name: "node1",
401+
},
402+
Availability: api.NodeAvailabilityActive,
403+
},
404+
Status: api.NodeStatus{
405+
State: api.NodeStatus_READY,
406+
},
407+
Role: api.NodeRoleWorker,
408+
Description: &api.NodeDescription{
409+
Resources: &api.Resources{
410+
Generic: []*api.GenericResource{
411+
genericresource.NewDiscrete("mygeneric", 2),
412+
},
413+
},
414+
},
415+
Attachments: []*api.NetworkAttachment{
416+
{
417+
Network: &api.Network{
418+
ID: "id1",
419+
},
420+
},
421+
},
422+
}
423+
424+
service := &api.Service{
425+
ID: "id1",
426+
Spec: api.ServiceSpec{
427+
Annotations: api.Annotations{
428+
Name: "service1",
429+
},
430+
Task: api.TaskSpec{
431+
Resources: &api.ResourceRequirements{
432+
Reservations: &api.Resources{
433+
Generic: []*api.GenericResource{
434+
genericresource.NewDiscrete("mygeneric", 2),
435+
},
436+
},
437+
},
438+
Networks: []*api.NetworkAttachmentConfig{
439+
{
440+
Target: "id1",
441+
},
442+
},
443+
},
444+
},
445+
}
446+
447+
task := &api.Task{
448+
ID: "id2",
449+
Spec: api.TaskSpec{
450+
Resources: &api.ResourceRequirements{
451+
Reservations: &api.Resources{
452+
Generic: []*api.GenericResource{
453+
genericresource.NewDiscrete("mygeneric", 2),
454+
},
455+
},
456+
},
457+
Networks: []*api.NetworkAttachmentConfig{
458+
{
459+
Target: "id1",
460+
},
461+
},
462+
},
463+
ServiceID: service.ID,
464+
NodeID: node.ID,
465+
Status: api.TaskStatus{
466+
State: api.TaskStateRunning,
467+
},
468+
DesiredState: api.TaskStateRunning,
469+
AssignedGenericResources: []*api.GenericResource{
470+
genericresource.NewDiscrete("mygeneric", 2),
471+
},
472+
}
473+
474+
s := store.NewMemoryStore(nil)
475+
require.NotNil(t, s)
476+
defer s.Close()
477+
478+
require.NoError(t, s.Update(func(tx store.Tx) error {
479+
// Prepopulate node, service, and task.
480+
for _, err := range []error{
481+
store.CreateNode(tx, node),
482+
store.CreateService(tx, service),
483+
store.CreateTask(tx, task),
484+
} {
485+
if err != nil {
486+
return err
487+
}
488+
}
489+
return nil
490+
}))
491+
492+
watch, cancel := state.Watch(s.WatchQueue(), api.EventUpdateTask{})
493+
defer cancel()
494+
495+
constraintEnforcer := New(s)
496+
defer constraintEnforcer.Stop()
497+
498+
// Update the node to remove the generic resource
499+
require.NoError(t, s.Update(func(tx store.Tx) error {
500+
node = store.GetNode(tx, node.ID)
501+
node.Description = &api.NodeDescription{
502+
Resources: &api.Resources{
503+
Generic: []*api.GenericResource{
504+
genericresource.NewDiscrete("mygeneric", 1),
505+
},
506+
NanoCPUs: 1e9,
507+
MemoryBytes: 1e9,
508+
},
509+
}
510+
return store.UpdateNode(tx, node)
511+
}))
512+
513+
go constraintEnforcer.Run()
514+
515+
// The task should be rejected immediately.
516+
task = testutils.WatchTaskUpdate(t, watch)
517+
assert.Equal(t, api.TaskStateRejected, task.Status.State)
518+
}

0 commit comments

Comments
 (0)