fix: timestamp of initial jvmOptions; bootstrap config comparison (aw…

…s-greengrass#908)
Xuqing888 · Apr 6, 2021 · 87d89a6 · 87d89a6
1 parent 2f81f7f
commit 87d89a6
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 5 deletions.
diff --git a/src/main/java/com/aws/greengrass/deployment/DeviceConfiguration.java b/src/main/java/com/aws/greengrass/deployment/DeviceConfiguration.java
@@ -275,7 +275,7 @@ void persistInitialLaunchParams(KernelAlternatives kernelAlts) {
             String jvmOptions = ManagementFactory.getRuntimeMXBean().getInputArguments().stream().sorted()
                     .filter(s -> !s.startsWith(JVM_OPTION_ROOT_PATH)).collect(Collectors.joining(" "));
             kernel.getConfig().lookup(SERVICES_NAMESPACE_TOPIC, getNucleusComponentName(), CONFIGURATION_CONFIG_KEY,
-                    DEVICE_PARAM_JVM_OPTIONS).dflt(jvmOptions);
+                    DEVICE_PARAM_JVM_OPTIONS).withNewerValue(DEFAULT_VALUE_TIMESTAMP + 1, jvmOptions);
 
             kernelAlts.writeLaunchParamsToFile(jvmOptions);
             logger.atInfo().log("Successfully setup Nucleus launch parameters");

diff --git a/src/main/java/com/aws/greengrass/lifecyclemanager/GenericExternalService.java b/src/main/java/com/aws/greengrass/lifecyclemanager/GenericExternalService.java
@@ -238,13 +238,45 @@ public boolean isBootstrapRequired(Map<String, Object> newServiceConfig) {
         }
         Node serviceOldBootstrap = getConfig().findNode(SERVICE_LIFECYCLE_NAMESPACE_TOPIC,
                 Lifecycle.LIFECYCLE_BOOTSTRAP_NAMESPACE_TOPIC);
-        boolean bootstrapStepChanged =  serviceOldBootstrap == null
-                || !serviceOldBootstrap.toPOJO().equals(newServiceLifecycle.get(lifecycleKey));
-        logger.atDebug().log(String.format("Bootstrap is %srequired: bootstrap step %schanged",
-                bootstrapStepChanged ? "" : "not ", bootstrapStepChanged ? "" : "un"));
+        boolean bootstrapStepChanged = serviceOldBootstrap == null || !serviceLifecycleBootstrapEquals(
+                serviceOldBootstrap.toPOJO(), newServiceLifecycle.get(lifecycleKey));
+        if (bootstrapStepChanged) {
+            logger.atDebug().kv("before", serviceOldBootstrap.toPOJO())
+                    .kv("after", newServiceLifecycle.get(lifecycleKey))
+                    .log("Bootstrap is required: bootstrap step changed");
+        } else {
+            logger.atDebug().log("Bootstrap is not required: bootstrap step unchanged");
+        }
         return bootstrapStepChanged;
     }
 
+    private boolean serviceLifecycleBootstrapEquals(Object before, Object after) {
+        if (!(before instanceof Map) || !(after instanceof Map)) {
+            return Objects.equals(before, after);
+        }
+        Map<String, Object> beforeMap = (Map<String, Object>) before;
+        Map<String, Object> afterMap = (Map<String, Object>) after;
+        if (beforeMap.size() != afterMap.size()) {
+            return false;
+        }
+        for (Map.Entry<String, Object> beforeEntry : beforeMap.entrySet()) {
+            CaseInsensitiveString key = new CaseInsensitiveString(beforeEntry.getKey());
+            boolean keyFound = false;
+            for (Map.Entry<String, Object> entry : afterMap.entrySet()) {
+                if (key.equals(new CaseInsensitiveString(entry.getKey())) && Objects.nonNull(entry.getValue())) {
+                    keyFound = true;
+                    if (!Objects.equals(entry.getValue().toString(), beforeEntry.getValue().toString())) {
+                        return false;
+                    }
+                }
+            }
+            if (!keyFound) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * Check if the case-insensitive lifecycle key is defined in the service lifecycle configuration map.
      * @param newServiceLifecycle service lifecycle configuration map

diff --git a/src/test/java/com/aws/greengrass/lifecyclemanager/GenericExternalServiceTest.java b/src/test/java/com/aws/greengrass/lifecyclemanager/GenericExternalServiceTest.java
@@ -126,6 +126,58 @@ void GIVEN_nested_bootstrap_definition_WHEN_isBootstrapRequired_THEN_return_fals
         }}));
     }
 
+    @Test
+    void GIVEN_bootstrap_definition_WHEN_isBootstrapRequired_THEN_key_case_insensitive_value_type_insensitive() {
+        Topics bootstrap = Topics.of(context, Lifecycle.LIFECYCLE_BOOTSTRAP_NAMESPACE_TOPIC, null);
+        bootstrap.createLeafChild("script").withValue("\necho complete\n");
+        bootstrap.createLeafChild("RequiresPrivilege").withValue("true");
+        bootstrap.createLeafChild("Timeout").withValue("120");
+        doReturn(bootstrap).when(config)
+                .findNode(eq(SERVICE_LIFECYCLE_NAMESPACE_TOPIC), eq(Lifecycle.LIFECYCLE_BOOTSTRAP_NAMESPACE_TOPIC));
+
+        assertFalse(ges.isBootstrapRequired(new HashMap<String, Object>() {{
+            put(VERSION_CONFIG_KEY, "1.0.0");
+            put(SERVICE_LIFECYCLE_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
+                put(Lifecycle.LIFECYCLE_BOOTSTRAP_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
+                    put("script", "\necho complete\n");
+                    put("requiresPrivilege", true);
+                    put("Timeout", 120);
+                }});
+            }});
+        }}));
+        assertFalse(ges.isBootstrapRequired(new HashMap<String, Object>() {{
+            put(VERSION_CONFIG_KEY, "1.0.0");
+            put(SERVICE_LIFECYCLE_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
+                put("Bootstrap", new HashMap<String, Object>() {{
+                    put("script", "\necho complete\n");
+                    put("RequiresPrivilege", true);
+                    put("timeout", 120);
+                }});
+            }});
+        }}));
+        assertTrue(ges.isBootstrapRequired(new HashMap<String, Object>() {{
+            put(VERSION_CONFIG_KEY, "1.0.0");
+            put(SERVICE_LIFECYCLE_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
+                put("Bootstrap", new HashMap<String, Object>() {{
+                    put("script", "\necho complete\n");
+                    put("RequiresPrivilege", true);
+                    put("timeout", 100);
+                }});
+            }});
+        }}));
+        assertTrue(ges.isBootstrapRequired(new HashMap<String, Object>() {{
+            put(VERSION_CONFIG_KEY, "1.0.0");
+            put(SERVICE_LIFECYCLE_NAMESPACE_TOPIC, new HashMap<String, Object>() {{
+                put("Bootstrap", new HashMap<String, Object>() {{
+                    put("script", "\necho complete\n");
+                    put("RequiresPrivilege", true);
+                    put("timeout", 120);
+                    put("Setenv", "");
+                }});
+            }});
+        }}));
+    }
+
     @Test
     void GIVEN_runwith_info_WHEN_exec_add_group_THEN_use_runwith() throws Exception {
         ges.runWith = RunWith.builder().user("foo").group("bar").build();