Move zone and system specific settings to ArchiveService implementation

4 files changed, 20 insertions, 21 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/ArchiveService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/ArchiveService.java
index 69eda662692..389d815249d 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/ArchiveService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/ArchiveService.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.archive;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
 
+import java.net.URI;
 import java.util.Map;
 import java.util.Set;
 
@@ -20,4 +21,8 @@ public interface ArchiveService {
     void updateBucketPolicy(ZoneId zoneId, ArchiveBucket bucket, Map<TenantName, String> authorizeIamRoleByTenantName);
 
     void updateKeyPolicy(ZoneId zoneId, String keyArn, Set<String> tenantAuthorizedIamRoles);
+
+    boolean canAddTenantToBucket(ZoneId zoneId, ArchiveBucket bucket);
+
+    URI bucketURI(ZoneId zoneId, String bucketName, TenantName tenantName);
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/MockArchiveService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/MockArchiveService.java
index ce7b56ad1f6..1db003f8067 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/MockArchiveService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/archive/MockArchiveService.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.archive;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
 
+import java.net.URI;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -32,4 +33,14 @@ public class MockArchiveService implements ArchiveService {
     public void updateKeyPolicy(ZoneId zoneId, String keyArn, Set<String> tenantAuthorizedIamRoles) {
         authorizedIamRolesForKey.put(keyArn, tenantAuthorizedIamRoles);
     }
+
+    @Override
+    public boolean canAddTenantToBucket(ZoneId zoneId, ArchiveBucket bucket) {
+        return bucket.tenants().size() < 5;
+    }
+
+    @Override
+    public URI bucketURI(ZoneId zoneId, String bucketName, TenantName tenantName) {
+        return URI.create(String.format("s3://%s/%s/", bucketName, tenantName.value()));
+    }
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDb.java
index 21914b87818..0a0adcfc252 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDb.java
@@ -46,7 +46,7 @@ public class CuratorArchiveBucketDb {
         return getBucketNameFromCache(zoneId, tenant)
                 .or(() -> findAndUpdateArchiveUriCache(zoneId, tenant, buckets(zoneId)))
                 .or(() -> createIfMissing ? Optional.of(assignToBucket(zoneId, tenant)) : Optional.empty())
-                .map(bucketName -> URI.create(Text.format("s3://%s/%s/", bucketName, tenant.value())));
+                .map(bucketName -> archiveService.bucketURI(zoneId, bucketName, tenant));
     }
 
     private String assignToBucket(ZoneId zoneId, TenantName tenant) {
@@ -57,7 +57,7 @@ public class CuratorArchiveBucketDb {
                     .orElseGet(() -> {
                         // If not, find an existing bucket with space
                         Optional<ArchiveBucket> unfilledBucket = zoneBuckets.stream()
-                                .filter(bucket -> bucket.tenants().size() < tenantsPerBucket().orElse(Integer.MAX_VALUE))
+                                .filter(bucket -> archiveService.canAddTenantToBucket(zoneId, bucket))
                                 .findAny();
 
                         // And place the tenant in that bucket.
@@ -94,23 +94,6 @@ public class CuratorArchiveBucketDb {
         return bucketName;
     }
 
-    private OptionalInt tenantsPerBucket() {
-        if (system.isPublic()) {
-            /*
-             * Due to policy limits, we can't put data for more than this many tenants in a bucket.
-             * Policy size limit is 20kb, about 550 bytes for non-tenant related policies. Each tenant
-             * needs about 500 + len(role_arn) bytes, we limit role_arn to 100 characters, so we can
-             * fit about (20k - 550) / 600 ~ 32 tenants per bucket.
-             */
-            return OptionalInt.of(30);
-        } else {
-            /*
-             * The S3 policies in main/cd have a fixed size.
-             */
-            return OptionalInt.empty();
-        }
-    }
-
     private Optional<String> getBucketNameFromCache(ZoneId zoneId, TenantName tenantName) {
         return Optional.ofNullable(archiveUriCache.get(zoneId)).map(map -> map.get(tenantName));
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDbTest.java
index 1a052b6a578..4c3d76b1b17 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDbTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/archive/CuratorArchiveBucketDbTest.java
@@ -32,7 +32,7 @@ public class CuratorArchiveBucketDbTest {
         assertEquals(Optional.of(URI.create("s3://existingBucket/default/")), bucketDb.archiveUriFor(ZoneId.defaultId(), TenantName.defaultName(), true));
 
         // Assigns to existing bucket while there is space
-        IntStream.range(0, 29).forEach(i ->
+        IntStream.range(0, 4).forEach(i ->
                 assertEquals(
                         Optional.of(URI.create("s3://existingBucket/tenant" + i + "/")), bucketDb
                                 .archiveUriFor(ZoneId.defaultId(), TenantName.from("tenant" + i), true)));
@@ -47,7 +47,7 @@ public class CuratorArchiveBucketDbTest {
         assertEquals(Optional.empty(), bucketDb.archiveUriFor(ZoneId.from("prod.us-east-3"), TenantName.from("newTenant"), false));
 
         // Lists all buckets by zone
-        Set<TenantName> existingBucketTenants = Streams.concat(Stream.of(TenantName.defaultName()), IntStream.range(0, 29).mapToObj(i -> TenantName.from("tenant" + i))).collect(Collectors.toUnmodifiableSet());
+        Set<TenantName> existingBucketTenants = Streams.concat(Stream.of(TenantName.defaultName()), IntStream.range(0, 4).mapToObj(i -> TenantName.from("tenant" + i))).collect(Collectors.toUnmodifiableSet());
         assertEquals(
                 Set.of(
                         new ArchiveBucket("existingBucket", "keyArn").withTenants(existingBucketTenants),