Skip to content

Commit

Permalink
fix: restart nucleus if deployment removes a plugin (#1362)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shaguptashaikh @MikeDombo
shaguptashaikh authored and MikeDombo committed Nov 17, 2022
1 parent fa79548 commit e76262f
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 35 deletions.
84 changes: 50 additions & 34 deletions ...ntests/java/com/aws/greengrass/integrationtests/lifecyclemanager/PluginComponentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@

import java.io.IOException;
import java.net.URISyntaxException;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand All @@ -83,7 +82,6 @@
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -133,14 +131,18 @@ void afterEach() {
}

private void launchAndWait() throws InterruptedException {
launchAndWait(kernel);
}

private void launchAndWait(Kernel k) throws InterruptedException {
CountDownLatch mainRunning = new CountDownLatch(1);
kernel.getContext().addGlobalStateChangeListener((service, oldState, newState) -> {
k.getContext().addGlobalStateChangeListener((service, oldState, newState) -> {
if (service.getName().equals("main") && newState.equals(State.FINISHED)) {
mainRunning.countDown();
}
});
kernel.launch();
thingGroupHelper = kernel.getContext().get(ThingGroupHelper.class);
k.launch();
thingGroupHelper = k.getContext().get(ThingGroupHelper.class);
assertTrue(mainRunning.await(5, TimeUnit.SECONDS));
}

Expand Down Expand Up @@ -226,43 +228,57 @@ void GIVEN_kernel_WHEN_deploy_new_plugin_THEN_plugin_is_loaded_into_JVM(Extensio
}

@Test
void GIVEN_plugin_added_and_removed_WHEN_plugin_added_again_THEN_plugin_is_loaded_into_JVM(ExtensionContext context) throws Exception {
// launch Nucleus
kernel.parseArgs();
void GIVEN_plugin_running_WHEN_plugin_removed_THEN_nucleus_bootstraps(ExtensionContext context) throws Exception {
ignoreExceptionOfType(context, PackageDownloadException.class);
ignoreExceptionOfType(context, IOException.class);
ignoreExceptionOfType(context, SdkClientException.class);

Kernel kernelSpy = spy(kernel.parseArgs());
setupPackageStoreAndConfigWithDigest();
launchAndWait();
String deploymentId = "deployment1";
KernelAlternatives kernelAltsSpy = spy(kernelSpy.getContext().get(KernelAlternatives.class));
kernelSpy.getContext().put(KernelAlternatives.class, kernelAltsSpy);
// In actual workflow, DeploymentService will setup deployment artifacts directory per deployment before
// submitting task. Here in test, it's called explicitly because the directory is required by snapshot file.
kernelSpy.getContext().get(DeploymentDirectoryManager.class)
.createNewDeploymentDirectory(deploymentId);
kernelSpy.getContext().put(KernelUpdateActivator.class,
new KernelUpdateActivator(kernelSpy, kernelSpy.getContext().get(BootstrapManager.class)));

// deploy plugin
submitSampleJobDocument(getPluginDeploymentDocument(System.currentTimeMillis(), "1.0.0",
"first-deployment", FailureHandlingPolicy.DO_NOTHING, componentName), kernel).get(30, TimeUnit.SECONDS);
// launch Nucleus
launchAndWait(kernelSpy);

GreengrassService eg = kernel.locate(componentName);
assertNotNull(kernel.getContext().getvIfExists(componentName));
assertNotNull(kernel.getContext().getvIfExists(eg.getClass()));
// Ensure that the dependency isn't somehow in our class loader already
assertThrows(ClassNotFoundException.class,
() -> Class.forName("com.aws.greengrass.integrationtests.lifecyclemanager.resource.PluginDependency"));

// The class loader is holding on to the jar file. This creates an error when deployment tries to delete the
// jar file. This code is a workaround for such problem.
ClassLoader cl = eg.getClass().getClassLoader();
if (cl instanceof URLClassLoader) {
((URLClassLoader)cl).close();
}
// First deployment to add plugin-1.0.0 to kernel
submitSampleJobDocument(getPluginDeploymentDocument(System.currentTimeMillis(), "1.0.0", deploymentId,
FailureHandlingPolicy.DO_NOTHING, componentName), kernelSpy).get(60, TimeUnit.SECONDS);

//remove plugin
DeploymentDocument doc = getPluginDeploymentDocument(System.currentTimeMillis(), "1.0.0",
"second-deployment", FailureHandlingPolicy.DO_NOTHING, componentName);
doc.setDeploymentPackageConfigurationList(Collections.emptyList());
submitSampleJobDocument(doc, kernel).get(30, TimeUnit.SECONDS);
assertNull(kernel.getContext().getvIfExists(componentName));
assertNull(kernel.getContext().getvIfExists(eg.getClass()));
GreengrassService eg = kernelSpy.locate(componentName);
assertEquals("com.aws.greengrass.integrationtests.lifecyclemanager.resource.APluginService",
eg.getClass().getName());
assertEquals(componentId.getVersion().toString(),
Coerce.toString(eg.getServiceConfig().findLeafChild(VERSION_CONFIG_KEY)));
kernelSpy.getContext().get(EZPlugins.class)
.forName("com.aws.greengrass.integrationtests.lifecyclemanager.resource.PluginDependency");

//re deploy plugin
// setup again because local files removed by cleanup in the previous deployment
setupPackageStoreAndConfigWithDigest();
submitSampleJobDocument(getPluginDeploymentDocument(System.currentTimeMillis(), "1.0.0",
"third-deployment", FailureHandlingPolicy.DO_NOTHING, componentName), kernel).get(30, TimeUnit.SECONDS);

assertNotNull(kernel.getContext().getvIfExists(componentName));
assertNotNull(kernel.getContext().getvIfExists(eg.getClass()));
String deploymentId2 = "deployment2";
// No need to actually verify directory setup or make directory changes here.
doReturn(true).when(kernelAltsSpy).isLaunchDirSetup();
doNothing().when(kernelAltsSpy).prepareBootstrap(eq(deploymentId2));

doNothing().when(kernelSpy).shutdown(anyInt(), eq(REQUEST_RESTART));
// Second deployment to remove plugin from kernel which should enter kernel restart workflow
DeploymentDocument doc2 = getPluginDeploymentDocument(System.currentTimeMillis(), "1.0.0",
deploymentId2, FailureHandlingPolicy.DO_NOTHING, componentName);
doc2.setDeploymentPackageConfigurationList(Collections.emptyList());
assertThrows(TimeoutException.class, () -> submitSampleJobDocument(doc2, kernelSpy)
.get(10, TimeUnit.SECONDS));
verify(kernelSpy).shutdown(eq(30), eq(REQUEST_RESTART));
}

@Test
Expand Down
20 changes: 19 additions & 1 deletion src/main/java/com/aws/greengrass/deployment/bootstrap/BootstrapManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.aws.greengrass.deployment.exceptions.ServiceUpdateException;
import com.aws.greengrass.lifecyclemanager.GreengrassService;
import com.aws.greengrass.lifecyclemanager.Kernel;
import com.aws.greengrass.lifecyclemanager.PluginService;
import com.aws.greengrass.lifecyclemanager.exceptions.InputValidationException;
import com.aws.greengrass.lifecyclemanager.exceptions.ServiceLoadException;
import com.aws.greengrass.logging.api.Logger;
Expand Down Expand Up @@ -46,6 +47,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import javax.annotation.concurrent.NotThreadSafe;
import javax.inject.Inject;

Expand Down Expand Up @@ -136,7 +138,10 @@ public boolean isBootstrapRequired(Map<String, Object> newConfig)
}
});
if (componentsRequiresBootstrapTask.isEmpty()) {
return nucleusConfigValidAndNeedsRestart;
// Force restart if
// 1. any nucleus config change requires restart or
// 2. if any plugin will be removed in the deployment to ensure plugin cleanup
return nucleusConfigValidAndNeedsRestart || willRemovePlugins(serviceConfig);
}
List<String> errors = new ArrayList<>();
// Figure out the dependency order within the subset of components which require changes
Expand All @@ -153,6 +158,19 @@ public boolean isBootstrapRequired(Map<String, Object> newConfig)
return nucleusConfigValidAndNeedsRestart || !bootstrapTaskStatusList.isEmpty();
}

