[ARVADOS] updated: 07b382c82d7d834e801cf9dc85e2ed5ffcd7cd91

git at public.curoverse.com git at public.curoverse.com
Wed Oct 14 13:44:50 EDT 2015


Summary of changes:
 sdk/go/arvadosclient/arvadosclient.go |  12 ----
 sdk/go/arvadostest/run_servers.go     |  13 +++-
 sdk/go/keepclient/keepclient.go       |  18 ++---
 sdk/go/keepclient/support.go          |   4 +-
 sdk/python/tests/run_test_server.py   |  18 ++---
 tools/keep-rsync/keep-rsync.go        | 122 ++++++++++++++++------------------
 tools/keep-rsync/keep-rsync_test.go   |  49 +++++---------
 7 files changed, 96 insertions(+), 140 deletions(-)

       via  07b382c82d7d834e801cf9dc85e2ed5ffcd7cd91 (commit)
      from  bc816b50fc16182fef2f5d17ffd61578432e83c3 (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 07b382c82d7d834e801cf9dc85e2ed5ffcd7cd91
Author: radhika <radhika at curoverse.com>
Date:   Wed Oct 14 13:43:54 2015 -0400

    7167: loadConfig setupKeepclient do only one set at a time.

diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index fae26c1..1cce0a7 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -78,18 +78,6 @@ type ArvadosClient struct {
 	DiscoveryDoc Dict
 }
 
-// APIConfig struct consists of:
-//    APIToken        string
-//    APIHost         string
-//    APIHostInsecure bool
-//    ExternalClient  bool
-type APIConfig struct {
-	APIToken        string
-	APIHost         string
-	APIHostInsecure bool
-	ExternalClient  bool
-}
-
 // Create a new ArvadosClient, initialized with standard Arvados environment
 // variables ARVADOS_API_HOST, ARVADOS_API_TOKEN, and (optionally)
 // ARVADOS_API_HOST_INSECURE.
diff --git a/sdk/go/arvadostest/run_servers.go b/sdk/go/arvadostest/run_servers.go
index 0d6c3b3..68153b8 100644
--- a/sdk/go/arvadostest/run_servers.go
+++ b/sdk/go/arvadostest/run_servers.go
@@ -133,9 +133,20 @@ func StartKeepWithParams(numKeepServers int, enforcePermissions bool) {
 }
 
 func StopKeep() {
+	StopKeepServers(2)
+}
+
+// StopKeepServers is used to stop keep servers while specifying numKeepServers
+func StopKeepServers(numKeepServers int) {
 	cwd, _ := os.Getwd()
 	defer os.Chdir(cwd)
 	chdirToPythonTests()
 
-	exec.Command("python", "run_test_server.py", "stop_keep").Run()
+	cmdArgs := []string{"run_test_server.py", "stop_keep"}
+
+	if numKeepServers != 2 {
+		cmdArgs = append(cmdArgs, "--num-keep-servers", strconv.Itoa(numKeepServers))
+	}
+
+	exec.Command("python", cmdArgs...)
 }
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index d3afb8d..4cd9f54 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -13,7 +13,6 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
-	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -56,26 +55,19 @@ type KeepClient struct {
 
 // MakeKeepClient creates a new KeepClient by contacting the API server to discover Keep servers.
 func MakeKeepClient(arv *arvadosclient.ArvadosClient) (*KeepClient, error) {
-	kc := initKeepClient(arv)
+	kc := New(arv)
 	return kc, kc.DiscoverKeepServers()
 }
 
-// MakeKeepClientFromJSON creates a new KeepClient using the given json to load keep servers.
-func MakeKeepClientFromJSON(arv *arvadosclient.ArvadosClient, svcJSON string) (*KeepClient, error) {
-	kc := initKeepClient(arv)
-	return kc, kc.DiscoverKeepServersFromJSON(svcJSON)
-}
-
-// Make a new KeepClient struct.
-func initKeepClient(arv *arvadosclient.ArvadosClient) *KeepClient {
-	var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
-	insecure := matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
+// New func creates a new KeepClient struct.
+// This func does not discover keep servers. It is the caller's responsibility.
+func New(arv *arvadosclient.ArvadosClient) *KeepClient {
 	kc := &KeepClient{
 		Arvados:       arv,
 		Want_replicas: 2,
 		Using_proxy:   false,
 		Client: &http.Client{Transport: &http.Transport{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}}},
+			TLSClientConfig: &tls.Config{InsecureSkipVerify: arv.ApiInsecure}}},
 	}
 	return kc
 }
diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go
index 8fabb30..0791d3c 100644
--- a/sdk/go/keepclient/support.go
+++ b/sdk/go/keepclient/support.go
@@ -94,8 +94,8 @@ func (this *KeepClient) DiscoverKeepServers() error {
 	return this.loadKeepServers(list)
 }
 
-// DiscoverKeepServersFromJSON gets list of available keep services from given JSON
-func (this *KeepClient) DiscoverKeepServersFromJSON(services string) error {
+// LoadKeepServicesFromJSON gets list of available keep services from given JSON
+func (this *KeepClient) LoadKeepServicesFromJSON(services string) error {
 	var list svcList
 
 	// Load keep services from given json
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index f8a60a7..d90d2ad 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -324,7 +324,7 @@ def _start_keep(n, keep_args):
     return port
 
 def run_keep(blob_signing_key=None, enforce_permissions=False, num_servers=2):
-    stop_keep()
+    stop_keep(num_servers)
 
     keep_args = {}
     if not blob_signing_key:
@@ -372,11 +372,9 @@ def _stop_keep(n):
     if os.path.exists(os.path.join(TEST_TMPDIR, "keep.blob_signing_key")):
         os.remove(os.path.join(TEST_TMPDIR, "keep.blob_signing_key"))
 
-def stop_keep():
-    _stop_keep(0)
-    _stop_keep(1)
-    # We may have created an additional keep server for keep-rsync testing
-    _stop_keep(2)
+def stop_keep(num_servers=2):
+    for n in range(0, num_servers):
+        _stop_keep(n)
 
 def run_keep_proxy():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
@@ -598,15 +596,11 @@ if __name__ == "__main__":
     parser = argparse.ArgumentParser()
     parser.add_argument('action', type=str, help="one of {}".format(actions))
     parser.add_argument('--auth', type=str, metavar='FIXTURE_NAME', help='Print authorization info for given api_client_authorizations fixture')
-    parser.add_argument('--num-keep-servers', metavar='int', type=int, help="Number of keep servers desired; default is 2 keep servers")
+    parser.add_argument('--num-keep-servers', metavar='int', type=int, default=2, help="Number of keep servers desired")
     parser.add_argument('--keep-enforce-permissions', action="store_true", help="Enforce keep permissions")
 
     args = parser.parse_args()
 
-    num_servers = 2
-    if args.num_keep_servers is not None:
-        num_servers = args.num_keep_servers
-
     if args.action not in actions:
         print("Unrecognized action '{}'. Actions are: {}.".format(args.action, actions), file=sys.stderr)
         sys.exit(1)
@@ -624,7 +618,7 @@ if __name__ == "__main__":
     elif args.action == 'stop':
         stop(force=('ARVADOS_TEST_API_HOST' not in os.environ))
     elif args.action == 'start_keep':
-        run_keep(enforce_permissions=args.keep_enforce_permissions, num_servers=num_servers)
+        run_keep(enforce_permissions=args.keep_enforce_permissions, num_servers=args.num_keep_servers)
     elif args.action == 'stop_keep':
         stop_keep()
     elif args.action == 'start_keep_proxy':
diff --git a/tools/keep-rsync/keep-rsync.go b/tools/keep-rsync/keep-rsync.go
index 9f14a9e..ec6bf74 100644
--- a/tools/keep-rsync/keep-rsync.go
+++ b/tools/keep-rsync/keep-rsync.go
@@ -56,7 +56,8 @@ func main() {
 		&replications,
 		"replications",
 		0,
-		"Number of replications to write to the destination.")
+		"Number of replications to write to the destination. If replications not specified, "+
+			"default replication level configured on destination server will be used.")
 
 	flag.StringVar(
 		&prefix,
@@ -66,15 +67,25 @@ func main() {
 
 	flag.Parse()
 
-	srcConfig, dstConfig, srcBlobSigningKey, _, err := loadConfig(srcConfigFile, dstConfigFile)
+	srcConfig, srcBlobSigningKey, err := loadConfig(srcConfigFile)
 	if err != nil {
-		log.Fatalf("Error loading configuration from files: %s", err.Error())
+		log.Fatalf("Error loading src configuration from file: %s", err.Error())
+	}
+
+	dstConfig, _, err := loadConfig(dstConfigFile)
+	if err != nil {
+		log.Fatalf("Error loading dst configuration from file: %s", err.Error())
 	}
 
 	// setup src and dst keepclients
-	kcSrc, kcDst, err := setupKeepClients(srcConfig, dstConfig, srcKeepServicesJSON, dstKeepServicesJSON, replications)
+	kcSrc, err := setupKeepClient(srcConfig, srcKeepServicesJSON, false, 0)
+	if err != nil {
+		log.Fatalf("Error configuring src keepclient: %s", err.Error())
+	}
+
+	kcDst, err := setupKeepClient(dstConfig, dstKeepServicesJSON, true, replications)
 	if err != nil {
-		log.Fatalf("Error configuring keep-rsync: %s", err.Error())
+		log.Fatalf("Error configuring dst keepclient: %s", err.Error())
 	}
 
 	// Copy blocks not found in dst from src
@@ -84,23 +95,22 @@ func main() {
 	}
 }
 
-// Load src and dst config from given files
-func loadConfig(srcConfigFile, dstConfigFile string) (srcConfig, dstConfig arvadosclient.APIConfig, srcBlobSigningKey, dstBlobSigningKey string, err error) {
-	if srcConfigFile == "" {
-		return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, errors.New("-src-config-file must be specified")
-	}
+type apiConfig struct {
+	APIToken        string
+	APIHost         string
+	APIHostInsecure bool
+	ExternalClient  bool
+}
 
-	srcConfig, srcBlobSigningKey, err = readConfigFromFile(srcConfigFile)
-	if err != nil {
-		return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, fmt.Errorf("Error reading source configuration: %v", err)
+// Load src and dst config from given files
+func loadConfig(configFile string) (config apiConfig, blobSigningKey string, err error) {
+	if configFile == "" {
+		return config, blobSigningKey, errors.New("config file not specified")
 	}
 
-	if dstConfigFile == "" {
-		return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, errors.New("-dst-config-file must be specified")
-	}
-	dstConfig, dstBlobSigningKey, err = readConfigFromFile(dstConfigFile)
+	config, blobSigningKey, err = readConfigFromFile(configFile)
 	if err != nil {
-		return srcConfig, dstConfig, srcBlobSigningKey, dstBlobSigningKey, fmt.Errorf("Error reading destination configuration: %v", err)
+		return config, blobSigningKey, fmt.Errorf("Error reading config file: %v", err)
 	}
 
 	return
@@ -109,7 +119,7 @@ func loadConfig(srcConfigFile, dstConfigFile string) (srcConfig, dstConfig arvad
 var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
 
 // Read config from file
-func readConfigFromFile(filename string) (config arvadosclient.APIConfig, blobSigningKey string, err error) {
+func readConfigFromFile(filename string) (config apiConfig, blobSigningKey string, err error) {
 	if !strings.Contains(filename, "/") {
 		filename = os.Getenv("HOME") + "/.config/arvados/" + filename
 	}
@@ -146,66 +156,46 @@ func readConfigFromFile(filename string) (config arvadosclient.APIConfig, blobSi
 	return
 }
 
-// Initializes keep-rsync using the config provided
-func setupKeepClients(srcConfig, dstConfig arvadosclient.APIConfig, srcKeepServicesJSON, dstKeepServicesJSON string, replications int) (kcSrc, kcDst *keepclient.KeepClient, err error) {
-	// arvSrc from srcConfig
-	arvSrc := arvadosclient.ArvadosClient{
-		ApiToken:    srcConfig.APIToken,
-		ApiServer:   srcConfig.APIHost,
-		ApiInsecure: srcConfig.APIHostInsecure,
+// setup keepclient using the config provided
+func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, replications int) (kc *keepclient.KeepClient, err error) {
+	arv := arvadosclient.ArvadosClient{
+		ApiToken:    config.APIToken,
+		ApiServer:   config.APIHost,
+		ApiInsecure: config.APIHostInsecure,
 		Client: &http.Client{Transport: &http.Transport{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: srcConfig.APIHostInsecure}}},
-		External: srcConfig.ExternalClient,
+			TLSClientConfig: &tls.Config{InsecureSkipVerify: config.APIHostInsecure}}},
+		External: config.ExternalClient,
 	}
 
-	// arvDst from dstConfig
-	arvDst := arvadosclient.ArvadosClient{
-		ApiToken:    dstConfig.APIToken,
-		ApiServer:   dstConfig.APIHost,
-		ApiInsecure: dstConfig.APIHostInsecure,
-		Client: &http.Client{Transport: &http.Transport{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: dstConfig.APIHostInsecure}}},
-		External: dstConfig.ExternalClient,
-	}
-
-	// Get default replications value from destination, if it is not already provided
-	if replications == 0 {
-		value, err := arvDst.Discovery("defaultCollectionReplication")
-		if err == nil {
-			replications = int(value.(float64))
-		} else {
-			replications = 2
-		}
-	}
-
-	// if srcKeepServicesJSON is provided, use it to load services; else, use DiscoverKeepServers
-	if srcKeepServicesJSON == "" {
-		kcSrc, err = keepclient.MakeKeepClient(&arvSrc)
+	// if keepServicesJSON is provided, use it to load services; else, use DiscoverKeepServers
+	if keepServicesJSON == "" {
+		kc, err = keepclient.MakeKeepClient(&arv)
 		if err != nil {
-			return nil, nil, err
+			return nil, err
 		}
 	} else {
-		kcSrc, err = keepclient.MakeKeepClientFromJSON(&arvSrc, srcKeepServicesJSON)
+		kc = keepclient.New(&arv)
+		err = kc.LoadKeepServicesFromJSON(keepServicesJSON)
 		if err != nil {
-			return kcSrc, kcDst, err
+			return kc, err
 		}
 	}
 
-	// if dstKeepServicesJSON is provided, use it to load services; else, use DiscoverKeepServers
-	if dstKeepServicesJSON == "" {
-		kcDst, err = keepclient.MakeKeepClient(&arvDst)
-		if err != nil {
-			return kcSrc, kcDst, err
-		}
-	} else {
-		kcDst, err = keepclient.MakeKeepClientFromJSON(&arvDst, dstKeepServicesJSON)
-		if err != nil {
-			return kcSrc, kcDst, err
+	if isDst {
+		// Get default replications value from destination, if it is not already provided
+		if replications == 0 {
+			value, err := arv.Discovery("defaultCollectionReplication")
+			if err == nil {
+				replications = int(value.(float64))
+			} else {
+				return nil, err
+			}
 		}
+
+		kc.Want_replicas = replications
 	}
-	kcDst.Want_replicas = replications
 
-	return kcSrc, kcDst, nil
+	return kc, nil
 }
 
 // Get unique block locators from src and dst
diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go
index a77ab21..9a6480e 100644
--- a/tools/keep-rsync/keep-rsync_test.go
+++ b/tools/keep-rsync/keep-rsync_test.go
@@ -9,7 +9,6 @@ import (
 	"testing"
 	"time"
 
-	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
 	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
 	"git.curoverse.com/arvados.git/sdk/go/keepclient"
 
@@ -45,7 +44,7 @@ func (s *ServerRequiredSuite) SetUpTest(c *C) {
 }
 
 func (s *ServerRequiredSuite) TearDownSuite(c *C) {
-	arvadostest.StopKeep()
+	arvadostest.StopKeepServers(3)
 	arvadostest.StopAPI()
 }
 
@@ -56,13 +55,13 @@ var testKeepServicesJSON = "{ \"kind\":\"arvados#keepServiceList\", \"etag\":\"\
 // and uses the first 2 as src and the 3rd as dst keep servers.
 func setupRsync(c *C, enforcePermissions, updateDstReplications bool, replications int) {
 	// srcConfig
-	var srcConfig arvadosclient.APIConfig
+	var srcConfig apiConfig
 	srcConfig.APIHost = os.Getenv("ARVADOS_API_HOST")
 	srcConfig.APIToken = os.Getenv("ARVADOS_API_TOKEN")
 	srcConfig.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 
 	// dstConfig
-	var dstConfig arvadosclient.APIConfig
+	var dstConfig apiConfig
 	dstConfig.APIHost = os.Getenv("ARVADOS_API_HOST")
 	dstConfig.APIToken = os.Getenv("ARVADOS_API_TOKEN")
 	dstConfig.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
@@ -77,7 +76,10 @@ func setupRsync(c *C, enforcePermissions, updateDstReplications bool, replicatio
 
 	// setup keepclients
 	var err error
-	kcSrc, kcDst, err = setupKeepClients(srcConfig, dstConfig, srcKeepServicesJSON, dstKeepServicesJSON, replications)
+	kcSrc, err = setupKeepClient(srcConfig, srcKeepServicesJSON, false, 0)
+	c.Check(err, IsNil)
+
+	kcDst, err = setupKeepClient(dstConfig, dstKeepServicesJSON, true, replications)
 	c.Check(err, IsNil)
 
 	for uuid := range kcSrc.LocalRoots() {
@@ -431,7 +433,7 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) {
 	dstConfigFile := dstFile.Name()
 
 	// load configuration from those files
-	srcConfig, dstConfig, srcBlobSigningKey, _, err := loadConfig(srcConfigFile, dstConfigFile)
+	srcConfig, srcBlobSigningKey, err := loadConfig(srcConfigFile)
 	c.Check(err, IsNil)
 
 	c.Assert(srcConfig.APIHost, Equals, "testhost")
@@ -439,6 +441,9 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) {
 	c.Assert(srcConfig.APIHostInsecure, Equals, true)
 	c.Assert(srcConfig.ExternalClient, Equals, false)
 
+	dstConfig, _, err := loadConfig(dstConfigFile)
+	c.Check(err, IsNil)
+
 	c.Assert(dstConfig.APIHost, Equals, "testhost")
 	c.Assert(dstConfig.APIToken, Equals, "testtoken")
 	c.Assert(dstConfig.APIHostInsecure, Equals, true)
@@ -449,37 +454,13 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) {
 
 // Test loadConfig func without setting up the config files
 func (s *ServerRequiredSuite) TestLoadConfig_MissingSrcConfig(c *C) {
-	_, _, _, _, err := loadConfig("", "")
-	c.Assert(err.Error(), Equals, "-src-config-file must be specified")
+	_, _, err := loadConfig("")
+	c.Assert(err.Error(), Equals, "config file not specified")
 }
 
-// Test loadConfig func - error reading src config
+// Test loadConfig func - error reading config
 func (s *ServerRequiredSuite) TestLoadConfig_ErrorLoadingSrcConfig(c *C) {
-	_, _, _, _, err := loadConfig("no-such-config-file", "")
-	c.Assert(strings.HasSuffix(err.Error(), "no such file or directory"), Equals, true)
-}
-
-// Test loadConfig func with no dst config file specified
-func (s *ServerRequiredSuite) TestLoadConfig_MissingDstConfig(c *C) {
-	// Setup a src config file
-	srcFile := setupConfigFile(c, "src-config")
-	defer os.Remove(srcFile.Name())
-	srcConfigFile := srcFile.Name()
-
-	// load configuration
-	_, _, _, _, err := loadConfig(srcConfigFile, "")
-	c.Assert(err.Error(), Equals, "-dst-config-file must be specified")
-}
-
-// Test loadConfig func
-func (s *ServerRequiredSuite) TestLoadConfig_ErrorLoadingDstConfig(c *C) {
-	// Setup a src config file
-	srcFile := setupConfigFile(c, "src-config")
-	defer os.Remove(srcFile.Name())
-	srcConfigFile := srcFile.Name()
-
-	// load configuration
-	_, _, _, _, err := loadConfig(srcConfigFile, "no-such-config-file")
+	_, _, err := loadConfig("no-such-config-file")
 	c.Assert(strings.HasSuffix(err.Error(), "no such file or directory"), Equals, true)
 }
 

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list