7167: replace the keep_existing logic and create all 3 keep servers at once and use...

diff --git a/sdk/go/arvadostest/run_servers.go b/sdk/go/arvadostest/run_servers.go
index 0b60709c62cc2bc33875f6de14a81e6e543734e2..1b7a5e02ca5c47ee016cd2f1b31e4830553faf00 100644 (file)
--- a/sdk/go/arvadostest/run_servers.go
+++ b/sdk/go/arvadostest/run_servers.go
@@ -99,21 +99,21 @@ func StopAPI() {
 }
 
 // StartKeep is used to start keep servers
-// with keepExisting = false and enforcePermissions = false
+// with needMore = false and enforcePermissions = false
 func StartKeep() {
        StartKeepWithParams(false, false)
 }
 
 // StartKeepWithParams is used to start keep servers while specifying
-// keepExisting and enforcePermissions parameters.
-func StartKeepWithParams(keepExisting bool, enforcePermissions bool) {
+// needMore and enforcePermissions parameters.
+func StartKeepWithParams(needMore bool, enforcePermissions bool) {
        cwd, _ := os.Getwd()
        defer os.Chdir(cwd)
        chdirToPythonTests()
 
        cmdArgs := []string{"run_test_server.py", "start_keep"}
-       if keepExisting {
-               cmdArgs = append(cmdArgs, "--keep-existing")
+       if needMore {
+               cmdArgs = append(cmdArgs, "--need-more")
        }
        if enforcePermissions {
                cmdArgs = append(cmdArgs, "--keep-enforce-permissions")

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 100e2ef6ced262767afde4c8f2c0f85100cd415c..5bda58bb3f60e452c7b15d3f5735dd13743761d5 100644 (file)
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -43,7 +43,6 @@ if not os.path.exists(TEST_TMPDIR):
 
 my_api_host = None
 _cached_config = {}
-keep_existing = False
 
 def find_server_pid(PID_PATH, wait=10):
     now = time.time()
@@ -324,9 +323,8 @@ def _start_keep(n, keep_args):
 
     return port
 
-def run_keep(blob_signing_key=None, enforce_permissions=False):
-    if not keep_existing:
-      stop_keep()
+def run_keep(blob_signing_key=None, enforce_permissions=False, num_keep_servers=2):
+    stop_keep()
 
     keep_args = {}
     if not blob_signing_key:
@@ -352,12 +350,7 @@ def run_keep(blob_signing_key=None, enforce_permissions=False):
     for d in api.keep_disks().list().execute()['items']:
         api.keep_disks().delete(uuid=d['uuid']).execute()
 
-    start_index = 0
-    end_index = 2
-    if keep_existing:
-        start_index = 2
-        end_index = 3
-    for d in range(start_index, end_index):
+    for d in range(0, num_keep_servers):
         port = _start_keep(d, keep_args)
         svc = api.keep_services().create(body={'keep_service': {
             'uuid': 'zzzzz-bi6l4-keepdisk{:07d}'.format(d),
@@ -382,7 +375,7 @@ def _stop_keep(n):
 def stop_keep():
     _stop_keep(0)
     _stop_keep(1)
-    # We may have created an additional keep servers when keep_existing is used
+    # We may have created an additional keep server for keep-rsync testing
     _stop_keep(2)
 
 def run_keep_proxy():
@@ -605,12 +598,14 @@ 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('--keep-existing', action="store_true", help="Used to add additional keep servers, without terminating existing servers")
+    parser.add_argument('--need-more', action="store_true", help="Used to indicate need one more than the default 2 keep servers")
     parser.add_argument('--keep-enforce-permissions', action="store_true", help="Enforce keep permissions")
 
     args = parser.parse_args()
 
-    keep_existing = args.keep_existing
+    num_keep_servers = 2
+    if args.need_more:
+        num_keep_servers = 3
 
     if args.action not in actions:
         print("Unrecognized action '{}'. Actions are: {}.".format(args.action, actions), file=sys.stderr)
@@ -629,7 +624,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)
+        run_keep(enforce_permissions=args.keep_enforce_permissions, num_keep_servers=num_keep_servers)
     elif args.action == 'stop_keep':
         stop_keep()
     elif args.action == 'start_keep_proxy':

diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go
index 0194f4c175df78d9195e90a5934b2dd35d8074d6..f722e9af8133b40723461d2331829672b2b02c66 100644 (file)
--- a/tools/keep-rsync/keep-rsync_test.go
+++ b/tools/keep-rsync/keep-rsync_test.go
@@ -52,10 +52,9 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) {
 var testKeepServicesJSON = "{ \"kind\":\"arvados#keepServiceList\", \"etag\":\"\", \"self_link\":\"\", \"offset\":null, \"limit\":null, \"items\":[ { \"href\":\"/keep_services/zzzzz-bi6l4-123456789012340\", \"kind\":\"arvados#keepService\", \"etag\":\"641234567890enhj7hzx432e5\", \"uuid\":\"zzzzz-bi6l4-123456789012340\", \"owner_uuid\":\"zzzzz-tpzed-123456789012345\", \"service_host\":\"keep0.zzzzz.arvadosapi.com\", \"service_port\":25107, \"service_ssl_flag\":false, \"service_type\":\"disk\", \"read_only\":false }, { \"href\":\"/keep_services/zzzzz-bi6l4-123456789012341\", \"kind\":\"arvados#keepService\", \"etag\":\"641234567890enhj7hzx432e5\", \"uuid\":\"zzzzz-bi6l4-123456789012341\", \"owner_uuid\":\"zzzzz-tpzed-123456789012345\", \"service_host\":\"keep0.zzzzz.arvadosapi.com\", \"service_port\":25108, \"service_ssl_flag\":false, \"service_type\":\"disk\", \"read_only\":false } ], \"items_available\":2 }"
 
 // Testing keep-rsync needs two sets of keep services: src and dst.
-// The test setup hence tweaks keep-rsync initialization to achieve this.
-// First invoke setupKeepClients and then invoke StartKeepWithParams
-// to create the keep servers to be used as destination.
-func setupRsync(c *C, enforcePermissions, setupDstServers bool, replications int) {
+// The test setup hence creates 3 servers instead of the default 2,
+// 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
        srcConfig.APIHost = os.Getenv("ARVADOS_API_HOST")
@@ -74,18 +73,46 @@ func setupRsync(c *C, enforcePermissions, setupDstServers bool, replications int
 
        // Start API and Keep servers
        arvadostest.StartAPI()
-       arvadostest.StartKeepWithParams(false, enforcePermissions)
+       arvadostest.StartKeepWithParams(true, enforcePermissions)
 
        // setup keepclients
        var err error
        kcSrc, kcDst, err = setupKeepClients(srcConfig, dstConfig, srcKeepServicesJSON, dstKeepServicesJSON, replications)
        c.Check(err, IsNil)
 
-       // Create an additional keep server to be used as destination and reload kcDst
-       if setupDstServers {
-               arvadostest.StartKeepWithParams(true, enforcePermissions)
-               kcDst, err = keepclient.MakeKeepClient(kcDst.Arvados)
-               c.Check(err, IsNil)
+       for uuid := range kcSrc.LocalRoots() {
+               if strings.HasSuffix(uuid, "02") {
+                       delete(kcSrc.LocalRoots(), uuid)
+               }
+       }
+       for uuid := range kcSrc.GatewayRoots() {
+               if strings.HasSuffix(uuid, "02") {
+                       delete(kcSrc.GatewayRoots(), uuid)
+               }
+       }
+       for uuid := range kcSrc.WritableLocalRoots() {
+               if strings.HasSuffix(uuid, "02") {
+                       delete(kcSrc.WritableLocalRoots(), uuid)
+               }
+       }
+
+       for uuid := range kcDst.LocalRoots() {
+               if strings.HasSuffix(uuid, "00") || strings.HasSuffix(uuid, "01") {
+                       delete(kcDst.LocalRoots(), uuid)
+               }
+       }
+       for uuid := range kcDst.GatewayRoots() {
+               if strings.HasSuffix(uuid, "00") || strings.HasSuffix(uuid, "01") {
+                       delete(kcDst.GatewayRoots(), uuid)
+               }
+       }
+       for uuid := range kcDst.WritableLocalRoots() {
+               if strings.HasSuffix(uuid, "00") || strings.HasSuffix(uuid, "01") {
+                       delete(kcDst.WritableLocalRoots(), uuid)
+               }
+       }
+
+       if updateDstReplications {
                kcDst.Want_replicas = replications
        }
 }