Merge pull request #23939 from vespa-engine/cleanup-after-failed-component-graph-take2

Cleanup after failed component graph take2 [run-systemtest]

14 files changed, 450 insertions, 202 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java
index f0ca5aee8a2..c1cf18582d5 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java
@@ -2,41 +2,48 @@
 package com.yahoo.container.core.config;
 
 import com.yahoo.config.FileReference;
+import com.yahoo.container.di.Osgi.GenerationStatus;
 import com.yahoo.osgi.Osgi;
 import org.osgi.framework.Bundle;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 /**
  * Manages the set of installed and active/inactive bundles.
  *
+ * TODO: This class and the CollisionHook currently only handles a "current" and "previous" generation.
+ *       In order to correctly handle rapid reconfiguration and hence multiple generations, we need to
+ *       consider the graph generation number for each bundle.
+ *
  * @author gjoranv
- * @author Tony Vaagenes
  */
 public class ApplicationBundleLoader {
 
     private static final Logger log = Logger.getLogger(ApplicationBundleLoader.class.getName());
 
-    /**
-     * Map of file refs of active bundles (not scheduled for uninstall) to the installed bundle.
-     *
-     * Used to:
-     * 1. Avoid installing already installed bundles. Just an optimization, installing the same bundle location is a NOP
-     * 2. Start bundles (all are started every time)
-     * 3. Calculate the set of bundles to uninstall
-     */
-    private final Map<FileReference, Bundle> reference2Bundle = new LinkedHashMap<>();
+    // The active bundles for the current application generation.
+    private final Map<FileReference, Bundle> activeBundles = new LinkedHashMap<>();
+
+    // The bundles that are obsolete from the previous generation, but kept in case the generation is reverted.
+    private Map<FileReference, Bundle> obsoleteBundles = Map.of();
+
+    // The bundles that exclusively belong to the current application generation.
+    private Map<FileReference, Bundle> bundlesFromNewGeneration = Map.of();
+
+    private boolean readyForNewBundles = true;
 
     private final Osgi osgi;
     private final FileAcquirerBundleInstaller bundleInstaller;
 
+
     public ApplicationBundleLoader(Osgi osgi, FileAcquirerBundleInstaller bundleInstaller) {
         this.osgi = osgi;
         this.bundleInstaller = bundleInstaller;
@@ -45,57 +52,113 @@ public class ApplicationBundleLoader {
     /**
      * Installs the given set of bundles and returns the set of bundles that is no longer used
      * by the application, and should therefore be scheduled for uninstall.
+     *
+     * TODO: return void, and instead return the bundles to remove from completeGeneration()
      */
     public synchronized Set<Bundle> useBundles(List<FileReference> newFileReferences) {
+        if (! readyForNewBundles)
+            throw new IllegalStateException("Bundles must be committed or reverted before using new bundles.");
 
-        Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newFileReferences);
-        Set<Bundle> bundlesToUninstall = getObsoleteBundles(obsoleteReferences);
+        obsoleteBundles = removeObsoleteBundles(newFileReferences);
+        Set<Bundle> bundlesToUninstall = new LinkedHashSet<>(obsoleteBundles.values());
         log.info("Bundles to schedule for uninstall: " + bundlesToUninstall);
 
         osgi.allowDuplicateBundles(bundlesToUninstall);
-        removeInactiveFileReferences(obsoleteReferences);
 
-        installBundles(newFileReferences);
-        BundleStarter.startBundles(reference2Bundle.values());
+        bundlesFromNewGeneration = installBundles(newFileReferences);
+        BundleStarter.startBundles(activeBundles.values());
         log.info(installedBundlesMessage());
 
+        readyForNewBundles = false;
+
         return bundlesToUninstall;
     }
 
-    private Set<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) {
-        Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundle.keySet());
-        obsoleteReferences.removeAll(newReferences);
-        return obsoleteReferences;
+    public synchronized Collection<Bundle> completeGeneration(GenerationStatus status) {
+        Collection<Bundle> ret = List.of();
+        if (readyForNewBundles) return ret;
+
+        if (status == GenerationStatus.SUCCESS) {
+            commitBundles();
+        } else {
+            ret = revertToPreviousGeneration();
+        }
+        readyForNewBundles = true;
+        return ret;
     }
 
     /**
-     * Returns the bundles that will not be retained by the new application generation.
+     * Commit to the current set of bundles. Must be called after the component graph creation proved successful,
+     * to prevent uninstalling bundles unintentionally upon a future call to {@link #revertToPreviousGeneration()}.
      */
-    private Set<Bundle> getObsoleteBundles(Set<FileReference> obsoleteReferences) {
-        return obsoleteReferences.stream().map(reference2Bundle::get).collect(Collectors.toSet());
+    private void commitBundles() {
+        bundlesFromNewGeneration = Map.of();
+        obsoleteBundles = Map.of();
+        readyForNewBundles = true;
     }
 
-    private void removeInactiveFileReferences(Set<FileReference> fileReferencesToRemove) {
-        fileReferencesToRemove.forEach(reference2Bundle::remove);
+    /**
+     * Restores state from the previous application generation and returns the set of bundles that
+     * exclusively belongs to the latest (failed) application generation. Uninstalling must
+     * be done by the Deconstructor as they may still be needed by components from the failed gen.
+     */
+    private Collection<Bundle> revertToPreviousGeneration() {
+        log.info("Reverting to previous generation with bundles: " + obsoleteBundles);
+        log.info("Bundles from latest generation will be removed: " + bundlesFromNewGeneration);
+        activeBundles.putAll(obsoleteBundles);
+        bundlesFromNewGeneration.forEach(activeBundles::remove);
+        Collection<Bundle> ret = bundlesFromNewGeneration.values();
+
+        // For correct operation of the CollisionHook (more specifically its FindHook implementation), the set of
+        // allowed duplicates must reflect the next set of bundles to uninstall, which is now the bundles from the
+        // failed generation.
+        osgi.allowDuplicateBundles(ret);
+
+        // Clear restore info in case this method is called multiple times, for some reason.
+        bundlesFromNewGeneration = Map.of();
+        obsoleteBundles = Map.of();
+
+        readyForNewBundles = true;
+        return ret;
     }
 
-    private void installBundles(List<FileReference> references) {
+    /**
+     * Calculates the set of bundles that are not needed by the new application generation and
+     * removes them from the map of active bundles.
+     *
+     * Returns the map of bundles that are not needed by the new application generation.
+     */
+    private Map<FileReference, Bundle> removeObsoleteBundles(List<FileReference> newReferences) {
+        Map<FileReference, Bundle> obsoleteReferences = new LinkedHashMap<>(activeBundles);
+        newReferences.forEach(obsoleteReferences::remove);
+
+        obsoleteReferences.forEach(activeBundles::remove);
+        return obsoleteReferences;
+    }
+
+    /**
+     * Returns the set of new bundles that were installed.
+     */
+    private Map<FileReference, Bundle> installBundles(List<FileReference> references) {
         Set<FileReference> bundlesToInstall = new HashSet<>(references);
 
         // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle.
-        bundlesToInstall.removeAll(reference2Bundle.keySet());
+        bundlesToInstall.removeAll(activeBundles.keySet());
 
-        if (!bundlesToInstall.isEmpty()) {
-            if (bundleInstaller.hasFileDistribution()) {
-                installWithFileDistribution(bundlesToInstall, bundleInstaller);
-            } else {
-                log.warning("Can't retrieve bundles since file distribution is disabled.");
-            }
+        if (bundlesToInstall.isEmpty()) return Map.of();
+
+        if (bundleInstaller.hasFileDistribution()) {
+            return installWithFileDistribution(bundlesToInstall, bundleInstaller);
+        } else {
+            log.warning("Can't retrieve bundles since file distribution is disabled.");
+            return Map.of();
         }
     }
 
-    private void installWithFileDistribution(Set<FileReference> bundlesToInstall,
-                                             FileAcquirerBundleInstaller bundleInstaller) {
+    private Map<FileReference, Bundle> installWithFileDistribution(Set<FileReference> bundlesToInstall,
+                                                                   FileAcquirerBundleInstaller bundleInstaller) {
+        var newBundles = new LinkedHashMap<FileReference, Bundle>();
+
         for (FileReference reference : bundlesToInstall) {
             try {
                 log.info("Installing bundle with reference '" + reference.value() + "'");
@@ -107,12 +170,14 @@ public class ApplicationBundleLoader {
                 if (bundles.size() > 1  && osgi.hasFelixFramework()) {
                     throw new RuntimeException("Bundle '" + bundles.get(0).getSymbolicName() + "' tried to pre-install bundles from disk.");
                 }
-                reference2Bundle.put(reference, bundles.get(0));
+                activeBundles.put(reference, bundles.get(0));
+                newBundles.put(reference, bundles.get(0));
             }
             catch(Exception e) {
                 throw new RuntimeException("Could not install bundle with reference '" + reference + "'", e);
             }
         }
+        return newBundles;
     }
 
     private String installedBundlesMessage() {
@@ -126,7 +191,7 @@ public class ApplicationBundleLoader {
 
     // Only for testing
     List<FileReference> getActiveFileReferences() {
-        return new ArrayList<>(reference2Bundle.keySet());
+        return new ArrayList<>(activeBundles.keySet());
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
index 363ff26eea9..f573de1f4ac 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
@@ -98,10 +98,15 @@ public class HandlersConfigurerDi {
         }
 
         @Override
-        public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles) {
-            log.info("Installing bundles from the latest application");
+        public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles, long generation) {
+            log.info("Installing bundles for application generation " + generation);
             return applicationBundleLoader.useBundles(new ArrayList<>(bundles));
         }
+
+        @Override
+        public Collection<Bundle> completeBundleGeneration(GenerationStatus status) {
+            return applicationBundleLoader.completeGeneration(status);
+        }
     }
 
     /**
diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java
index dbf2ba4a9a8..c34e393ac02 100644
--- a/container-core/src/main/java/com/yahoo/container/di/Container.java
+++ b/container-core/src/main/java/com/yahoo/container/di/Container.java
@@ -67,25 +67,30 @@ public class Container {
         this.retriever = new ConfigRetriever(bootstrapKeys, subscriberFactory);
     }
 
-    public Container(SubscriberFactory subscriberFactory, String configId, ComponentDeconstructor destructor) {
-        this(subscriberFactory, configId, destructor, new Osgi() {
-        });
-    }
-
+    // TODO: try to simplify by returning the result even when the graph failed, instead of throwing here.
     public ComponentGraphResult waitForNextGraphGeneration(ComponentGraph oldGraph, Injector fallbackInjector, boolean isInitializing) {
         try {
+            ComponentGraph newGraph;
             Collection<Bundle> obsoleteBundles = new HashSet<>();
-            ComponentGraph newGraph = waitForNewConfigGenAndCreateGraph(oldGraph, fallbackInjector, isInitializing, obsoleteBundles);
-            newGraph.reuseNodes(oldGraph);
+            try {
+                newGraph = waitForNewConfigGenAndCreateGraph(oldGraph, fallbackInjector, isInitializing, obsoleteBundles);
+                newGraph.reuseNodes(oldGraph);
+            } catch (Throwable t) {
+                log.warning("Failed to set up component graph - uninstalling latest bundles. Bootstrap generation: " + getBootstrapGeneration());
+                Collection<Bundle> newBundlesFromFailedGen = osgi.completeBundleGeneration(Osgi.GenerationStatus.FAILURE);
+                deconstructComponentsAndBundles(getBootstrapGeneration(), newBundlesFromFailedGen, List.of());
+                throw t;
+            }
             try {
                 constructComponents(newGraph);
             } catch (Throwable e) {
-                log.log(Level.WARNING, String.format(
-                        "Failed to construct graph for generation '%d' - scheduling partial graph for deconstruction",
-                        newGraph.generation()), e);
-                deconstructFailedGraph(oldGraph, newGraph);
+                log.warning("Failed to construct components for generation '" + newGraph.generation() + "' - scheduling partial graph for deconstruction");
+                Collection<Bundle> newBundlesFromFailedGen = osgi.completeBundleGeneration(Osgi.GenerationStatus.FAILURE);
+                deconstructFailedGraph(oldGraph, newGraph, newBundlesFromFailedGen);
                 throw e;
             }
+            // TODO: take obsoleteBundles as return value here!
+            osgi.completeBundleGeneration(Osgi.GenerationStatus.SUCCESS);
             Runnable cleanupTask = createPreviousGraphDeconstructionTask(oldGraph, newGraph, obsoleteBundles);
             return new ComponentGraphResult(newGraph, cleanupTask);
         } catch (Throwable t) {
@@ -168,7 +173,7 @@ public class Container {
         return componentGraph;
     }
 
-    private void deconstructFailedGraph(ComponentGraph currentGraph, ComponentGraph failedGraph) {
+    private void deconstructFailedGraph(ComponentGraph currentGraph, ComponentGraph failedGraph, Collection<Bundle> bundlesFromFailedGraph) {
         Set<Object> currentComponents = Collections.newSetFromMap(new IdentityHashMap<>(currentGraph.size()));
         currentComponents.addAll(currentGraph.allConstructedComponentsAndProviders());
 
@@ -176,7 +181,11 @@ public class Container {
         for (Object component : failedGraph.allConstructedComponentsAndProviders()) {
             if (!currentComponents.contains(component)) unusedComponents.add(component);
         }
-        destructor.deconstruct(failedGraph.generation(), unusedComponents, List.of());
+        deconstructComponentsAndBundles(failedGraph.generation(), bundlesFromFailedGraph, unusedComponents);
+    }
+
+    private void deconstructComponentsAndBundles(long generation, Collection<Bundle> bundlesFromFailedGraph, List<Object> unusedComponents) {
+        destructor.deconstruct(generation, unusedComponents, bundlesFromFailedGraph);
     }
 
     private Runnable createPreviousGraphDeconstructionTask(ComponentGraph oldGraph,
@@ -196,7 +205,7 @@ public class Container {
 
     private Set<Bundle> installApplicationBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) {
         ApplicationBundlesConfig applicationBundlesConfig = getConfig(applicationBundlesConfigKey, configsIncludingBootstrapConfigs);
-        return osgi.useApplicationBundles(applicationBundlesConfig.bundles());
+        return osgi.useApplicationBundles(applicationBundlesConfig.bundles(), getBootstrapGeneration());
     }
 
     private ComponentGraph createComponentGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs,
@@ -268,7 +277,8 @@ public class Container {
     public void shutdown(ComponentGraph graph) {
         shutdownConfigRetriever();
         if (graph != null) {
-            scheduleGraphForDeconstruction(graph);
+            // As we are shutting down, there is no need to uninstall bundles.
+            deconstructComponentsAndBundles(graph.generation(), List.of(), graph.allConstructedComponentsAndProviders());
             destructor.shutdown();
         }
     }
@@ -282,11 +292,6 @@ public class Container {
         subscriberFactory.reloadActiveSubscribers(generation);
     }
 
-    private void scheduleGraphForDeconstruction(ComponentGraph graph) {
-        // This is only used for shutdown and cleanup of failed graph, so no need to uninstall any bundles.
-        destructor.deconstruct(graph.generation(), graph.allConstructedComponentsAndProviders(), List.of());
-    }
-
     public static <T extends ConfigInstance> T getConfig(ConfigKey<T> key,
                                                          Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configs) {
         ConfigInstance inst = configs.get(key);
diff --git a/container-core/src/main/java/com/yahoo/container/di/Osgi.java b/container-core/src/main/java/com/yahoo/container/di/Osgi.java
index a3ebc909a6e..c13ebca6f62 100644
--- a/container-core/src/main/java/com/yahoo/container/di/Osgi.java
+++ b/container-core/src/main/java/com/yahoo/container/di/Osgi.java
@@ -9,7 +9,6 @@ import org.osgi.framework.Bundle;
 
 import java.util.Collection;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import static java.util.Collections.emptySet;
 
@@ -17,27 +16,46 @@ import static java.util.Collections.emptySet;
  * This interface has default implementations of all methods, to allow using it
  * for testing, instead of mocking or a test implementation.
  *
+ * TODO: remove test code from this interface.
+ *
  * @author gjoranv
  * @author Tony Vaagenes
  * @author ollivir
  */
 public interface Osgi {
 
+    enum GenerationStatus { SUCCESS, FAILURE }
+
     default void installPlatformBundles(Collection<String> bundlePaths) {
-        //System.out.println("installPlatformBundles " + bundlePaths);
     }
 
     /**
-     * Returns the set of bundles that is not used by the current application generation,
+     * TODO: return void and let all obsolete bundles be returned by completeBundleGeneration
+     *
+     * Returns the set of bundles that is not needed by the new application generation,
      * and therefore should be scheduled for uninstalling.
+     *
+     * @param bundles The bundles for the new application.
+     * @param generation The generation number of the new application.
+     * @return the set of bundles that is not needed by the new application generation,
+     */
+    default Set<Bundle> useApplicationBundles(Collection<FileReference> bundles, long generation) {
+        return emptySet();
+    }
+
+    /**
+     * If the current generation is a failure, all state related to application bundles is reverted to
+     * the previous generation. The set of bundles that was exclusively needed by the new generation,
+     * and therefore should be scheduled for uninstalling, is returned.
+     *
+     * @param status The success or failure of the new generation
+     * @return The set of bundles that are no longer needed by the latest good generation.
      */
-    default Set<Bundle> useApplicationBundles(Collection<FileReference> bundles) {
-        //System.out.println("useBundles " + bundles.stream().map(Object::toString).collect(Collectors.joining(", ")));
+    default Collection<Bundle> completeBundleGeneration(GenerationStatus status) {
         return emptySet();
     }
 
     default Class<?> resolveClass(BundleInstantiationSpecification spec) {
-        //System.out.println("resolving class " + spec.classId);
         try {
             return Class.forName(spec.classId.getName());
         } catch (ClassNotFoundException e) {
@@ -46,7 +64,6 @@ public interface Osgi {
     }
 
     default Bundle getBundle(ComponentSpecification spec) {
-        //System.out.println("resolving bundle " + spec);
         return new MockBundle();
     }
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/Osgi.java b/container-core/src/main/java/com/yahoo/osgi/Osgi.java
index c8fa901dd0a..10cd41cce08 100644
--- a/container-core/src/main/java/com/yahoo/osgi/Osgi.java
+++ b/container-core/src/main/java/com/yahoo/osgi/Osgi.java
@@ -22,6 +22,7 @@ public interface Osgi {
 
     List<Bundle> install(String absolutePath);
 
+    /** Sets the collection of bundles to allow duplicates for. */
     void allowDuplicateBundles(Collection<Bundle> bundles);
 
     default boolean hasFelixFramework() {
diff --git a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java
index cf8cc6053cd..fdd44fe358d 100644
--- a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java
+++ b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java
@@ -1,19 +1,22 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.core.config;
 
-import com.yahoo.config.FileReference;
-import com.yahoo.filedistribution.fileacquirer.FileAcquirer;
-import com.yahoo.filedistribution.fileacquirer.MockFileAcquirer;
-import com.yahoo.osgi.Osgi;
+import com.yahoo.container.di.Osgi.GenerationStatus;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.osgi.framework.Bundle;
 
+import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
+import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_1;
+import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_1_REF;
+import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_2;
+import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_2_REF;
+import static com.yahoo.container.core.config.BundleTestUtil.testBundles;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
@@ -21,20 +24,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class ApplicationBundleLoaderTest {
 
-    private static final FileReference BUNDLE_1_REF = new FileReference("bundle-1");
-    private static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value());
-    private static final FileReference BUNDLE_2_REF = new FileReference("bundle-2");
-    private static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value());
-
     private ApplicationBundleLoader bundleLoader;
     private TestOsgi osgi;
 
     @BeforeEach
     public void setup() {
         osgi = new TestOsgi(testBundles());
-        var bundleInstaller = new TestBundleInstaller(MockFileAcquirer.returnFile(null));
-
-        bundleLoader = new ApplicationBundleLoader(osgi, bundleInstaller);
+        bundleLoader = osgi.bundleLoader();
     }
 
     @Test
@@ -53,8 +49,28 @@ public class ApplicationBundleLoaderTest {
     }
 
     @Test
+    void generation_must_be_marked_complete_before_using_new_bundles() {
+        bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        assertThrows(IllegalStateException.class,
+                     () -> bundleLoader.useBundles(List.of(BUNDLE_1_REF)));
+    }
+
+    @Test
+    void no_bundles_are_marked_obsolete_upon_reconfig_with_unchanged_bundles() {
+        bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        bundleLoader.completeGeneration(GenerationStatus.SUCCESS);
+
+        Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF));
+
+        // No bundles are obsolete
+        assertTrue(obsoleteBundles.isEmpty());
+    }
+
+    @Test
     void new_bundle_can_be_installed_in_reconfig() {
         bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        bundleLoader.completeGeneration(GenerationStatus.SUCCESS);
+
         Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF));
 
         // No bundles are obsolete
@@ -70,7 +86,6 @@ public class ApplicationBundleLoaderTest {
         assertEquals(BUNDLE_1.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName());
         assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(1).getSymbolicName());
 
-
         // Both file references are active
         assertEquals(2, bundleLoader.getActiveFileReferences().size());
         assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0));
@@ -80,6 +95,8 @@ public class ApplicationBundleLoaderTest {
     @Test
     void unused_bundle_is_marked_obsolete_after_reconfig() {
         bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        bundleLoader.completeGeneration(GenerationStatus.SUCCESS);
+
         Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_2_REF));
 
         // The returned set of obsolete bundles contains bundle-1
@@ -101,22 +118,44 @@ public class ApplicationBundleLoaderTest {
     }
 
 
-    private static Map<String, Bundle> testBundles() {
-        return Map.of(BUNDLE_1_REF.value(), BUNDLE_1,
-                      BUNDLE_2_REF.value(), BUNDLE_2);
+    @Test
+    void previous_generation_can_be_restored_after_a_failed_reconfig() {
+        bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        bundleLoader.completeGeneration(GenerationStatus.SUCCESS);
+
+        Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_2_REF));
+        assertEquals(BUNDLE_1.getSymbolicName(), obsoleteBundles.iterator().next().getSymbolicName());
+
+        // Revert to the previous generation, as will be done upon a failed reconfig.
+        Collection<Bundle> bundlesToUninstall = bundleLoader.completeGeneration(GenerationStatus.FAILURE);
+
+        assertEquals(1, bundlesToUninstall.size());
+        assertEquals(BUNDLE_2.getSymbolicName(), bundlesToUninstall.iterator().next().getSymbolicName());
+
+        // The bundle from the failed generation is not seen as current. It will still be installed,
+        // as uninstalling is handled by the Deconstructor, not included in this test setup.
+        assertEquals(1, osgi.getCurrentBundles().size());
+        assertEquals(BUNDLE_1.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName());
+
+        // Only the bundle-1 file reference is active, bundle-2 is removed.
+        assertEquals(1, bundleLoader.getActiveFileReferences().size());
+        assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0));
     }
 
-    static class TestBundleInstaller extends FileAcquirerBundleInstaller {
+    @Test
+    void bundles_from_committed_config_generation_are_not_uninstalled_upon_future_failed_reconfig() {
+        bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        bundleLoader.completeGeneration(GenerationStatus.SUCCESS);
 
-        TestBundleInstaller(FileAcquirer fileAcquirer) {
-            super(fileAcquirer);
-        }
+        // Revert to the previous generation, as will be done upon a failed reconfig.
+        Collection<Bundle> bundlesToUninstall = bundleLoader.completeGeneration(GenerationStatus.FAILURE);
 
-        @Override
-        public List<Bundle> installBundles(FileReference reference, Osgi osgi) {
-            return osgi.install(reference.value());
-        }
+        assertEquals(0, bundlesToUninstall.size());
+        assertEquals(1, osgi.getCurrentBundles().size());
 
+        Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF));
+        assertTrue(obsoleteBundles.isEmpty());
+        assertEquals(1, osgi.getCurrentBundles().size());
     }
 
 }
diff --git a/container-core/src/test/java/com/yahoo/container/core/config/BundleTestUtil.java b/container-core/src/test/java/com/yahoo/container/core/config/BundleTestUtil.java
new file mode 100644
index 00000000000..fddbeaf42be
--- /dev/null
+++ b/container-core/src/test/java/com/yahoo/container/core/config/BundleTestUtil.java
@@ -0,0 +1,39 @@
+package com.yahoo.container.core.config;
+
+import com.yahoo.config.FileReference;
+import com.yahoo.filedistribution.fileacquirer.FileAcquirer;
+import com.yahoo.osgi.Osgi;
+import org.osgi.framework.Bundle;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author gjoranv
+ */
+public class BundleTestUtil {
+
+    public static final FileReference BUNDLE_1_REF = new FileReference("bundle-1");
+    public static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value());
+    public static final FileReference BUNDLE_2_REF = new FileReference("bundle-2");
+    public static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value());
+
+    public static Map<String, Bundle> testBundles() {
+        return Map.of(BUNDLE_1_REF.value(), BUNDLE_1,
+                      BUNDLE_2_REF.value(), BUNDLE_2);
+    }
+
+    public static class TestBundleInstaller extends FileAcquirerBundleInstaller {
+
+        TestBundleInstaller(FileAcquirer fileAcquirer) {
+            super(fileAcquirer);
+        }
+
+        @Override
+        public List<Bundle> installBundles(FileReference reference, Osgi osgi) {
+            return osgi.install(reference.value());
+        }
+
+    }
+
+}
diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java
index 4a607520b2a..17709844f99 100644
--- a/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java
+++ b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java
@@ -16,7 +16,7 @@ import java.util.List;
 /**
  * @author gjoranv
  */
-class TestBundle extends MockBundle {
+public class TestBundle extends MockBundle {
 
     private static final BundleRevision revision = new TestBundleRevision();
 
@@ -24,7 +24,7 @@ class TestBundle extends MockBundle {
 
     boolean started = false;
 
-    TestBundle(String symbolicName) {
+    public TestBundle(String symbolicName) {
         this.symbolicName = symbolicName;
     }
 
diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java
index 2e22af889ed..d634d7b7ce2 100644
--- a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java
+++ b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java
@@ -1,6 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.core.config;
 
+import com.yahoo.config.FileReference;
+import com.yahoo.filedistribution.fileacquirer.MockFileAcquirer;
 import com.yahoo.osgi.MockOsgi;
 import org.osgi.framework.Bundle;
 
@@ -8,21 +10,28 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * @author gjoranv
  */
-class TestOsgi extends MockOsgi {
+public class TestOsgi extends MockOsgi implements com.yahoo.container.di.Osgi {
 
+    private final ApplicationBundleLoader bundleLoader;
     private final Map<String, Bundle> availableBundles;
 
     private final List<Bundle> installedBundles = new ArrayList<>();
     private final List<Bundle> allowedDuplicates = new ArrayList<>();
 
-    TestOsgi(Map<String, Bundle> availableBundles) {
+    public TestOsgi(Map<String, Bundle> availableBundles) {
         this.availableBundles = availableBundles;
+
+        var bundleInstaller = new BundleTestUtil.TestBundleInstaller(MockFileAcquirer.returnFile(null));
+        bundleLoader = new ApplicationBundleLoader(this, bundleInstaller);
     }
 
+    public ApplicationBundleLoader bundleLoader() { return bundleLoader; }
+
     @Override
     public List<Bundle> install(String fileReferenceValue) {
         if (! availableBundles.containsKey(fileReferenceValue))
@@ -51,7 +60,21 @@ class TestOsgi extends MockOsgi {
 
     @Override
     public void allowDuplicateBundles(Collection<Bundle> bundles) {
+        allowedDuplicates.clear();
         allowedDuplicates.addAll(bundles);
     }
 
+    @Override
+    public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles, long generation) {
+        return bundleLoader.useBundles(new ArrayList<>(bundles));
+    }
+
+    @Override
+    public Collection<Bundle> completeBundleGeneration(GenerationStatus status) {
+        return bundleLoader.completeGeneration(status);
+    }
+
+    public void removeBundle(Bundle bundle) {
+        installedBundles.remove(bundle);
+    }
 }
diff --git a/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java
index 04091c25ad6..c0d691c8a5a 100644
--- a/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java
+++ b/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java
@@ -1,7 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.di;
 
-import com.google.inject.Guice;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.config.di.IntConfig;
 import com.yahoo.config.test.TestConfig;
@@ -11,16 +10,21 @@ import com.yahoo.container.di.componentgraph.core.ComponentGraphTest.SimpleCompo
 import com.yahoo.container.di.componentgraph.core.ComponentNode.ComponentConstructorException;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
-import org.osgi.framework.Bundle;
 
-import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
-import static org.junit.jupiter.api.Assertions.*;
+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.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author Tony Vaagenes
@@ -88,6 +92,28 @@ public class ContainerTest extends ContainerTestBase {
         container.shutdownConfigRetriever();
     }
 
+    @Test
+    void bundle_from_previous_generation_is_uninstalled_when_not_used_in_the_new_generation() {
+        ComponentEntry component1 = new ComponentEntry("component1", SimpleComponent.class);
+        ComponentEntry component2 = new ComponentEntry("component2", SimpleComponent.class);
+
+        writeBootstrapConfigsWithBundles(List.of("bundle-1"), List.of(component1));
+        Container container = newContainer(dirConfigSource);
+        ComponentGraph graph = getNewComponentGraph(container);
+
+        // bundle-1 is installed
+        assertEquals(1, osgi.getBundles().length);
+        assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName());
+
+        writeBootstrapConfigsWithBundles(List.of("bundle-2"), List.of(component2));
+        container.reloadConfig(2);
+        getNewComponentGraph(container, graph);
+
+        // bundle-2 is installed, bundle-1 has been uninstalled
+        assertEquals(1, osgi.getBundles().length);
+        assertEquals("bundle-2", osgi.getBundles()[0].getSymbolicName());
+    }
+
     //@Test TODO
     public void deconstructor_is_given_guice_components() {
     }
@@ -120,6 +146,7 @@ public class ContainerTest extends ContainerTestBase {
         }
     }
 
+    // Failure in component construction phase
     @Test
     void previous_graph_is_retained_when_new_graph_contains_component_that_throws_exception_in_ctor() {
         ComponentEntry simpleComponentEntry = new ComponentEntry("simpleComponent", SimpleComponent.class);
@@ -132,14 +159,7 @@ public class ContainerTest extends ContainerTestBase {
 
         writeBootstrapConfigs("thrower", ComponentThrowingExceptionInConstructor.class);
         container.reloadConfig(2);
-        try {
-            currentGraph = getNewComponentGraph(container, currentGraph);
-            fail("Expected exception");
-        } catch (ComponentConstructorException ignored) {
-            // Expected, do nothing
-        } catch (Throwable t) {
-            fail("Expected ComponentConstructorException");
-        }
+        assertNewComponentGraphFails(container, currentGraph, ComponentConstructorException.class);
         assertEquals(1, currentGraph.generation());
 
         // Also verify that next reconfig is successful
@@ -155,6 +175,30 @@ public class ContainerTest extends ContainerTestBase {
     }
 
     @Test
+    void bundle_from_generation_that_fails_in_component_construction_is_uninstalled() {
+        ComponentEntry simpleComponentEntry = new ComponentEntry("simpleComponent", SimpleComponent.class);
+        ComponentEntry throwingComponentEntry = new ComponentEntry("throwingComponent", ComponentThrowingExceptionInConstructor.class);
+
+        writeBootstrapConfigsWithBundles(List.of("bundle-1"), List.of(simpleComponentEntry));
+        Container container = newContainer(dirConfigSource);
+        ComponentGraph currentGraph = getNewComponentGraph(container);
+
+        // bundle-1 is installed
+        assertEquals(1, osgi.getBundles().length);
+        assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName());
+
+        writeBootstrapConfigsWithBundles(List.of("bundle-2"), List.of(throwingComponentEntry));
+        container.reloadConfig(2);
+        assertNewComponentGraphFails(container, currentGraph, ComponentConstructorException.class);
+        assertEquals(1, currentGraph.generation());
+
+        // bundle-1 is kept, bundle-2 has been uninstalled
+        assertEquals(1, osgi.getBundles().length);
+        assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName());
+    }
+
+    // Failure in graph creation phase
+    @Test
     void previous_graph_is_retained_when_new_graph_throws_exception_for_missing_config() {
         ComponentEntry simpleComponentEntry = new ComponentEntry("simpleComponent", SimpleComponent.class);
 
@@ -167,15 +211,42 @@ public class ContainerTest extends ContainerTestBase {
         writeBootstrapConfigs("thrower", ComponentThrowingExceptionForMissingConfig.class);
         dirConfigSource.writeConfig("test", "stringVal \"myString\"");
         container.reloadConfig(2);
+        assertNewComponentGraphFails(container, currentGraph, IllegalArgumentException.class);
+        assertEquals(1, currentGraph.generation());
+    }
+
+    @Test
+    void bundle_from_generation_that_throws_in_graph_creation_phase_is_uninstalled() {
+        ComponentEntry simpleComponent = new ComponentEntry("simpleComponent", SimpleComponent.class);
+        ComponentEntry configThrower = new ComponentEntry("configThrower", ComponentThrowingExceptionForMissingConfig.class);
+
+        writeBootstrapConfigsWithBundles(List.of("bundle-1"), List.of(simpleComponent));
+        Container container = newContainer(dirConfigSource);
+        ComponentGraph currentGraph = getNewComponentGraph(container);
+
+        // bundle-1 is installed
+        assertEquals(1, osgi.getBundles().length);
+        assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName());
+
+        writeBootstrapConfigsWithBundles(List.of("bundle-2"), List.of(configThrower));
+        dirConfigSource.writeConfig("test", "stringVal \"myString\"");
+        container.reloadConfig(2);
+
+        assertNewComponentGraphFails(container, currentGraph, IllegalArgumentException.class);
+        assertEquals(1, currentGraph.generation());
+
+        // bundle-1 is kept, bundle-2 has been uninstalled
+        assertEquals(1, osgi.getBundles().length);
+        assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName());
+    }
+
+    private void assertNewComponentGraphFails(Container container, ComponentGraph currentGraph, Class<? extends RuntimeException> exception) {
         try {
-            currentGraph = getNewComponentGraph(container, currentGraph);
+            getNewComponentGraph(container, currentGraph);
             fail("Expected exception");
-        } catch (IllegalArgumentException ignored) {
-            // Expected, do nothing
-        } catch (Throwable t) {
-            fail("Expected IllegalArgumentException");
+        } catch (Exception e) {
+            assertEquals(exception, e.getClass());
         }
-        assertEquals(1, currentGraph.generation());
     }
 
     @Test
@@ -198,7 +269,7 @@ public class ContainerTest extends ContainerTestBase {
         try {
             newGraph.get(1, TimeUnit.SECONDS);
             fail("Expected waiting for new config.");
-        } catch (Exception ignored) {
+        } catch (TimeoutException ignored) {
             // expect to time out
         }
 
@@ -303,38 +374,6 @@ public class ContainerTest extends ContainerTestBase {
         }
     }
 
-    public static class TestDeconstructor implements ComponentDeconstructor {
-        @Override
-        public void deconstruct(long generation, List<Object> components, Collection<Bundle> bundles) {
-            components.forEach(component -> {
-                if (component instanceof DestructableComponent) {
-                    DestructableComponent vespaComponent = (DestructableComponent) component;
-                    vespaComponent.deconstruct();
-                }
-            });
-            if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles");
-        }
-    }
-
-    private static Container newContainer(DirConfigSource dirConfigSource,
-                                          ComponentDeconstructor deconstructor) {
-        return new Container(new CloudSubscriberFactory(dirConfigSource.configSource), dirConfigSource.configId(), deconstructor);
-    }
-
-    private static Container newContainer(DirConfigSource dirConfigSource) {
-        return newContainer(dirConfigSource, new TestDeconstructor());
-    }
-
-    ComponentGraph getNewComponentGraph(Container container, ComponentGraph oldGraph) {
-        Container.ComponentGraphResult result = container.waitForNextGraphGeneration(oldGraph, Guice.createInjector(), true);
-        result.oldComponentsCleanupTask().run();
-        return result.newGraph();
-    }
-
-    ComponentGraph getNewComponentGraph(Container container) {
-        return container.waitForNextGraphGeneration(new ComponentGraph(), Guice.createInjector(), true).newGraph();
-    }
-
     private ComponentTakingConfig createComponentTakingConfig(ComponentGraph componentGraph) {
         return componentGraph.getInstance(ComponentTakingConfig.class);
     }
diff --git a/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java
index 4383bd08f71..7442eb2068d 100644
--- a/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java
+++ b/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java
@@ -2,9 +2,8 @@
 package com.yahoo.container.di;
 
 import com.google.inject.Guice;
-import com.yahoo.component.ComponentSpecification;
-import com.yahoo.config.FileReference;
-import com.yahoo.container.bundle.BundleInstantiationSpecification;
+import com.yahoo.container.core.config.BundleTestUtil;
+import com.yahoo.container.core.config.TestOsgi;
 import com.yahoo.container.di.ContainerTest.ComponentTakingConfig;
 import com.yahoo.container.di.componentgraph.core.ComponentGraph;
 import org.junit.jupiter.api.AfterEach;
@@ -14,9 +13,7 @@ import org.osgi.framework.Bundle;
 
 import java.io.File;
 import java.util.Collection;
-import java.util.Set;
-
-import static java.util.Collections.emptySet;
+import java.util.List;
 
 /**
  * @author Tony Vaagenes
@@ -27,6 +24,7 @@ public class ContainerTestBase {
 
     private ComponentGraph componentGraph;
     protected DirConfigSource dirConfigSource = null;
+    protected TestOsgi osgi;
 
     @TempDir
     File tmpDir;
@@ -34,6 +32,8 @@ public class ContainerTestBase {
     @BeforeEach
     public void setup() {
         dirConfigSource = new DirConfigSource(tmpDir, "ContainerTest-");
+        componentGraph = new ComponentGraph(0);
+        osgi = new TestOsgi(BundleTestUtil.testBundles());
     }
 
     @AfterEach
@@ -41,47 +41,43 @@ public class ContainerTestBase {
         dirConfigSource.cleanup();
     }
 
-    @BeforeEach
-    public void createGraph() {
-        componentGraph = new ComponentGraph(0);
+
+    protected Container newContainer(DirConfigSource dirConfigSource,
+                                            ComponentDeconstructor deconstructor) {
+        return new Container(new CloudSubscriberFactory(dirConfigSource.configSource),
+                             dirConfigSource.configId(),
+                             deconstructor,
+                             osgi);
     }
 
-    public void complete() {
-        try {
-            Container container = new Container(new CloudSubscriberFactory(dirConfigSource.configSource()), dirConfigSource.configId(),
-                    new ContainerTest.TestDeconstructor(), new Osgi() {
-                        @SuppressWarnings("unchecked")
-                        @Override
-                        public Class<Object> resolveClass(BundleInstantiationSpecification spec) {
-                            try {
-                                return (Class<Object>) Class.forName(spec.classId.getName());
-                            } catch (ClassNotFoundException e) {
-                                throw new RuntimeException(e);
-                            }
-                        }
-
-                        @Override
-                        public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles) {
-                            return emptySet();
-                        }
-
-                        @Override
-                        public Bundle getBundle(ComponentSpecification spec) {
-                            throw new UnsupportedOperationException("getBundle not supported.");
-                        }
-                    });
-            Container.ComponentGraphResult result = container.waitForNextGraphGeneration(this.componentGraph, Guice.createInjector(), true);
-            result.oldComponentsCleanupTask().run();
-            this.componentGraph = result.newGraph();
-        } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
+    protected Container newContainer(DirConfigSource dirConfigSource) {
+        return newContainer(dirConfigSource, new TestDeconstructor(osgi));
     }
 
+    ComponentGraph getNewComponentGraph(Container container, ComponentGraph oldGraph) {
+        Container.ComponentGraphResult result = container.waitForNextGraphGeneration(oldGraph, Guice.createInjector(), true);
+        result.oldComponentsCleanupTask().run();
+        return result.newGraph();
+    }
+
+    ComponentGraph getNewComponentGraph(Container container) {
+        return container.waitForNextGraphGeneration(new ComponentGraph(), Guice.createInjector(), true).newGraph();
+    }
+
+
     public <T> T getInstance(Class<T> componentClass) {
         return componentGraph.getInstance(componentClass);
     }
 
+    protected void writeBootstrapConfigsWithBundles(List<String> applicationBundles, List<ComponentEntry> components) {
+        writeBootstrapConfigs(components.toArray(new ComponentEntry[0]));
+        StringBuilder bundles = new StringBuilder();
+        for (int i = 0; i < applicationBundles.size(); i++) {
+            bundles.append("bundles[" + i + "] \"" + applicationBundles.get(i) + "\"\n");
+        }
+        dirConfigSource.writeConfig("application-bundles", String.format("bundles[%s]\n%s", applicationBundles.size(), bundles));
+    }
+
     protected void writeBootstrapConfigs(ComponentEntry... componentEntries) {
         dirConfigSource.writeConfig("platform-bundles", "");
         dirConfigSource.writeConfig("application-bundles", "");
@@ -115,11 +111,29 @@ public class ContainerTestBase {
         }
 
         String asConfig(int position) {
-            return "<config>\n" + //
-                    "components[" + position + "].id \"" + componentId + "\"\n" + //
-                    "components[" + position + "].classId \"" + classId.getName() + "\"\n" + //
-                    "components[" + position + "].configId \"" + dirConfigSource.configId() + "\"\n" + //
-                    "</config>";
+            return  "components[" + position + "].id \"" + componentId + "\"\n" +
+                    "components[" + position + "].classId \"" + classId.getName() + "\"\n" +
+                    "components[" + position + "].configId \"" + dirConfigSource.configId() + "\"\n" ;
+        }
+    }
+
+
+    public static class TestDeconstructor implements ComponentDeconstructor {
+
+        final TestOsgi osgi;
+
+        public TestDeconstructor(TestOsgi osgi) {
+            this.osgi = osgi;
+        }
+
+        @Override
+        public void deconstruct(long generation, List<Object> components, Collection<Bundle> bundles) {
+            components.forEach(component -> {
+                if (component instanceof ContainerTest.DestructableComponent vespaComponent) {
+                    vespaComponent.deconstruct();
+                }
+            });
+            bundles.forEach(osgi::removeBundle);
         }
     }
 
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java
index 438ad8d8ebe..f8c158ab178 100644
--- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java
@@ -94,7 +94,7 @@ public interface OsgiFramework {
      * Allows this framework to install duplicates of the given collection of bundles. Duplicate detection
      * is handled by the {@link com.yahoo.jdisc.core.BundleCollisionHook}.
      *
-     * @param bundles The bundles to allow duplicates of
+     * @param bundles The bundles to allow duplicates of. An empty collection will prohibit any duplicates.
      */
     void allowDuplicateBundles(Collection<Bundle> bundles);
 
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java
index 2f90b4e067f..3212bb4e6de 100644
--- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java
@@ -34,7 +34,7 @@ public class BundleCollisionHook implements CollisionHook, EventHook, FindHook {
     private static final Logger log = Logger.getLogger(BundleCollisionHook.class.getName());
 
     private ServiceRegistration<?> registration;
-    private Map<Bundle, BsnVersion> allowedDuplicates = new HashMap<>(5);
+    private final Map<Bundle, BsnVersion> allowedDuplicates = new HashMap<>(5);
 
     public void start(BundleContext context) {
         if (registration != null) {
@@ -50,11 +50,10 @@ public class BundleCollisionHook implements CollisionHook, EventHook, FindHook {
     }
 
     /**
-     * Adds a collection of bundles to the allowed duplicates.
-     * Also clears any previous allowed duplicates of the new allowed duplicates.
+     * Sets a collection of bundles to allow duplicates for.
      */
     synchronized void allowDuplicateBundles(Collection<Bundle> bundles) {
-        allowedDuplicates.values().removeAll(bundles.stream().map(BsnVersion::new).collect(Collectors.toSet()));
+        allowedDuplicates.clear();
         for (var bundle : bundles) {
             allowedDuplicates.put(bundle, new BsnVersion(bundle));
         }
diff --git a/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/BundleCollisionHookIntegrationTest.java b/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/BundleCollisionHookIntegrationTest.java
index 07797838a4b..713f0397984 100644
--- a/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/BundleCollisionHookIntegrationTest.java
+++ b/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/BundleCollisionHookIntegrationTest.java
@@ -81,14 +81,18 @@ public class BundleCollisionHookIntegrationTest {
     }
 
     @Test
-    public void duplicates_whitelist_can_be_updated_with_additional_bundles() throws Exception {
+    public void duplicates_whitelist_is_reset_upon_each_call() throws Exception {
         Bundle bundleL = startBundle(felix, "cert-l1.jar");
         Bundle bundleMl = startBundle(felix, "cert-ml.jar");
         felix.allowDuplicateBundles(Collections.singleton(bundleL));
         felix.allowDuplicateBundles(Collections.singleton(bundleMl));
 
-        startBundle(felix, "cert-l1-dup.jar");
-        startBundle(felix, "cert-ml-dup.jar");
+        try {
+            startBundle(felix, "cert-l1-dup.jar");
+            fail("Expected exception due to duplicate bundles");
+        } catch (BundleException e) {
+            assertTrue(e.getMessage().contains("Bundle symbolic name and version are not unique"));
+        }
     }
 
     @Test
@@ -97,8 +101,7 @@ public class BundleCollisionHookIntegrationTest {
         Bundle bundleB = startBundle(felix, "cert-b.jar");
 
         // Makes A and B visible to each other
-        felix.allowDuplicateBundles(Set.of(bundleA));
-        felix.allowDuplicateBundles(Set.of(bundleB));
+        felix.allowDuplicateBundles(Set.of(bundleA, bundleB));
 
         List<Bundle> visibleBundles = felix.getBundles(bundleA);
 
@@ -117,8 +120,7 @@ public class BundleCollisionHookIntegrationTest {
         Bundle bundleB = startBundle(felix, "cert-b.jar");
 
         // Makes A and B visible to each other
-        felix.allowDuplicateBundles(Set.of(bundleA));
-        felix.allowDuplicateBundles(Set.of(bundleB));
+        felix.allowDuplicateBundles(Set.of(bundleA, bundleB));
 
         List<Bundle> visibleBundles = felix.getBundles(bundleA);
         assertEquals(3, visibleBundles.size());