[ARVADOS] updated: 1.3.0-200-ga27b2bf3e

Git user git at public.curoverse.com
Mon Jan 28 09:45:06 EST 2019


Summary of changes:
 lib/dispatchcloud/container/queue_test.go     |   5 +-
 lib/dispatchcloud/dispatcher_test.go          |  14 +--
 lib/dispatchcloud/scheduler/run_queue_test.go |   9 +-
 lib/dispatchcloud/test/lame_instance_set.go   | 118 --------------------------
 lib/dispatchcloud/test/logger.go              |  19 +++++
 lib/dispatchcloud/test/stub_driver.go         |  25 +++++-
 lib/dispatchcloud/worker/pool_test.go         |  23 +++--
 lib/dispatchcloud/worker/worker_test.go       |   3 +-
 8 files changed, 63 insertions(+), 153 deletions(-)
 delete mode 100644 lib/dispatchcloud/test/lame_instance_set.go
 create mode 100644 lib/dispatchcloud/test/logger.go

       via  a27b2bf3e33a80213a42dcf1e01144209eb2603a (commit)
       via  5fbcb53ad80972cbe0d8c46ff821ccd32c56c11f (commit)
      from  b105602902e38f18a48505e2091ffea77b2c7c89 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit a27b2bf3e33a80213a42dcf1e01144209eb2603a
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jan 28 02:26:49 2019 -0500

    14325: Clean up test suite logging.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/dispatchcloud/container/queue_test.go b/lib/dispatchcloud/container/queue_test.go
index a84497424..75326921f 100644
--- a/lib/dispatchcloud/container/queue_test.go
+++ b/lib/dispatchcloud/container/queue_test.go
@@ -12,7 +12,6 @@ import (
 
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
-	"github.com/sirupsen/logrus"
 	check "gopkg.in/check.v1"
 )
 
@@ -36,7 +35,7 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
 	}
 
 	client := arvados.NewClientFromEnv()
-	cq := NewQueue(logrus.StandardLogger(), nil, typeChooser, client)
+	cq := NewQueue(test.Logger(), nil, typeChooser, client)
 
 	err := cq.Update()
 	c.Check(err, check.IsNil)
@@ -93,7 +92,7 @@ func (suite *IntegrationSuite) TestCancelIfNoInstanceType(c *check.C) {
 	}
 
 	client := arvados.NewClientFromEnv()
-	cq := NewQueue(logrus.StandardLogger(), nil, errorTypeChooser, client)
+	cq := NewQueue(test.Logger(), nil, errorTypeChooser, client)
 
 	var ctr arvados.Container
 	err := client.RequestAndDecode(&ctr, "GET", "arvados/v1/containers/"+arvadostest.QueuedContainerUUID, nil, nil)
diff --git a/lib/dispatchcloud/dispatcher_test.go b/lib/dispatchcloud/dispatcher_test.go
index 5525dacd4..674cacd56 100644
--- a/lib/dispatchcloud/dispatcher_test.go
+++ b/lib/dispatchcloud/dispatcher_test.go
@@ -16,7 +16,6 @@ import (
 
 	"git.curoverse.com/arvados.git/lib/dispatchcloud/test"
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
-	"github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
 	check "gopkg.in/check.v1"
 )
@@ -29,12 +28,6 @@ type DispatcherSuite struct {
 	disp       *dispatcher
 }
 
