From 07b382c82d7d834e801cf9dc85e2ed5ffcd7cd91 Mon Sep 17 00:00:00 2001 From: radhika Date: Wed, 14 Oct 2015 13:43:54 -0400 Subject: [PATCH] 7167: loadConfig setupKeepclient do only one set at a time. --- 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(-) diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go index fae26c1019..1cce0a7fc9 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 0d6c3b3937..68153b8b51 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 d3afb8df81..4cd9f544f7 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 8fabb308f7..0791d3cf85 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 f8a60a7e74..d90d2ad1a7 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 9f14a9e5c0..ec6bf74983 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 a77ab21afe..9a6480e864 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) } -- 2.30.2