private boolean willRemovePlugins(Map<String, Object> serviceConfig) {
Set<String> pluginsToRemove = kernel.orderedDependencies().stream()
.filter(s -> s instanceof PluginService)
.filter(s -> !serviceConfig.containsKey(s.getName()))
.map(GreengrassService::getName)
.collect(Collectors.toSet());
if (!pluginsToRemove.isEmpty()) {
logger.atInfo().kv("plugins-to-remove", pluginsToRemove)
.log("Bootstrap required for cleaning up plugin(s)");
}
return !pluginsToRemove.isEmpty();
}

private boolean networkProxyHasChanged(Map<String, Object> newNucleusParameters,
DeviceConfiguration currentDeviceConfiguration) {
Map<String, Object> newNetworkProxy =
Expand Down
98 changes: 98 additions & 0 deletions src/test/java/com/aws/greengrass/deployment/bootstrap/BootstrapManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.aws.greengrass.lifecyclemanager.GenericExternalService;
import com.aws.greengrass.lifecyclemanager.GreengrassService;
import com.aws.greengrass.lifecyclemanager.Kernel;
import com.aws.greengrass.lifecyclemanager.PluginService;
import com.aws.greengrass.lifecyclemanager.exceptions.ServiceLoadException;
import com.aws.greengrass.testcommons.testutilities.GGExtension;
import com.aws.greengrass.util.CommitableWriter;
Expand Down Expand Up @@ -179,6 +180,103 @@ void GIVEN_new_component_without_bootstrap_WHEN_check_serviceBootstrapRequired_T
}}));
}