-func (s *DispatcherSuite) SetUpSuite(c *check.C) {
-	if os.Getenv("ARVADOS_DEBUG") != "" {
-		logrus.StandardLogger().SetLevel(logrus.DebugLevel)
-	}
-}
-
 func (s *DispatcherSuite) SetUpTest(c *check.C) {
 	dispatchpub, _ := test.LoadTestKey(c, "test/sshkey_dispatch")
 	dispatchprivraw, err := ioutil.ReadFile("test/sshkey_dispatch")
diff --git a/lib/dispatchcloud/scheduler/run_queue_test.go b/lib/dispatchcloud/scheduler/run_queue_test.go
index 090a53435..7dd6866c0 100644
--- a/lib/dispatchcloud/scheduler/run_queue_test.go
+++ b/lib/dispatchcloud/scheduler/run_queue_test.go
@@ -11,13 +11,10 @@ import (
 	"git.curoverse.com/arvados.git/lib/dispatchcloud/test"
 	"git.curoverse.com/arvados.git/lib/dispatchcloud/worker"
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
-	"github.com/sirupsen/logrus"
 	check "gopkg.in/check.v1"
 )
 
 var (
-	logger = logrus.StandardLogger()
-
 	// arbitrary example container UUIDs
 	uuids = func() (r []string) {
 		for i := 0; i < 16; i++ {
@@ -177,7 +174,7 @@ func (*SchedulerSuite) TestUseIdleWorkers(c *check.C) {
 		running:   map[string]time.Time{},
 		canCreate: 0,
 	}
-	New(logger, &queue, &pool, time.Millisecond, time.Millisecond).runQueue()
+	New(test.Logger(), &queue, &pool, time.Millisecond, time.Millisecond).runQueue()
 	c.Check(pool.creates, check.DeepEquals, []arvados.InstanceType{test.InstanceType(1)})
 	c.Check(pool.starts, check.DeepEquals, []string{test.ContainerUUID(4)})
 	c.Check(pool.running, check.HasLen, 1)
@@ -232,7 +229,7 @@ func (*SchedulerSuite) TestShutdownAtQuota(c *check.C) {
 			starts:    []string{},
 			canCreate: 0,
 		}
-		New(logger, &queue, &pool, time.Millisecond, time.Millisecond).runQueue()
+		New(test.Logger(), &queue, &pool, time.Millisecond, time.Millisecond).runQueue()
 		c.Check(pool.creates, check.DeepEquals, shouldCreate)
 		c.Check(pool.starts, check.DeepEquals, []string{})
 		c.Check(pool.shutdowns, check.Not(check.Equals), 0)
@@ -320,7 +317,7 @@ func (*SchedulerSuite) TestStartWhileCreating(c *check.C) {
 		},
 	}
 	queue.Update()
-	New(logger, &queue, &pool, time.Millisecond, time.Millisecond).runQueue()
+	New(test.Logger(), &queue, &pool, time.Millisecond, time.Millisecond).runQueue()
 	c.Check(pool.creates, check.DeepEquals, []arvados.InstanceType{test.InstanceType(2), test.InstanceType(1)})
 	c.Check(pool.starts, check.DeepEquals, []string{uuids[6], uuids[5], uuids[3], uuids[2]})
 	running := map[string]bool{}
diff --git a/lib/dispatchcloud/test/logger.go b/lib/dispatchcloud/test/logger.go
new file mode 100644
index 000000000..a59eeb689
--- /dev/null
+++ b/lib/dispatchcloud/test/logger.go
@@ -0,0 +1,19 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package test
+
+import (
+	"os"
+
+	"github.com/sirupsen/logrus"
+)
+
+func Logger() logrus.FieldLogger {
+	logger := logrus.StandardLogger()
+	if os.Getenv("ARVADOS_DEBUG") != "" {
+		logger.SetLevel(logrus.DebugLevel)
+	}
+	return logger
+}
diff --git a/lib/dispatchcloud/worker/pool_test.go b/lib/dispatchcloud/worker/pool_test.go
index 928d4e902..3b66eeb41 100644
--- a/lib/dispatchcloud/worker/pool_test.go
+++ b/lib/dispatchcloud/worker/pool_test.go
@@ -13,7 +13,6 @@ import (
 	"git.curoverse.com/arvados.git/lib/dispatchcloud/test"
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/sirupsen/logrus"
 	check "gopkg.in/check.v1"
 )
 
@@ -33,10 +32,6 @@ var less = &lessChecker{&check.CheckerInfo{Name: "less", Params: []string{"obtai
 
 type PoolSuite struct{}
 
-func (suite *PoolSuite) SetUpSuite(c *check.C) {
-	logrus.StandardLogger().SetLevel(logrus.DebugLevel)
-}
-
 func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 	type1 := test.InstanceType(1)
 	type2 := test.InstanceType(2)
@@ -67,7 +62,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 		}
 	}
 
-	logger := logrus.StandardLogger()
+	logger := test.Logger()
 	driver := &test.StubDriver{}
 	is, err := driver.InstanceSet(nil, "", logger)
 	c.Assert(err, check.IsNil)
@@ -129,7 +124,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 }
 
 func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
-	logger := logrus.StandardLogger()
+	logger := test.Logger()
 	driver := test.StubDriver{HoldCloudOps: true}
 	instanceSet, err := driver.InstanceSet(nil, "", logger)
 	c.Assert(err, check.IsNil)
@@ -138,7 +133,7 @@ func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
 	type2 := arvados.InstanceType{Name: "a2m", ProviderType: "a2.medium", VCPUs: 2, RAM: 2 * GiB, Price: .02}
 	type3 := arvados.InstanceType{Name: "a2l", ProviderType: "a2.large", VCPUs: 4, RAM: 4 * GiB, Price: .04}
 	pool := &Pool{
-		logger:      logrus.StandardLogger(),
+		logger:      logger,
 		newExecutor: func(cloud.Instance) Executor { return stubExecutor{} },
 		instanceSet: &throttledInstanceSet{InstanceSet: instanceSet},
 		instanceTypes: arvados.InstanceTypeMap{
diff --git a/lib/dispatchcloud/worker/worker_test.go b/lib/dispatchcloud/worker/worker_test.go
index 570d3b5a9..2eb5255b8 100644
--- a/lib/dispatchcloud/worker/worker_test.go
+++ b/lib/dispatchcloud/worker/worker_test.go
@@ -12,7 +12,6 @@ import (
 	"git.curoverse.com/arvados.git/lib/cloud"
 	"git.curoverse.com/arvados.git/lib/dispatchcloud/test"
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
-	"github.com/sirupsen/logrus"
 	check "gopkg.in/check.v1"
 )
 
@@ -21,7 +20,7 @@ var _ = check.Suite(&WorkerSuite{})
 type WorkerSuite struct{}
 
 func (suite *WorkerSuite) TestProbeAndUpdate(c *check.C) {
-	logger := logrus.StandardLogger()
+	logger := test.Logger()
 	bootTimeout := time.Minute
 	probeTimeout := time.Second
 

commit 5fbcb53ad80972cbe0d8c46ff821ccd32c56c11f
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jan 28 02:19:27 2019 -0500

    14325: Change remaining LameInstanceSet uses to StubDriver.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/dispatchcloud/dispatcher_test.go b/lib/dispatchcloud/dispatcher_test.go
index 9ef170e46..5525dacd4 100644
--- a/lib/dispatchcloud/dispatcher_test.go
+++ b/lib/dispatchcloud/dispatcher_test.go
@@ -24,10 +24,9 @@ import (
 var _ = check.Suite(&DispatcherSuite{})
 
 type DispatcherSuite struct {
-	cluster     *arvados.Cluster
-	instanceSet *test.LameInstanceSet
-	stubDriver  *test.StubDriver
-	disp        *dispatcher
+	cluster    *arvados.Cluster
+	stubDriver *test.StubDriver
+	disp       *dispatcher
 }
 
 func (s *DispatcherSuite) SetUpSuite(c *check.C) {
diff --git a/lib/dispatchcloud/test/lame_instance_set.go b/lib/dispatchcloud/test/lame_instance_set.go
deleted file mode 100644
index baab407a7..000000000
--- a/lib/dispatchcloud/test/lame_instance_set.go
+++ /dev/null
@@ -1,118 +0,0 @@
-// Copyright (C) The Arvados Authors. All rights reserved.
-//
-// SPDX-License-Identifier: AGPL-3.0
-
-package test
-
-import (
-	"fmt"
-	"math/rand"
-	"sync"
-
-	"git.curoverse.com/arvados.git/lib/cloud"
-	"git.curoverse.com/arvados.git/sdk/go/arvados"
-	"golang.org/x/crypto/ssh"
-)
-
-// LameInstanceSet creates instances that boot but can't run
-// containers.
-type LameInstanceSet struct {
-	Hold chan bool // set to make(chan bool) to hold operations until Release is called
-
-	mtx       sync.Mutex
-	instances map[*lameInstance]bool
-}
-
-// Create returns a new instance.
-func (p *LameInstanceSet) Create(instType arvados.InstanceType, imageID cloud.ImageID, tags cloud.InstanceTags, pubkey ssh.PublicKey) (cloud.Instance, error) {
-	inst := &lameInstance{
-		p:            p,
-		id:           cloud.InstanceID(fmt.Sprintf("lame-%x", rand.Uint64())),
-		providerType: instType.ProviderType,
-	}
-	inst.SetTags(tags)
-	if p.Hold != nil {
-		p.Hold <- true
-	}
-	p.mtx.Lock()
-	defer p.mtx.Unlock()
-	if p.instances == nil {
-		p.instances = map[*lameInstance]bool{}
-	}
-	p.instances[inst] = true
-	return inst, nil
-}
-
-// Instances returns the instances that haven't been destroyed.
-func (p *LameInstanceSet) Instances(cloud.InstanceTags) ([]cloud.Instance, error) {
-	p.mtx.Lock()
-	defer p.mtx.Unlock()
-	var instances []cloud.Instance
-	for i := range p.instances {
-		instances = append(instances, i)
-	}
-	return instances, nil
-}
-
-// Stop is a no-op, but exists to satisfy cloud.InstanceSet.
-func (p *LameInstanceSet) Stop() {
-}
-
-// Release n held calls. Blocks if n calls aren't already
-// waiting. Blocks forever if Hold is nil.
-func (p *LameInstanceSet) Release(n int) {
-	for i := 0; i < n; i++ {
-		<-p.Hold
-	}
-}
-
-type lameInstance struct {
-	p            *LameInstanceSet
-	id           cloud.InstanceID
-	providerType string
-	tags         cloud.InstanceTags
-}
-
-func (inst *lameInstance) ID() cloud.InstanceID {
-	return inst.id
-}
-
-func (inst *lameInstance) String() string {
-	return fmt.Sprint(inst.id)
-}
-
-func (inst *lameInstance) ProviderType() string {
-	return inst.providerType
-}
-
-func (inst *lameInstance) Address() string {
-	return "0.0.0.0:1234"
-}
-
-func (inst *lameInstance) SetTags(tags cloud.InstanceTags) error {
-	inst.p.mtx.Lock()
-	defer inst.p.mtx.Unlock()
-	inst.tags = cloud.InstanceTags{}
-	for k, v := range tags {
-		inst.tags[k] = v
-	}
-	return nil
-}
-
-func (inst *lameInstance) Destroy() error {
-	if inst.p.Hold != nil {
-		inst.p.Hold <- true
-	}
-	inst.p.mtx.Lock()
-	defer inst.p.mtx.Unlock()
-	delete(inst.p.instances, inst)
-	return nil
-}
-
-func (inst *lameInstance) Tags() cloud.InstanceTags {
-	return inst.tags
-}
-
-func (inst *lameInstance) VerifyHostKey(ssh.PublicKey, *ssh.Client) error {
-	return nil
-}
diff --git a/lib/dispatchcloud/test/stub_driver.go b/lib/dispatchcloud/test/stub_driver.go
index 2277a3f23..a2231673f 100644
--- a/lib/dispatchcloud/test/stub_driver.go
+++ b/lib/dispatchcloud/test/stub_driver.go
@@ -46,11 +46,19 @@ type StubDriver struct {
 	MinTimeBetweenCreateCalls    time.Duration
 	MinTimeBetweenInstancesCalls time.Duration
 
+	// If true, Create and Destroy calls block until Release() is
+	// called.
+	HoldCloudOps bool
+
 	instanceSets []*StubInstanceSet
+	holdCloudOps chan bool
 }
 
 // InstanceSet returns a new *StubInstanceSet.
 func (sd *StubDriver) InstanceSet(params map[string]interface{}, id cloud.InstanceSetID, logger logrus.FieldLogger) (cloud.InstanceSet, error) {
+	if sd.holdCloudOps == nil {
+		sd.holdCloudOps = make(chan bool)
+	}
 	sis := StubInstanceSet{
 		driver:  sd,
 		servers: map[cloud.InstanceID]*StubVM{},
@@ -66,6 +74,15 @@ func (sd *StubDriver) InstanceSets() []*StubInstanceSet {
 	return sd.instanceSets
 }
 
+// ReleaseCloudOps releases n pending Create/Destroy calls. If there
+// are fewer than n blocked calls pending, it waits for the rest to
+// arrive.
+func (sd *StubDriver) ReleaseCloudOps(n int) {
+	for i := 0; i < n; i++ {
+		<-sd.holdCloudOps
+	}
+}
+
 type StubInstanceSet struct {
 	driver  *StubDriver
 	servers map[cloud.InstanceID]*StubVM
@@ -77,6 +94,9 @@ type StubInstanceSet struct {
 }
 
 func (sis *StubInstanceSet) Create(it arvados.InstanceType, image cloud.ImageID, tags cloud.InstanceTags, authKey ssh.PublicKey) (cloud.Instance, error) {
+	if sis.driver.HoldCloudOps {
+		sis.driver.holdCloudOps <- true
+	}
 	sis.mtx.Lock()
 	defer sis.mtx.Unlock()
 	if sis.stopped {
@@ -295,11 +315,14 @@ func (si stubInstance) Address() string {
 }
 
 func (si stubInstance) Destroy() error {
+	sis := si.svm.sis
+	if sis.driver.HoldCloudOps {
+		sis.driver.holdCloudOps <- true
+	}
 	if math_rand.Float64() < si.svm.sis.driver.ErrorRateDestroy {
 		return errors.New("instance could not be destroyed")
 	}
 	si.svm.SSHService.Close()
-	sis := si.svm.sis
 	sis.mtx.Lock()
 	defer sis.mtx.Unlock()
 	delete(sis.servers, si.svm.id)
diff --git a/lib/dispatchcloud/worker/pool_test.go b/lib/dispatchcloud/worker/pool_test.go
index 60e21b716..928d4e902 100644
--- a/lib/dispatchcloud/worker/pool_test.go
+++ b/lib/dispatchcloud/worker/pool_test.go
@@ -129,14 +129,18 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 }
 
 func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
-	lameInstanceSet := &test.LameInstanceSet{Hold: make(chan bool)}
+	logger := logrus.StandardLogger()
+	driver := test.StubDriver{HoldCloudOps: true}
+	instanceSet, err := driver.InstanceSet(nil, "", logger)
+	c.Assert(err, check.IsNil)
+
 	type1 := arvados.InstanceType{Name: "a1s", ProviderType: "a1.small", VCPUs: 1, RAM: 1 * GiB, Price: .01}
 	type2 := arvados.InstanceType{Name: "a2m", ProviderType: "a2.medium", VCPUs: 2, RAM: 2 * GiB, Price: .02}
 	type3 := arvados.InstanceType{Name: "a2l", ProviderType: "a2.large", VCPUs: 4, RAM: 4 * GiB, Price: .04}
 	pool := &Pool{
 		logger:      logrus.StandardLogger(),
 		newExecutor: func(cloud.Instance) Executor { return stubExecutor{} },
-		instanceSet: &throttledInstanceSet{InstanceSet: lameInstanceSet},
+		instanceSet: &throttledInstanceSet{InstanceSet: instanceSet},
 		instanceTypes: arvados.InstanceTypeMap{
 			type1.Name: type1,
 			type2.Name: type2,
@@ -160,7 +164,7 @@ func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
 	c.Check(pool.Unallocated()[type3], check.Equals, 1)
 
 	// Unblock the pending Create calls.
-	go lameInstanceSet.Release(4)
+	go driver.ReleaseCloudOps(4)
 
 	// Wait for each instance to either return from its Create
 	// call, or show up in a poll.
@@ -174,7 +178,7 @@ func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
 	ivs := suite.instancesByType(pool, type3)
 	c.Assert(ivs, check.HasLen, 1)
 	type3instanceID := ivs[0].Instance
-	err := pool.SetIdleBehavior(type3instanceID, IdleBehaviorHold)
+	err = pool.SetIdleBehavior(type3instanceID, IdleBehaviorHold)
 	c.Check(err, check.IsNil)
 
 	// Check admin-hold behavior: refuse to shutdown, and don't
@@ -240,7 +244,7 @@ func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
 	// if a node still appears in the provider list after a
 	// previous attempt, so there might be more than 4 Destroy
 	// calls to unblock.
-	go lameInstanceSet.Release(4444)
+	go driver.ReleaseCloudOps(4444)
 
 	// Sync until all instances disappear from the provider list.
 	suite.wait(c, pool, notify, func() bool {

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list