12018: Several changes, listed below:
authorLucas Di Pentima <ldipentima@veritasgenetics.com>
Wed, 18 Oct 2017 17:17:01 +0000 (14:17 -0300)
committerLucas Di Pentima <ldipentima@veritasgenetics.com>
Wed, 18 Oct 2017 17:17:01 +0000 (14:17 -0300)
* Removed initial Python version.
* Removed superfluous input files checks and moved file opening
to be before any API calls, to avoid doing them is there's a problem
with the file.
* Added user, group & link types so they're populated by json decoding.
* Cleaned up ListAll() func so it can work with different resource
types.
* Changed usage of set style types from map[string]struct{} to be
map[string]bool to simplify membership checking.
* Corrected error messages to start with lowercase.
* Added more debug messages for -verbose mode.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

sdk/cli/bin/arv-sync-groups [deleted file]
sdk/python/arvados/commands/sync_groups.py [deleted file]
tools/arv-sync-groups/arv-sync-groups.go

diff --git a/sdk/cli/bin/arv-sync-groups b/sdk/cli/bin/arv-sync-groups
deleted file mode 100755 (executable)
index f744bff..0000000
+++ /dev/null
@@ -1,7 +0,0 @@
-#!/usr/bin/env python
-# Copyright (C) The Arvados Authors. All rights reserved.
-#
-# SPDX-License-Identifier: Apache-2.0
-
-from arvados.commands.sync_groups import main
-main()
diff --git a/sdk/python/arvados/commands/sync_groups.py b/sdk/python/arvados/commands/sync_groups.py
deleted file mode 100644 (file)
index a1125de..0000000
+++ /dev/null
@@ -1,190 +0,0 @@
-# Copyright (C) The Arvados Authors. All rights reserved.
-#
-# SPDX-License-Identifier: Apache-2.0
-
-import argparse
-import arvados
-import csv
-import logging
-import os
-import sys
-
-from apiclient import errors as apiclient_errors
-from arvados._version import __version__
-
-import arvados.commands._util as arv_cmd
-
-api_client = None
-
-GROUP_TAG = 'remote_group'
-
-opts = argparse.ArgumentParser(add_help=False)
-
-opts.add_argument('--version', action='version',
-                    version="%s %s" % (sys.argv[0], __version__),
-                    help='Print version and exit.')
-opts.add_argument('--verbose', action='store_true', default=False,
-                  help="""
-Log informational messages. By default is deactivated.
-""")
-opts.add_argument('path', metavar='PATH', type=str, 
-                    help="""
-Local file path containing a CSV-like format.
-""")
-
-_user_id = opts.add_mutually_exclusive_group()
-_user_id.add_argument('--user-email', action='store_true', default=True,
-                       help="""
-Identify users by their email addresses instead of user names.
-This is the default.
-""")
-_user_id.add_argument('--user-name', action='store_false', dest='user_email',
-                      help="""
-Identify users by their name instead of email addresses.
-""")
-
-arg_parser = argparse.ArgumentParser(
-    description='Synchronize group memberships from a CSV file.',
-    parents=[opts, arv_cmd.retry_opt])
-
-def parse_arguments(arguments):
-    args = arg_parser.parse_args(arguments)
-    if args.path is None or args.path == '':
-        arg_parser.error("Please provide a path to an input file.")
-    elif not os.path.exists(args.path):
-        arg_parser.error("File not found: '%s'" % args.path)
-    elif not os.path.isfile(args.path):
-        arg_parser.error("Path provided is not a file: '%s'" % args.path)
-    return args
-
-def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
-    global api_client
-
-    args = parse_arguments(arguments)
-    logger = logging.getLogger('arvados.arv_sync_groups')
-
-    if api_client is None:
-        api_client = arvados.api('v1')
-
-    # How are users going to be identified on the input file?
-    if args.user_email:
-        user_id = 'email'
-    else:
-        user_id = 'username'
-    
-    if args.verbose:
-        logger.setLevel(logging.INFO)
-        
-    logger.info("Group sync starting. Using '%s' as users id" % user_id)
-    
-    # Get the complete user list to minimize API Server requests
-    all_users = {}
-    userid_to_uuid = {} # Index by user_id (email/username)
-    for u in arvados.util.list_all(api_client.users().list, args.retries):
-        all_users[u['uuid']] = u
-        userid_to_uuid[u[user_id]] = u['uuid']
-    logger.info('Found %d users' % len(all_users))
-
-    # Request all UUIDs for groups tagged as remote
-    remote_group_uuids = set()
-    for link in arvados.util.list_all(
-                            api_client.links().list, 
-                            args.retries,
-                            filters=[['link_class', '=', 'tag'],
-                                     ['name', '=', GROUP_TAG],
-                                     ['head_kind', '=', 'arvados#group']]):
-        remote_group_uuids.add(link['head_uuid'])
-    # Get remote groups and their members
-    remote_groups = {}
-    group_name_to_uuid = {} # Index by group name
-    for group in arvados.util.list_all(
-                            api_client.groups().list,
-                            args.retries,
-                            filters=[['uuid', 'in', list(remote_group_uuids)]]):
-        member_links = arvados.util.list_all(
-                            api_client.links().list,
-                            args.retries,
-                            filters=[['link_class', '=', 'permission'],
-                                      ['name', '=', 'can_read'],
-                                      ['tail_uuid', '=', group['uuid']],
-                                      ['head_kind', '=', 'arvados#user']])
-        # Build a list of user_ids (email/username) belonging to this group
-        members = set([all_users[link['head_uuid']][user_id] 
-                       for link in member_links])
-        remote_groups[group['uuid']] = {'object': group,
-                                        'previous_members': members,
-                                        'current_members': set()}
-        # FIXME: There's an index (group_name, group.owner_uuid), should we
-        # ask for our own groups tagged as remote? (with own being 'system'?)
-        group_name_to_uuid[group['name']] = group['uuid']
-    logger.info('Found %d remote groups' % len(remote_groups))
-    
-    groups_created = 0
-    members_added = 0
-    members_removed = 0
-    with open(args.path, 'rb') as f:
-        reader = csv.reader(f)
-        try:
-            for group, user in reader:
-                group = group.strip()
-                user = user.strip()
-                if not user in userid_to_uuid:
-                    # User not present on the system, skip.
-                    logger.warning("There's no user with %s '%s' on the system"
-                                   ", skipping." % (user_id, user))
-                    continue
-                if not group in group_name_to_uuid:
-                    # Group doesn't exist, create and tag it before continuing
-                    g = api_client.groups().create(body={
-                        'name': group}).execute(num_retries=args.retries)
-                    api_client.links().create(body={
-                        'link_class': 'tag',
-                        'name': GROUP_TAG,
-                        'head_uuid': g['uuid'],
-                    }).execute(num_retries=args.retries)
-                    # Update cached group data
-                    group_name_to_uuid[g['name']] = g['uuid']
-                    remote_groups[g['uuid']] = {'object': g,
-                                                'previous_members': set(),
-                                                'current_members': set()}
-                    groups_created += 1
-                # Both group & user exist, check if user is a member
-                g_uuid = group_name_to_uuid[group]
-                if not (user in remote_groups[g_uuid]['previous_members'] or
-                        user in remote_groups[g_uuid]['current_members']):
-                    # User wasn't a member, but should.
-                    api_client.links().create(body={
-                        'link_class': 'permission',
-                        'name': 'can_read',
-                        'tail_uuid': g_uuid,
-                        'head_uuid': userid_to_uuid[user],
-                    }).execute(num_retries=args.retries)
-                    members_added += 1
-                remote_groups[g_uuid]['current_members'].add(user)
-        except (ValueError, csv.Error) as e:
-            logger.warning('Error on line %d: %s' % (reader.line_num, e))
-    # Remove previous members not listed on this run
-    for group_uuid in remote_groups:
-        previous = remote_groups[group_uuid]['previous_members']
-        current = remote_groups[group_uuid]['current_members']
-        evicted = previous - current
-        if len(evicted) > 0:
-            logger.info("Removing %d users from group '%s'" % (
-                len(evicted), remote_groups[group_uuid]['object']['name']))
-        for evicted_user in evicted:
-            links = arvados.util.list_all(
-                api_client.links().list,
-                args.retries,
-                filters=[['link_class', '=', 'permission'],
-                         ['name', '=', 'can_read'],
-                         ['tail_uuid', '=', group_uuid],
-                         ['head_uuid', '=', userid_to_uuid[evicted_user]]])
-            for l in links:
-                api_client.links().delete(
-                    uuid=l['uuid']).execute(num_retries=args.retries)
-            members_removed += 1
-    logger.info("Groups created: %d, members added: %s, members removed: %d" % \
-                (groups_created, members_added, members_removed))
-
-if __name__ == '__main__':
-    main()
index c600c6e6d73a393eb1a98e06018b3263be66373c..d8af3b054bc4ec641536fa62a4a9cc7ad283e0fc 100644 (file)
@@ -16,6 +16,119 @@ import (
        "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
 )
 
