7167: loadConfig setupKeepclient do only one set at a time.
authorradhika <radhika@curoverse.com>
Wed, 14 Oct 2015 17:43:54 +0000 (13:43 -0400)
committerradhika <radhika@curoverse.com>
Wed, 14 Oct 2015 17:43:54 +0000 (13:43 -0400)
sdk/go/arvadosclient/arvadosclient.go
sdk/go/arvadostest/run_servers.go
sdk/go/keepclient/keepclient.go
sdk/go/keepclient/support.go
sdk/python/tests/run_test_server.py
tools/keep-rsync/keep-rsync.go
tools/keep-rsync/keep-rsync_test.go

index fae26c101958dfd235ca0e7129b98ad20afac593..1cce0a7fc92d24e21fa694add86c75c63952eb46 100644 (file)
@@ -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.
index 0d6c3b393753ef3eae9ec68c6a047072df1626fa..68153b8b5119e4094698a3edbb26d7284ba6dcbc 100644 (file)
@@ -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...)
 }
index d3afb8df81e62ebac43565ab77ec5d450b8e351a..4cd9f544f7d75f56f3869bc211749b22792a47e3 100644 (file)
@@ -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
 }
index 8fabb308f7be306ea4de494d2698666a20061022..0791d3cf856ee7d5d1268338eafa883fe9bcbb18 100644 (file)
@@ -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
index f8a60a7e7469b407ae502fe5259c648f4695719e..d90d2ad1a7b61dd567bc6931c95a06b8d8463226 100644 (file)
@@ -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':
index 9f14a9e5c08cd7e5670bc21e70017ede04fcb92e..ec6bf74983a447d4cba44e0c20cb919b41f7348f 100644 (file)
@@ -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
index a77ab21afe62209ef3eb255a7abe563a7db8017a..9a6480e864d6e56802f95dcb65af0db89486b315 100644 (file)
@@ -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)
 }