From ed19e2f04df3b7bbeaf14a1f09e6d1dcdf92517e Mon Sep 17 00:00:00 2001 From: Emmanuel Hugonnet Date: Wed, 20 Mar 2024 18:50:37 +0100 Subject: [PATCH] [WFCORE-6503]: Fixing the reloading issue Signed-off-by: Emmanuel Hugonnet --- .../org/jboss/as/controller/ModelControllerImpl.java | 2 +- .../org/jboss/as/controller/RunningModeControl.java | 9 +++++++++ .../persistence/BackupXmlConfigurationPersister.java | 4 ++++ .../persistence/ConfigurationPersister.java | 11 +++++++++++ .../persistence/NullConfigurationPersister.java | 5 +++++ .../persistence/XmlConfigurationPersister.java | 7 +++++++ .../as/model/test/StringConfigurationPersister.java | 7 +++++++ .../resources/ServerRootResourceDefinition.java | 2 +- .../operations/ServerDomainProcessReloadHandler.java | 2 +- .../server/operations/ServerProcessReloadHandler.java | 9 ++++++++- .../management/persistence/YamlExtensionTestCase.java | 2 +- 11 files changed, 55 insertions(+), 5 deletions(-) diff --git a/controller/src/main/java/org/jboss/as/controller/ModelControllerImpl.java b/controller/src/main/java/org/jboss/as/controller/ModelControllerImpl.java index 02f08ccb8cd..cd6d7e7154b 100644 --- a/controller/src/main/java/org/jboss/as/controller/ModelControllerImpl.java +++ b/controller/src/main/java/org/jboss/as/controller/ModelControllerImpl.java @@ -518,7 +518,7 @@ boolean boot(final List bootList, final OperationMessageHandler handl headers, handler, null, managementModel.get(), control, processState, auditLogger, bootingFlag.get(), true, hostServerGroupTracker, null, notificationSupport, true, extraValidationStepHandler, partialModel, securityIdentitySupplier); - if (configExtension != null && configExtension.shouldProcessOperations(runningModeControl.getRunningMode()) && !runningModeControl.isReloaded()) { + if (configExtension != null && configExtension.shouldProcessOperations(runningModeControl.getRunningMode()) && (!runningModeControl.isReloaded() || runningModeControl.isApplyConfigurationExtension())) { configExtension.processOperations(managementModel.get().getRootResourceRegistration(), bootOperations.postExtensionOps); } for (ParsedBootOp parsedOp : bootOperations.postExtensionOps) { diff --git a/controller/src/main/java/org/jboss/as/controller/RunningModeControl.java b/controller/src/main/java/org/jboss/as/controller/RunningModeControl.java index ffba4eab7b5..183d2c3eaa8 100644 --- a/controller/src/main/java/org/jboss/as/controller/RunningModeControl.java +++ b/controller/src/main/java/org/jboss/as/controller/RunningModeControl.java @@ -17,6 +17,7 @@ public class RunningModeControl { private volatile boolean useCurrentConfig; private volatile String newBootFileName; private volatile Boolean suspend; + private volatile boolean applyConfigurationExtension; public RunningModeControl(final RunningMode initialMode) { this.runningMode = initialMode; @@ -58,6 +59,14 @@ public void setSuspend(Boolean suspend) { this.suspend = suspend; } + public boolean isApplyConfigurationExtension() { + return applyConfigurationExtension; + } + + public void setApplyConfigurationExtension(boolean applyConfigurationExtension) { + this.applyConfigurationExtension = applyConfigurationExtension; + } + /** * Get the new boot file name. For a standalone server this will be the location of the server configuration * (i.e. the standalone.xml variety). For a host controller this will be the location of the host configuration diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/BackupXmlConfigurationPersister.java b/controller/src/main/java/org/jboss/as/controller/persistence/BackupXmlConfigurationPersister.java index 1302ca8e62c..6c428cb20c8 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/BackupXmlConfigurationPersister.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/BackupXmlConfigurationPersister.java @@ -68,6 +68,7 @@ private static boolean isSuppressLoad(ConfigurationFile configurationFile, boole return initialEmpty && !reload; } + @Override public void registerAdditionalRootElement(final QName anotherRoot, final XMLElementReader> parser){ super.registerAdditionalRootElement(anotherRoot, parser); } @@ -93,13 +94,16 @@ public boolean isPersisting() { public PersistenceResource store(final ModelNode model, Set affectedAddresses) throws ConfigurationPersistenceException { if(!successfulBoot.get()) { return new PersistenceResource() { + @Override public void commit() { } + @Override public void rollback() { } }; } + this.stored = true; return new ConfigurationFilePersistenceResource(model, configurationFile, this); } diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationPersister.java b/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationPersister.java index 32ad21a7724..e05a06dea5f 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationPersister.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/ConfigurationPersister.java @@ -50,6 +50,17 @@ default boolean isPersisting() { return true; } + /** + * Gets whether a call persist to persistent storage has been successfully completed. + *

+ * The default implementation always returns {@code false} + * + * @return {@code true} if a call to {@link #store(ModelNode, Set)} will return an object that actually writes + */ + default boolean hasStored() { + return false; + } + /** * Persist the given configuration model if {@link #isPersisting()} would return {@code true}, otherwise * return a no-op {@link PersistenceResource}. diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/NullConfigurationPersister.java b/controller/src/main/java/org/jboss/as/controller/persistence/NullConfigurationPersister.java index 51d38e7b7c0..c7f11592a92 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/NullConfigurationPersister.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/NullConfigurationPersister.java @@ -40,6 +40,11 @@ public List load() { return Collections.emptyList(); } + @Override + public boolean hasStored() { + return false; + } + private static class NullPersistenceResource implements ConfigurationPersister.PersistenceResource { private static final NullPersistenceResource INSTANCE = new NullPersistenceResource(); diff --git a/controller/src/main/java/org/jboss/as/controller/persistence/XmlConfigurationPersister.java b/controller/src/main/java/org/jboss/as/controller/persistence/XmlConfigurationPersister.java index 4e0f223f318..7ee06195b0b 100644 --- a/controller/src/main/java/org/jboss/as/controller/persistence/XmlConfigurationPersister.java +++ b/controller/src/main/java/org/jboss/as/controller/persistence/XmlConfigurationPersister.java @@ -44,6 +44,7 @@ public class XmlConfigurationPersister extends AbstractConfigurationPersister { private final XMLElementReader> rootParser; private final Map>> additionalParsers; private final boolean suppressLoad; + protected volatile boolean stored = false; /** * Construct a new instance. @@ -84,6 +85,7 @@ public void registerAdditionalRootElement(final QName anotherRoot, final XMLElem /** {@inheritDoc} */ @Override public PersistenceResource store(final ModelNode model, Set affectedAddresses) throws ConfigurationPersistenceException { + stored = true; return new FilePersistenceResource(model, fileName, this); } @@ -157,4 +159,9 @@ protected void successfulBoot(File file) throws ConfigurationPersistenceExceptio } + @Override + public boolean hasStored() { + return isPersisting() && stored; + } + } diff --git a/model-test/src/main/java/org/jboss/as/model/test/StringConfigurationPersister.java b/model-test/src/main/java/org/jboss/as/model/test/StringConfigurationPersister.java index bda6d3dc17f..ed521a8534c 100644 --- a/model-test/src/main/java/org/jboss/as/model/test/StringConfigurationPersister.java +++ b/model-test/src/main/java/org/jboss/as/model/test/StringConfigurationPersister.java @@ -27,6 +27,7 @@ public class StringConfigurationPersister extends AbstractConfigurationPersister private final List bootOperations; private final boolean persistXml; volatile String marshalled; + private volatile boolean stored = false; public StringConfigurationPersister(List bootOperations, XMLElementWriter rootDeparser, boolean persistXml) { super(rootDeparser); @@ -40,6 +41,7 @@ public PersistenceResource store(ModelNode model, Set affectedAddre if (!persistXml) { return new NullConfigurationPersister().store(model, affectedAddresses); } + stored = true; return new StringPersistenceResource(model, this); } @@ -56,6 +58,11 @@ public String getMarshalled() { return marshalled; } + @Override + public boolean hasStored() { + return isPersisting() && stored; + } + private class StringPersistenceResource implements PersistenceResource { private byte[] bytes; diff --git a/server/src/main/java/org/jboss/as/server/controller/resources/ServerRootResourceDefinition.java b/server/src/main/java/org/jboss/as/server/controller/resources/ServerRootResourceDefinition.java index 21f593bae9e..c0bad7a8bd1 100644 --- a/server/src/main/java/org/jboss/as/server/controller/resources/ServerRootResourceDefinition.java +++ b/server/src/main/java/org/jboss/as/server/controller/resources/ServerRootResourceDefinition.java @@ -413,7 +413,7 @@ public void registerOperations(ManagementResourceRegistration resourceRegistrati resourceRegistration.registerOperationHandler(ServerDomainProcessShutdownHandler.DOMAIN_DEFINITION, new ServerDomainProcessShutdownHandler()); } else { - final ServerProcessReloadHandler reloadHandler = new ServerProcessReloadHandler(Services.JBOSS_AS, runningModeControl, processState, serverEnvironment); + final ServerProcessReloadHandler reloadHandler = new ServerProcessReloadHandler(Services.JBOSS_AS, runningModeControl, processState, serverEnvironment, extensibleConfigurationPersister); resourceRegistration.registerOperationHandler(ServerProcessReloadHandler.DEFINITION, reloadHandler, false); resourceRegistration.registerOperationHandler(ServerSuspendHandler.DEFINITION, ServerSuspendHandler.INSTANCE); diff --git a/server/src/main/java/org/jboss/as/server/operations/ServerDomainProcessReloadHandler.java b/server/src/main/java/org/jboss/as/server/operations/ServerDomainProcessReloadHandler.java index e9151d7f24f..f963c1f21e2 100644 --- a/server/src/main/java/org/jboss/as/server/operations/ServerDomainProcessReloadHandler.java +++ b/server/src/main/java/org/jboss/as/server/operations/ServerDomainProcessReloadHandler.java @@ -26,7 +26,7 @@ public class ServerDomainProcessReloadHandler extends ServerProcessReloadHandler public ServerDomainProcessReloadHandler(ServiceName rootService, RunningModeControl runningModeControl, ControlledProcessState processState, final DomainServerCommunicationServices.OperationIDUpdater operationIDUpdater, final ServerEnvironment serverEnvironment) { - super(rootService, runningModeControl, processState, serverEnvironment); + super(rootService, runningModeControl, processState, serverEnvironment, null); this.operationIDUpdater = operationIDUpdater; } diff --git a/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java b/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java index 8c9b36ac1ff..f1d4fe8cd06 100644 --- a/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java +++ b/server/src/main/java/org/jboss/as/server/operations/ServerProcessReloadHandler.java @@ -4,6 +4,7 @@ */ package org.jboss.as.server.operations; + import java.util.Locale; import org.jboss.as.controller.AttributeDefinition; @@ -20,6 +21,7 @@ import org.jboss.as.controller.descriptions.ModelDescriptionConstants; import org.jboss.as.controller.operations.common.ProcessReloadHandler; import org.jboss.as.controller.operations.validation.EnumValidator; +import org.jboss.as.controller.persistence.ExtensibleConfigurationPersister; import org.jboss.as.server.ServerEnvironment; import org.jboss.as.server.controller.descriptions.ServerDescriptions; import org.jboss.as.server.logging.ServerLogger; @@ -59,10 +61,12 @@ public class ServerProcessReloadHandler extends ProcessReloadHandler initializeReloa } final boolean finalSuspend = suspend; final boolean finalAdminOnly = adminOnly; + final boolean applyConfigurationExtension = !(context.isNormalServer() || finalAdminOnly) || + (extensibleConfigurationPersister != null && !extensibleConfigurationPersister.hasStored()); final String serverConfig = unmanaged && operation.hasDefined(SERVER_CONFIG.getName()) ? SERVER_CONFIG.resolveModelAttribute(context, operation).asString() : null; @@ -113,6 +119,7 @@ public void doReload(RunningModeControl runningModeControl) { runningModeControl.setUseCurrentConfig(useCurrentConfig); runningModeControl.setNewBootFileName(serverConfig); runningModeControl.setSuspend(finalSuspend); + runningModeControl.setApplyConfigurationExtension(applyConfigurationExtension); } }; } diff --git a/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/persistence/YamlExtensionTestCase.java b/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/persistence/YamlExtensionTestCase.java index 9d43b47a367..8f0bd7f5152 100644 --- a/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/persistence/YamlExtensionTestCase.java +++ b/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/management/persistence/YamlExtensionTestCase.java @@ -197,7 +197,7 @@ public void testFailedDeploymentYaml() throws URISyntaxException, Exception { } catch (RuntimeException ex) { Assert.assertFalse(container.isStarted()); try (final BufferedReader reader = Files.newBufferedReader(basedir.resolve("log").resolve("server.log"), StandardCharsets.UTF_8)) { - Assert.assertTrue(reader.lines().anyMatch(line -> line.contains("WFLYCTL0505: Unsuported deployment yaml file hello.jar with attributes [empty]"))); + Assert.assertTrue(reader.lines().anyMatch(line -> line.contains("WFLYCTL0507: Unsuported deployment yaml file hello.jar with attributes [empty]"))); } } finally { container.stop();