diff options
author | Martin Polden <mpolden@mpolden.no> | 2022-03-11 09:43:38 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2022-03-11 10:08:40 +0100 |
commit | f1b6af070d5badd5c610d4a052df5ca5952dfb82 (patch) | |
tree | 84ca5d2d7f1b630d4e11600f2a63ce4bc0356d90 | |
parent | 80abc0659f07445536d92c59fae58dfb1f0ecae8 (diff) |
Reject config request when major version is incompatible
8 files changed, 122 insertions, 44 deletions
diff --git a/config/src/main/java/com/yahoo/vespa/config/ErrorCode.java b/config/src/main/java/com/yahoo/vespa/config/ErrorCode.java index 164fe36e2c6..6f0f121a9a6 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ErrorCode.java +++ b/config/src/main/java/com/yahoo/vespa/config/ErrorCode.java @@ -38,31 +38,34 @@ public final class ErrorCode { public static final int INCONSISTENT_CONFIG_MD5 = UNKNOWN_CONFIG + 400; + public static final int INCOMPATIBLE_VESPA_VERSION = UNKNOWN_CONFIG + 500; + private ErrorCode() { } public static String getName(int error) { switch(error) { - case UNKNOWN_CONFIG: return "UNKNOWN_CONFIG"; - case UNKNOWN_DEFINITION: return "UNKNOWN_DEFINITION"; - case UNKNOWN_DEF_MD5: return "UNKNOWN_DEF_MD5"; - case ILLEGAL_NAME: return "ILLEGAL_NAME"; - case ILLEGAL_VERSION: return "ILLEGAL_VERSION"; - case ILLEGAL_CONFIGID: return "ILLEGAL_CONFIGID"; - case ILLEGAL_DEF_MD5: return "ILLEGAL_DEF_MD5"; - case ILLEGAL_CONFIG_MD5: return "ILLEGAL_CONFIG_MD5"; - case ILLEGAL_TIMEOUT: return "ILLEGAL_TIMEOUT"; - case ILLEGAL_GENERATION: return "ILLEGAL_GENERATION"; - case ILLEGAL_SUB_FLAG: return "ILLEGAL_SUBSCRIBE_FLAG"; - case ILLEGAL_NAME_SPACE: return "ILLEGAL_NAME_SPACE"; - case ILLEGAL_CLIENT_HOSTNAME: return "ILLEGAL_CLIENT_HOSTNAME"; - case OUTDATED_CONFIG: return "OUTDATED_CONFIG"; - case INTERNAL_ERROR: return "INTERNAL_ERROR"; - case APPLICATION_NOT_LOADED: return "APPLICATION_NOT_LOADED"; - case ILLEGAL_PROTOCOL_VERSION: return "ILLEGAL_PROTOCOL_VERSION"; - case INCONSISTENT_CONFIG_MD5: return "INCONSISTENT_CONFIG_MD5"; - case UNKNOWN_VESPA_VERSION: return "UNKNOWN_VESPA_VERSION"; - default: return "Unknown error"; + case UNKNOWN_CONFIG: return "UNKNOWN_CONFIG"; + case UNKNOWN_DEFINITION: return "UNKNOWN_DEFINITION"; + case UNKNOWN_DEF_MD5: return "UNKNOWN_DEF_MD5"; + case ILLEGAL_NAME: return "ILLEGAL_NAME"; + case ILLEGAL_VERSION: return "ILLEGAL_VERSION"; + case ILLEGAL_CONFIGID: return "ILLEGAL_CONFIGID"; + case ILLEGAL_DEF_MD5: return "ILLEGAL_DEF_MD5"; + case ILLEGAL_CONFIG_MD5: return "ILLEGAL_CONFIG_MD5"; + case ILLEGAL_TIMEOUT: return "ILLEGAL_TIMEOUT"; + case ILLEGAL_GENERATION: return "ILLEGAL_GENERATION"; + case ILLEGAL_SUB_FLAG: return "ILLEGAL_SUBSCRIBE_FLAG"; + case ILLEGAL_NAME_SPACE: return "ILLEGAL_NAME_SPACE"; + case ILLEGAL_CLIENT_HOSTNAME: return "ILLEGAL_CLIENT_HOSTNAME"; + case OUTDATED_CONFIG: return "OUTDATED_CONFIG"; + case INTERNAL_ERROR: return "INTERNAL_ERROR"; + case APPLICATION_NOT_LOADED: return "APPLICATION_NOT_LOADED"; + case ILLEGAL_PROTOCOL_VERSION: return "ILLEGAL_PROTOCOL_VERSION"; + case INCONSISTENT_CONFIG_MD5: return "INCONSISTENT_CONFIG_MD5"; + case UNKNOWN_VESPA_VERSION: return "UNKNOWN_VESPA_VERSION"; + case INCOMPATIBLE_VESPA_VERSION: return "INCOMPATIBLE_VESPA_VERSION"; + default: return "Unknown error"; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java index 99f5cce642a..20b16614a53 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java @@ -92,4 +92,8 @@ public interface RequestHandler { * @return set of file references that is owned by the application */ Set<FileReference> listFileReferences(ApplicationId applicationId); + + /** Returns whether the latest deployed version of application is compatible with given vespaVersion */ + boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application); + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java index 9fbc3eb21df..0e4fffaccef 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java @@ -108,6 +108,11 @@ public class SuperModelRequestHandler implements RequestHandler { throw new UnsupportedOperationException(); } + @Override + public boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application) { + return true; + } + public void enable() { enabled = true; superModelManager.markAsComplete(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 937ec2bb0e7..7c669f290d3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -26,6 +26,9 @@ import com.yahoo.vespa.curator.CompletionTimeoutException; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.curator.transaction.CuratorTransaction; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.ListFlag; +import com.yahoo.vespa.flags.PermanentFlags; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; @@ -71,11 +74,12 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica private final Clock clock; private final TenantFileSystemDirs tenantFileSystemDirs; private final ConfigserverConfig configserverConfig; + private final ListFlag<Integer> incompatibleMajorVersions; public TenantApplications(TenantName tenant, Curator curator, StripedExecutor<TenantName> zkWatcherExecutor, ExecutorService zkCacheExecutor, Metrics metrics, ReloadListener reloadListener, ConfigserverConfig configserverConfig, HostRegistry hostRegistry, - TenantFileSystemDirs tenantFileSystemDirs, Clock clock) { + TenantFileSystemDirs tenantFileSystemDirs, Clock clock, FlagSource flagSource) { this.curator = curator; this.database = new ApplicationCuratorDatabase(tenant, curator); this.tenant = tenant; @@ -91,6 +95,7 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica this.tenantFileSystemDirs = tenantFileSystemDirs; this.clock = clock; this.configserverConfig = configserverConfig; + this.incompatibleMajorVersions = PermanentFlags.INCOMPATIBLE_MAJOR_VERSIONS.bindTo(flagSource); } /** The curator backed ZK storage of this. */ @@ -383,6 +388,15 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica } @Override + public boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application) { + if (vespaVersion.isEmpty()) return true; + Version latestDeployed = applicationMapper.getForVersion(application, Optional.empty(), clock.instant()) + .getVespaVersion(); + boolean compatibleMajor = !incompatibleMajorVersions.value().contains(latestDeployed.getMajor()); + return compatibleMajor || vespaVersion.get().getMajor() == latestDeployed.getMajor(); + } + + @Override public void verifyHosts(ApplicationId applicationId, Collection<String> newHosts) { hostRegistry.verifyHosts(applicationId, newHosts); reloadListener.verifyHostsAreAvailable(applicationId, newHosts); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java index 88a30a9d72b..55645d83b3c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java @@ -100,11 +100,16 @@ class GetConfigProcessor implements Runnable { request.getVespaVersion().map(VespaVersion::toString).map(Version::fromString) : Optional.empty(); if (logDebug(trace)) { - debugLog(trace, "Using version " + getPrintableVespaVersion(vespaVersion)); + debugLog(trace, "Using version " + printableVespaVersion(vespaVersion)); } if ( ! context.requestHandler().hasApplication(context.applicationId(), vespaVersion)) { - handleError(request, ErrorCode.UNKNOWN_VESPA_VERSION, "Unknown Vespa version in request: " + getPrintableVespaVersion(vespaVersion)); + handleError(request, ErrorCode.UNKNOWN_VESPA_VERSION, "Unknown Vespa version in request: " + printableVespaVersion(vespaVersion)); + return null; + } + + if ( ! context.requestHandler().compatibleWith(vespaVersion, context.applicationId())) { + handleError(request, ErrorCode.INCOMPATIBLE_VESPA_VERSION, "Version " + printableVespaVersion(vespaVersion) + " is binary incompatible with the latest deployed version"); return null; } @@ -161,7 +166,7 @@ class GetConfigProcessor implements Runnable { request.getConfigKey().getNamespace().equals(SentinelConfig.getDefNamespace()); } - private static String getPrintableVespaVersion(Optional<Version> vespaVersion) { + private static String printableVespaVersion(Optional<Version> vespaVersion) { return (vespaVersion.isPresent() ? vespaVersion.get().toFullString() : "LATEST"); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index 7d53c383cf5..82d8a42bc92 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -342,7 +342,8 @@ public class TenantRepository { configserverConfig, hostRegistry, new TenantFileSystemDirs(configServerDB, tenantName), - clock); + clock, + flagSource); PermanentApplicationPackage permanentApplicationPackage = new PermanentApplicationPackage(configserverConfig); SessionPreparer sessionPreparer = new SessionPreparer(modelFactoryRegistry, fileDistributionFactory, diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index 5afcd62a25c..2b361e825e5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -26,6 +26,8 @@ import com.yahoo.vespa.curator.CompletionTimeoutException; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.curator.mock.MockCuratorFramework; +import com.yahoo.vespa.flags.InMemoryFlagSource; +import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.VespaModelFactory; import org.apache.curator.framework.CuratorFramework; @@ -47,6 +49,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; import java.util.stream.IntStream; import static com.yahoo.vespa.config.server.application.TenantApplications.RemoveApplicationWaiter; @@ -66,7 +69,6 @@ public class TenantApplicationsTest { private Curator curator; private CuratorFramework curatorFramework; - private TenantApplications applications; private ConfigserverConfig configserverConfig; @Rule @@ -88,7 +90,6 @@ public class TenantApplicationsTest { .build(); tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT); tenantRepository.addTenant(tenantName); - applications = createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener()); } @Test @@ -150,6 +151,43 @@ public class TenantApplicationsTest { assertEquals(0, repo.activeApplications().size()); } + @Test + public void major_version_compatibility() throws Exception { + InMemoryFlagSource flagSource = new InMemoryFlagSource(); + TenantApplications applications = createZKAppRepo(flagSource); + ApplicationId app1 = createApplicationId("myapp"); + applications.createApplication(app1); + applications.createPutTransaction(app1, 1).commit(); + VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app"))); + Function<Version, ApplicationSet> createApplicationSet = (version) -> { + return ApplicationSet.from(new Application(model, + new ServerCache(), + 1, + version, + MetricUpdater.createTestUpdater(), + app1)); + }; + + Version deployedVersion0 = Version.fromString("6.1"); + applications.activateApplication(createApplicationSet.apply(deployedVersion0), 1); + assertTrue("Empty version is compatible", applications.compatibleWith(Optional.empty(), app1)); + + Version nodeVersion0 = Version.fromString("6.0"); + assertTrue("Lower version is compatible", applications.compatibleWith(Optional.of(nodeVersion0), app1)); + + Version deployedVersion1 = Version.fromString("7.1"); + applications.activateApplication(createApplicationSet.apply(deployedVersion1), 1); + assertTrue("New major is compatible", applications.compatibleWith(Optional.of(nodeVersion0), app1)); + + flagSource.withListFlag(PermanentFlags.INCOMPATIBLE_MAJOR_VERSIONS.id(), List.of(8), Integer.class); + Version deployedVersion2 = Version.fromString("8.1"); + applications.activateApplication(createApplicationSet.apply(deployedVersion2), 1); + assertFalse("New major is incompatible", applications.compatibleWith(Optional.of(nodeVersion0), app1)); + + Version nodeVersion1 = Version.fromString("8.0"); + assertTrue("Node is compatible after upgrading", applications.compatibleWith(Optional.of(nodeVersion1), app1)); + } + public static class MockReloadListener implements ReloadListener { public final AtomicInteger reloaded = new AtomicInteger(0); final AtomicInteger removed = new AtomicInteger(0); @@ -175,14 +213,10 @@ public class TenantApplicationsTest { } } - private void assertdefaultAppNotFound() { - assertFalse(applications.hasApplication(ApplicationId.defaultId(), Optional.of(vespaVersion))); - } - @Test public void testListConfigs() throws IOException, SAXException { - applications = createTenantApplications(TenantName.defaultName(), new MockCurator(), configserverConfig, new MockReloadListener()); - assertdefaultAppNotFound(); + TenantApplications applications = createTenantApplications(TenantName.defaultName(), new MockCurator(), configserverConfig, new MockReloadListener(), new InMemoryFlagSource()); + assertFalse(applications.hasApplication(ApplicationId.defaultId(), Optional.of(vespaVersion))); VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app"))); ApplicationId applicationId = ApplicationId.defaultId(); @@ -209,6 +243,7 @@ public class TenantApplicationsTest { @Test public void testAppendIdsInNonRecursiveListing() { + TenantApplications applications = createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener(), new InMemoryFlagSource()); assertEquals(applications.appendOneLevelOfId("search/music", "search/music/qrservers/default/qr.0"), "search/music/qrservers"); assertEquals(applications.appendOneLevelOfId("search", "search/music/qrservers/default/qr.0"), "search/music"); assertEquals(applications.appendOneLevelOfId("search/music/qrservers/default/qr.0", "search/music/qrservers/default/qr.0"), "search/music/qrservers/default/qr.0"); @@ -251,9 +286,12 @@ public class TenantApplicationsTest { } private TenantApplications createZKAppRepo() { - return createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener()); + return createZKAppRepo(new InMemoryFlagSource()); } + private TenantApplications createZKAppRepo(InMemoryFlagSource flagSource) { + return createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener(), flagSource); + } private static ApplicationId createApplicationId() { return createApplicationId("foo"); @@ -285,17 +323,18 @@ public class TenantApplicationsTest { private TenantApplications createTenantApplications(TenantName tenantName, Curator curator, ConfigserverConfig configserverConfig, - ReloadListener reloadListener) { + ReloadListener reloadListener, InMemoryFlagSource flagSource) { return new TenantApplications(tenantName, - curator, - new StripedExecutor<>(new InThreadExecutorService()), - new InThreadExecutorService(), - Metrics.createTestMetrics(), - reloadListener, - configserverConfig, - new HostRegistry(), - new TenantFileSystemDirs(new ConfigServerDB(configserverConfig), tenantName), - Clock.systemUTC()); + curator, + new StripedExecutor<>(new InThreadExecutorService()), + new InThreadExecutorService(), + Metrics.createTestMetrics(), + reloadListener, + configserverConfig, + new HostRegistry(), + new TenantFileSystemDirs(new ConfigServerDB(configserverConfig), tenantName), + Clock.systemUTC(), + flagSource); } private static class MockCurator3ConfigServers extends Curator { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java index 3ae85a54453..225c45fbfdb 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java @@ -240,6 +240,13 @@ public class PermanentFlags { "Takes effect immediately.", ZONE_ID, APPLICATION_ID); + public static final UnboundListFlag<Integer> INCOMPATIBLE_MAJOR_VERSIONS = defineListFlag( + "incompatible-major-versions", List.of(8), Integer.class, + "A list of major versions which are binary-incompatible and requires an application package to " + + "be built specifically for that Vespa version. When an application upgrades to an incompatible major " + + "version, the config server will refuse to serve config to nodes still running on older major versions", + "Takes effect immediately"); + private PermanentFlags() {} private static UnboundBooleanFlag defineFeatureFlag( |