+type resourceList interface {
+       items() []interface{}
+       itemsAvailable() int
+       offset() int
+}
+
+type groupInfo struct {
+       Group           group
+       PreviousMembers map[string]bool
+       CurrentMembers  map[string]bool
+}
+
+type user struct {
+       UUID     string `json:"uuid,omitempty"`
+       Email    string `json:"email,omitempty"`
+       Username string `json:"username,omitempty"`
+}
+
+func (u user) GetID(idSelector string) (string, error) {
+       switch idSelector {
+       case "email":
+               return u.Email, nil
+       case "username":
+               return u.Username, nil
+       default:
+               return "", fmt.Errorf("cannot identify user by %q selector", idSelector)
+       }
+}
+
+// userList implements resourceList interface
+type userList struct {
+       Items          []user `json:"items"`
+       ItemsAvailable int    `json:"items_available"`
+       Offset         int    `json:"offset"`
+}
+
+func (l userList) items() []interface{} {
+       var out []interface{}
+       for _, item := range l.Items {
+               out = append(out, item)
+       }
+       return out
+}
+
+func (l userList) itemsAvailable() int {
+       return l.ItemsAvailable
+}
+
+func (l userList) offset() int {
+       return l.Offset
+}
+
+type group struct {
+       UUID string `json:"uuid,omitempty"`
+       Name string `json:"name,omitempty"`
+}
+
+// groupList implements resourceList interface
+type groupList struct {
+       Items          []group `json:"items"`
+       ItemsAvailable int     `json:"items_available"`
+       Offset         int     `json:"offset"`
+}
+
+func (l groupList) items() []interface{} {
+       var out []interface{}
+       for _, item := range l.Items {
+               out = append(out, item)
+       }
+       return out
+}
+
+func (l groupList) itemsAvailable() int {
+       return l.ItemsAvailable
+}
+
+func (l groupList) offset() int {
+       return l.Offset
+}
+
+type link struct {
+       UUID      string `json:"uuid, omiempty"`
+       Name      string `json:"name,omitempty"`
+       LinkClass string `json:"link_class,omitempty"`
+       HeadUUID  string `json:"head_uuid,omitempty"`
+       HeadKind  string `json:"head_kind,omitempty"`
+       TailUUID  string `json:"tail_uuid,omitempty"`
+       TailKind  string `json:"tail_kind,omitempty"`
+}
+
+// linkList implements resourceList interface
+type linkList struct {
+       Items          []link `json:"items"`
+       ItemsAvailable int    `json:"items_available"`
+       Offset         int    `json:"offset"`
+}
+
+func (l linkList) items() []interface{} {
+       var out []interface{}
+       for _, item := range l.Items {
+               out = append(out, item)
+       }
+       return out
+}
+
+func (l linkList) itemsAvailable() int {
+       return l.ItemsAvailable
+}
+
+func (l linkList) offset() int {
+       return l.Offset
+}
+
 func main() {
        err := doMain()
        if err != nil {
@@ -32,7 +145,7 @@ func doMain() error {
        srcPath := flags.String(
                "path",
                "",
-               "Local file path containing a CSV-like format.")
+               "Local file path containing a CSV format.")
 
        userID := flags.String(
                "user-id",
@@ -56,19 +169,19 @@ func doMain() error {
 
        // Validations
        if *retries < 0 {
-               return fmt.Errorf("Retry quantity must be >= 0")
+               return fmt.Errorf("retry quantity must be >= 0")
        }
 
        if *srcPath == "" {
-               return fmt.Errorf("Please provide a path to an input file")
+               return fmt.Errorf("please provide a path to an input file")
        }
-       fileInfo, err := os.Stat(*srcPath)
-       switch {
-       case os.IsNotExist(err):
-               return fmt.Errorf("File not found: %s", *srcPath)
-       case fileInfo.IsDir():
-               return fmt.Errorf("Path provided is not a file: %s", *srcPath)
+
+       // Try opening the input file early, just in case there's problems.
+       f, err := os.Open(*srcPath)
+       if err != nil {
+               return fmt.Errorf("%s", err)
        }
+       defer f.Close()
 
        validUserID := false
        for _, opt := range userIDOpts {
@@ -77,99 +190,101 @@ func doMain() error {
                }
        }
        if !validUserID {
-               return fmt.Errorf("User ID must be one of: %s",
+               return fmt.Errorf("user ID must be one of: %s",
                        strings.Join(userIDOpts, ", "))
        }
 
        arv, err := arvadosclient.MakeArvadosClient()
        if err != nil {
-               return fmt.Errorf("Error setting up arvados client %s", err.Error())
+               return fmt.Errorf("error setting up arvados client %s", err)
        }
        arv.Retries = *retries
 
-       log.Printf("Group sync starting. Using '%s' as users id", *userID)
+       log.Printf("Group sync starting. Using %q as users id", *userID)
 
        // Get the complete user list to minimize API Server requests
-       allUsers := make(map[string]interface{})
+       allUsers := make(map[string]user)
        userIDToUUID := make(map[string]string) // Index by email or username
-       results := make([]interface{}, 0)
-       err = ListAll(arv, "users", arvadosclient.Dict{}, &results)
+       results, err := ListAll(arv, "users", arvadosclient.Dict{}, &userList{})
        if err != nil {
-               return fmt.Errorf("Error getting user list from the API Server %s",
-                       err.Error())
+               return fmt.Errorf("error getting user list: %s", err)
        }
        log.Printf("Found %d users", len(results))
        for _, item := range results {
-               userMap := item.(map[string]interface{})
-               allUsers[userMap["uuid"].(string)] = userMap
-               userIDToUUID[userMap[*userID].(string)] = userMap["uuid"].(string)
+               u := item.(user)
+               allUsers[u.UUID] = u
+               uID, err := u.GetID(*userID)
+               if err != nil {
+                       return err
+               }
+               userIDToUUID[uID] = u.UUID
                if *verbose {
-                       log.Printf("Seen user %s", userMap[*userID].(string))
+                       log.Printf("Seen user %q (%s)", u.Username, u.Email)
                }
        }
 
        // Request all UUIDs for groups tagged as remote
-       remoteGroupUUIDs := make(map[string]struct{})
-       results = make([]interface{}, 0)
-       err = ListAll(arv, "links", arvadosclient.Dict{
+       remoteGroupUUIDs := make(map[string]bool)
+       results, err = ListAll(arv, "links", arvadosclient.Dict{
                "filters": [][]string{
                        {"link_class", "=", "tag"},
                        {"name", "=", groupTag},
                        {"head_kind", "=", "arvados#group"},
                },
-       }, &results)
+       }, &linkList{})
        if err != nil {
-               return fmt.Errorf("Error getting remote group UUIDs: %s", err.Error())
+               return fmt.Errorf("error getting remote group UUIDs: %s", err)
        }
        for _, item := range results {
-               link := item.(map[string]interface{})
-               remoteGroupUUIDs[link["head_uuid"].(string)] = struct{}{}
+               link := item.(link)
+               remoteGroupUUIDs[link.HeadUUID] = true
        }
        // Get remote groups and their members
-       uuidList := make([]string, 0)
+       var uuidList []string
        for uuid := range remoteGroupUUIDs {
                uuidList = append(uuidList, uuid)
        }
-       remoteGroups := make(map[string]arvadosclient.Dict)
+       remoteGroups := make(map[string]*groupInfo)
        groupNameToUUID := make(map[string]string) // Index by group name
-       results = make([]interface{}, 0)
-       err = ListAll(arv, "groups", arvadosclient.Dict{
+       results, err = ListAll(arv, "groups", arvadosclient.Dict{
                "filters": [][]interface{}{
                        {"uuid", "in", uuidList},
                },
-       }, &results)
+       }, &groupList{})
        if err != nil {
-               return fmt.Errorf("Error getting remote groups by UUID: %s", err.Error())
+               return fmt.Errorf("error getting remote groups by UUID: %s", err)
        }
        for _, item := range results {
-               group := item.(map[string]interface{})
-               results := make([]interface{}, 0)
-               err := ListAll(arv, "links", arvadosclient.Dict{
+               group := item.(group)
+               results, err := ListAll(arv, "links", arvadosclient.Dict{
                        "filters": [][]string{
                                {"link_class", "=", "permission"},
                                {"name", "=", "can_read"},
-                               {"tail_uuid", "=", group["uuid"].(string)},
+                               {"tail_uuid", "=", group.UUID},
                                {"head_kind", "=", "arvados#user"},
                        },
-               }, &results)
+               }, &linkList{})
                if err != nil {
-                       return fmt.Errorf("Error getting member links: %s", err.Error())
+                       return fmt.Errorf("error getting member links: %s", err)
                }
                // Build a list of user ids (email or username) belonging to this group
-               membersSet := make(map[string]struct{}, 0)
-               for _, linkItem := range results {
-                       link := linkItem.(map[string]interface{})
-                       memberID := allUsers[link["head_uuid"].(string)].(map[string]interface{})[*userID].(string)
-                       membersSet[memberID] = struct{}{}
+               membersSet := make(map[string]bool)
+               for _, item := range results {
+                       link := item.(link)
+                       memberID, err := allUsers[link.HeadUUID].GetID(*userID)
+                       if err != nil {
+                               return err
+                       }
+                       membersSet[memberID] = true
                }
-               remoteGroups[group["uuid"].(string)] = arvadosclient.Dict{
-                       "object":           group,
-                       "previous_members": membersSet,
-                       "current_members":  make(map[string]struct{}), // Empty set
+               remoteGroups[group.UUID] = &groupInfo{
+                       Group:           group,
+                       PreviousMembers: membersSet,
+                       CurrentMembers:  make(map[string]bool), // Empty set
                }
                // FIXME: There's an index (group_name, group.owner_uuid), should we
                // ask for our own groups tagged as remote? (with own being 'system'?)
-               groupNameToUUID[group["name"].(string)] = group["uuid"].(string)
+               groupNameToUUID[group.Name] = group.UUID
        }
        log.Printf("Found %d remote groups", len(remoteGroups))
 
@@ -177,12 +292,6 @@ func doMain() error {
        membersAdded := 0
        membersRemoved := 0
 
-       f, err := os.Open(*srcPath)
-       if err != nil {
-               return fmt.Errorf("Error opening file %s: %s", *srcPath, err.Error())
-       }
-       defer f.Close()
-
        csvReader := csv.NewReader(f)
        for {
                record, err := csvReader.Read()
@@ -190,55 +299,55 @@ func doMain() error {
                        break
                }
                if err != nil {
-                       return fmt.Errorf("Error reading CSV file: %s", err.Error())
+                       return fmt.Errorf("error reading %q: %s", *srcPath, err)
                }
                groupName := record[0]
                groupMember := record[1] // User ID (username or email)
                if _, found := userIDToUUID[groupMember]; !found {
                        // User not present on the system, skip.
-                       log.Printf("Warning: there's no user with %s '%s' on the system, skipping.", *userID, groupMember)
+                       log.Printf("Warning: there's no user with %s %q on the system, skipping.", *userID, groupMember)
                        continue
                }
                if _, found := groupNameToUUID[groupName]; !found {
                        // Group doesn't exist, create and tag it before continuing
-                       group := make(map[string]interface{})
+                       var group group
                        err := arv.Create("groups", arvadosclient.Dict{
                                "group": arvadosclient.Dict{
                                        "name": groupName,
                                },
                        }, &group)
                        if err != nil {
-                               return fmt.Errorf("Error creating group named '%s': %s",
-                                       groupName, err.Error())
+                               return fmt.Errorf("error creating group named %q: %s",
+                                       groupName, err)
                        }
-                       groupUUID := group["uuid"].(string)
                        link := make(map[string]interface{})
                        err = arv.Create("links", arvadosclient.Dict{
                                "link": arvadosclient.Dict{
                                        "link_class": "tag",
                                        "name":       groupTag,
-                                       "head_uuid":  groupUUID,
+                                       "head_uuid":  group.UUID,
                                },
                        }, &link)
                        if err != nil {
-                               return fmt.Errorf("Error creating tag for group '%s': %s",
-                                       groupName, err.Error())
+                               return fmt.Errorf("error creating tag for group %q: %s",
+                                       groupName, err)
                        }
                        // Update cached group data
-                       groupNameToUUID[groupName] = groupUUID
-                       remoteGroups[groupUUID] = arvadosclient.Dict{
-                               "object":           group,
-                               "previous_members": make(map[string]struct{}), // Empty set
-                               "current_members":  make(map[string]struct{}), // Empty set
+                       groupNameToUUID[groupName] = group.UUID
+                       remoteGroups[group.UUID] = &groupInfo{
+                               Group:           group,
+                               PreviousMembers: make(map[string]bool), // Empty set
+                               CurrentMembers:  make(map[string]bool), // Empty set
                        }
-                       groupsCreated = groupsCreated + 1
+                       groupsCreated++
                }
                // Both group & user exist, check if user is a member
                groupUUID := groupNameToUUID[groupName]
-               previousMembersSet := remoteGroups[groupUUID]["previous_members"].(map[string]struct{})
-               currentMembersSet := remoteGroups[groupUUID]["current_members"].(map[string]struct{})
-               if !(contains(previousMembersSet, groupMember) ||
-                       contains(currentMembersSet, groupMember)) {
+               gi := remoteGroups[groupUUID]
+               if !gi.PreviousMembers[groupMember] && !gi.CurrentMembers[groupMember] {
+                       if *verbose {
+                               log.Printf("Adding %q to group %q", groupMember, groupName)
+                       }
                        // User wasn't a member, but should.
                        link := make(map[string]interface{})
                        err := arv.Create("links", arvadosclient.Dict{
@@ -250,45 +359,46 @@ func doMain() error {
                                },
                        }, &link)
                        if err != nil {
-                               return fmt.Errorf("Error adding user '%s' to group '%s': %s",
-                                       groupMember, groupName, err.Error())
+                               return fmt.Errorf("error adding user %q to group %q: %s",
+                                       groupMember, groupName, err)
                        }
-                       membersAdded = membersAdded + 1
+                       membersAdded++
                }
-               currentMembersSet[groupMember] = struct{}{}
+               gi.CurrentMembers[groupMember] = true
        }
 
        // Remove previous members not listed on this run
        for groupUUID := range remoteGroups {
-               previousMembersSet := remoteGroups[groupUUID]["previous_members"].(map[string]struct{})
-               currentMembersSet := remoteGroups[groupUUID]["current_members"].(map[string]struct{})
-               evictedMembersSet := subtract(previousMembersSet, currentMembersSet)
-               groupName := remoteGroups[groupUUID]["object"].(map[string]interface{})["name"]
-               if len(evictedMembersSet) > 0 {
-                       log.Printf("Removing %d users from group '%s'", len(evictedMembersSet), groupName)
+               gi := remoteGroups[groupUUID]
+               evictedMembers := subtract(gi.PreviousMembers, gi.CurrentMembers)
+               groupName := gi.Group.Name
+               if len(evictedMembers) > 0 {
+                       log.Printf("Removing %d users from group %q", len(evictedMembers), groupName)
                }
-               for evictedUser := range evictedMembersSet {
-                       links := make([]interface{}, 0)
-                       err := ListAll(arv, "links", arvadosclient.Dict{
+               for evictedUser := range evictedMembers {
+                       links, err := ListAll(arv, "links", arvadosclient.Dict{
                                "filters": [][]string{
                                        {"link_class", "=", "permission"},
                                        {"name", "=", "can_read"},
                                        {"tail_uuid", "=", groupUUID},
                                        {"head_uuid", "=", userIDToUUID[evictedUser]},
                                },
-                       }, &links)
+                       }, &linkList{})
                        if err != nil {
-                               return fmt.Errorf("Error getting links needed to remove user '%s' from group '%s': %s", evictedUser, groupName, err.Error())
+                               return fmt.Errorf("error getting links needed to remove user %q from group %q: %s", evictedUser, groupName, err)
                        }
-                       for _, link := range links {
-                               linkUUID := link.(map[string]interface{})["uuid"].(string)
-                               l := make(map[string]interface{})
-                               err := arv.Delete("links", linkUUID, arvadosclient.Dict{}, &l)
+                       for _, item := range links {
+                               link := item.(link)
+                               var l map[string]interface{}
+                               if *verbose {
+                                       log.Printf("Removing %q from group %q", evictedUser, gi.Group.Name)
+                               }
+                               err := arv.Delete("links", link.UUID, arvadosclient.Dict{}, &l)
                                if err != nil {
-                                       return fmt.Errorf("Error removing user '%s' from group '%s': %s", evictedUser, groupName, err.Error())
+                                       return fmt.Errorf("error removing user %q from group %q: %s", evictedUser, groupName, err)
                                }
                        }
-                       membersRemoved = membersRemoved + 1
+                       membersRemoved++
                }
        }
        log.Printf("Groups created: %d, members added: %d, members removed: %d", groupsCreated, membersAdded, membersRemoved)
@@ -297,42 +407,33 @@ func doMain() error {
 }
 
 // ListAll : Adds all objects of type 'resource' to the 'output' list
-func ListAll(arv *arvadosclient.ArvadosClient, resource string, parameters arvadosclient.Dict, output *[]interface{}) (err error) {
-       // Default limit value
+func ListAll(arv *arvadosclient.ArvadosClient, resource string, parameters arvadosclient.Dict, rl resourceList) (allItems []interface{}, err error) {
        if _, ok := parameters["limit"]; !ok {
-               parameters["limit"] = 1000
+               // Default limit value: use the maximum page size the server allows
+               parameters["limit"] = 1<<31 - 1
        }
        offset := 0
        itemsAvailable := parameters["limit"].(int)
-       for len(*output) < itemsAvailable {
-               results := make(arvadosclient.Dict)
+       for len(allItems) < itemsAvailable {
                parameters["offset"] = offset
-               err = arv.List(resource, parameters, &results)
+               err = arv.List(resource, parameters, &rl)
                if err != nil {
-                       return err
+                       return allItems, err
                }
-               if value, ok := results["items"]; ok {
-                       items := value.([]interface{})
-                       for _, item := range items {
-                               *output = append(*output, item)
-                       }
-                       offset = int(results["offset"].(float64)) + len(items)
+               for _, i := range rl.items() {
+                       allItems = append(allItems, i)
                }
-               itemsAvailable = int(results["items_available"].(float64))
+               offset = rl.offset() + len(rl.items())
+               itemsAvailable = rl.itemsAvailable()
        }
-       return nil
-}
-
-func contains(set map[string]struct{}, element string) bool {
-       _, found := set[element]
-       return found
+       return allItems, nil
 }
 
-func subtract(setA map[string]struct{}, setB map[string]struct{}) map[string]struct{} {
-       result := make(map[string]struct{})
+func subtract(setA map[string]bool, setB map[string]bool) map[string]bool {
+       result := make(map[string]bool)
        for element := range setA {
-               if !contains(setB, element) {
-                       result[element] = struct{}{}
+               if !setB[element] {
+                       result[element] = true
                }
        }
        return result