@Test
void GIVEN_deployment_config_WHEN_check_boostrap_required_THEN_boostrap_only_if_plugin_is_removed() throws Exception {
Map<String, Object> runWith = new HashMap<String, Object>() {{
put(DeviceConfiguration.RUN_WITH_DEFAULT_POSIX_USER, "foo:bar");
put(DeviceConfiguration.RUN_WITH_DEFAULT_POSIX_SHELL, "sh");
}};
when(kernel.getContext()).thenReturn(context);
when(context.get(DeviceConfiguration.class)).thenReturn(deviceConfiguration);
when(deviceConfiguration.getRunWithTopic().toPOJO()).thenReturn(runWith);

GenericExternalService nucleus = mock(GenericExternalService.class);
doReturn(false).when(nucleus).isBootstrapRequired(anyMap());
when(kernel.locate(DEFAULT_NUCLEUS_COMPONENT_NAME)).thenReturn(nucleus);

PluginService plugin = mock(PluginService.class);
when(plugin.getName()).thenReturn("SomePlugin");
when(plugin.isBootstrapRequired(anyMap())).thenReturn(false);
when(kernel.locate("SomePlugin")).thenReturn(plugin);

GenericExternalService serviceA = mock(GenericExternalService.class);
when(serviceA.isBootstrapRequired(anyMap())).thenReturn(false);
when(kernel.locate(componentA)).thenReturn(serviceA);

GenericExternalService serviceB = mock(GenericExternalService.class);
when(serviceB.isBootstrapRequired(anyMap())).thenReturn(false);
when(kernel.locate(componentB)).thenReturn(serviceB);

List<GreengrassService> runningServices = Arrays.asList(serviceA, serviceB, plugin);
when(kernel.orderedDependencies()).thenReturn(runningServices);

BootstrapManager bootstrapManager = new BootstrapManager(kernel);

assertTrue(bootstrapManager.isBootstrapRequired(new HashMap<String, Object>() {{
put(SERVICES_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
put(DEFAULT_NUCLEUS_COMPONENT_NAME, new HashMap<String, Object>() {{
put(SERVICE_TYPE_TOPIC_KEY, ComponentType.NUCLEUS.toString());
put(CONFIGURATION_CONFIG_KEY, new HashMap<String, Object>() {{
put(DeviceConfiguration.RUN_WITH_TOPIC, runWith);
}});
}});
put(componentA, Collections.emptyMap());
}});
}}));

assertTrue(bootstrapManager.isBootstrapRequired(new HashMap<String, Object>() {{
put(SERVICES_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
put(DEFAULT_NUCLEUS_COMPONENT_NAME, new HashMap<String, Object>() {{
put(SERVICE_TYPE_TOPIC_KEY, ComponentType.NUCLEUS.toString());
put(CONFIGURATION_CONFIG_KEY, new HashMap<String, Object>() {{
put(DeviceConfiguration.RUN_WITH_TOPIC, runWith);
}});
}});
put(componentA, Collections.emptyMap());
put(componentB, Collections.emptyMap());
}});
}}));

assertFalse(bootstrapManager.isBootstrapRequired(new HashMap<String, Object>() {{
put(SERVICES_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
put(DEFAULT_NUCLEUS_COMPONENT_NAME, new HashMap<String, Object>() {{
put(SERVICE_TYPE_TOPIC_KEY, ComponentType.NUCLEUS.toString());
put(CONFIGURATION_CONFIG_KEY, new HashMap<String, Object>() {{
put(DeviceConfiguration.RUN_WITH_TOPIC, runWith);
}});
}});
put(componentB, Collections.emptyMap());
put(componentA, Collections.emptyMap());
put("SomePlugin", Collections.emptyMap());
}});
}}));

assertFalse(bootstrapManager.isBootstrapRequired(new HashMap<String, Object>() {{
put(SERVICES_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
put(DEFAULT_NUCLEUS_COMPONENT_NAME, new HashMap<String, Object>() {{
put(SERVICE_TYPE_TOPIC_KEY, ComponentType.NUCLEUS.toString());
put(CONFIGURATION_CONFIG_KEY, new HashMap<String, Object>() {{
put(DeviceConfiguration.RUN_WITH_TOPIC, runWith);
}});
}});
put(componentA, Collections.emptyMap());
put("SomePlugin", Collections.emptyMap());
}});
}}));

assertFalse(bootstrapManager.isBootstrapRequired(new HashMap<String, Object>() {{
put(SERVICES_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
put(DEFAULT_NUCLEUS_COMPONENT_NAME, new HashMap<String, Object>() {{
put(SERVICE_TYPE_TOPIC_KEY, ComponentType.NUCLEUS.toString());
put(CONFIGURATION_CONFIG_KEY, new HashMap<String, Object>() {{
put(DeviceConfiguration.RUN_WITH_TOPIC, runWith);
}});
}});
put("SomePlugin", Collections.emptyMap());
}});
}}));
}

@Test
void GIVEN_bootstrap_task_requires_reboot_WHEN_executeAllBootstrapTasksSequentially_THEN_request_reboot() throws Exception {
List<BootstrapTaskStatus> pendingTasks = Arrays.asList(
Expand Down

0 comments on commit e76262f

Please sign in to comment.