Repository navigation
Resume interrupted feature deployments on startup - #3010
holgerfriedrich wants to merge 1 commit into
Conversation
If the framework stops after a deployment has installed its bundles and saved the features state, but before or while the bundles are started, those bundles stay installed/resolved after the next start: the framework does not start them (starts during shutdown are not persisted) and the features service considers the deployment done. Persist a deploymentPending flag while the deployment is in progress, keep it if the framework is shutting down at the end of the start phase, and resume pending deployments in BootFeaturesInstaller.
| public void start() { | ||
| if (bootManaged.isBootDone()) { | ||
| publishBootFinished(); | ||
| if (bootManaged.isDeploymentPending()) { |
There was a problem hiding this comment.
deploymentPending is also true on disk while a deployment is still running, so this check can't tell "interrupted" from "in progress".
When a deployment refreshes the features bundle itself, the Activator is restarted. For example, feature:install eventadmin does this because features-core has an optional import of org.osgi.service.event, which is why ensureAllClassesLoaded() exists. The new FeaturesServiceImpl loads state.json with deploymentPending=true, and this calls resumeDeployment() while the old deployment thread is still resolving and starting bundles. executor.shutdown() doesn't interrupt that thread. A feature <config> for org.apache.karaf.features has the same effect, since it re-runs doStop()/doStart(). Either way, two deployers end up stopping, refreshing and starting the same bundles at the same time.
Could we resume only after a real framework restart? One way: store the framework UUID of the launch that started the deployment (bundleContext.getProperty(Constants.FRAMEWORK_UUID)) instead of a boolean, and resume only when it differs from the current one. The UUID is regenerated each time the framework is initialized but stays the same across a refresh of the features bundle.
| Bundle systemBundle = dstate.bundles.get(0L); | ||
| if (systemBundle == null || (systemBundle.getState() & (Bundle.STARTING | Bundle.ACTIVE)) != 0) { | ||
| newState.deploymentPending.set(false); | ||
| callback.saveState(newState); |
There was a problem hiding this comment.
If this deployment refreshed the features bundle (the toRefresh.containsKey(serviceBundle) branch above), this save runs on the old instance, whose bundle context is no longer valid. StateStorage calls bundleContext.getDataFile("state.json"), which throws IllegalStateException, and saveState() only catches IOException.
So deploy() fails before the MultiException, the feature events and DEPLOYMENT_FINISHED, and feature:install reports an error even though everything is installed. The flag also stays true on disk. Before this change, nothing touched the bundle context after the refresh.
I'd skip this save when serviceBundle was refreshed and let the new instance own the state from there. With a per-launch check like the one suggested in BootFeaturesInstaller, the worst case is one unneeded re-provision on the next restart.
| public void resumeDeployment() throws Exception { | ||
| State state = copyState(); | ||
| doProvisionInThread(state.requirements, emptyMap(), state, getFeaturesById(), | ||
| EnumSet.noneOf(Option.class)); |
There was a problem hiding this comment.
The options of the interrupted deployment aren't saved, so this replays it with default options. If a feature:install --no-auto-refresh (or a deployment with NoAutoManageBundles, NoAutoRefreshUnmanagedBundles, etc.) is interrupted, it is resumed with auto refresh and bundle management turned on, so it changes bundles the user asked us to leave alone.
The self-update path already handles this: persistResolveRequest() writes the requirements and options to the resolve file, and checkResolve() replays them with those options. Could we save the options the same way next to the pending marker?
Also, feature:uninstall -c interrupted before deleteConfigs won't delete the configurations on resume, even with the options. By then the features are already gone from installedFeatures, so the resume sees nothing to uninstall.
| newState.managedBundles.putAll(managedBundles); | ||
| // the bundles are installed, but not yet refreshed/resolved/started: if the framework stops before this | ||
| // deployment is complete, they would stay unstarted, so remember to resume it on the next start | ||
| newState.deploymentPending.set(true); |
There was a problem hiding this comment.
By the time the flag is saved, bundles have already been stopped, uninstalled, updated and installed. For a large feature or KAR, that is the longest part of the deployment. If Karaf stops during that phase, state.json has neither the new requirements nor the flag, so nothing is resumed and the new bundles stay installed but unmanaged and not started. That is the same bug this PR fixes, just in a different window. Bundles that were stopped persistently because their start level changes (stopBundle(bundle, 0) at line 783) also stay stopped.
Can we save the new requirements and the pending marker before the first bundle operation (right after DEPLOYMENT_STARTED)? The resume re-resolves from the requirements, and the Deployer already turns installed bundles that a feature needs into managed bundles (around line 471). So the resume shouldn't need the intermediate state, but a test for that case would confirm it.
| throw new MultiException("Error restarting bundles", exceptions); | ||
| } | ||
| } | ||
| // All bundles have been started (failures included, resuming would not help with those), unless the framework |
There was a problem hiding this comment.
The comment says failed deployments shouldn't be resumed, but that only holds for BundleExceptions collected in the start loop. Anything thrown between the first save and here leaves deploymentPending=true on disk: installConfigs/installLibraries, refreshPackages, resolveBundles, or a RuntimeException from startBundle. The same failing deployment then runs again on every restart, and every time the features Activator is reconfigured. With the default featuresBootAsynchronous=false, that blocks the Activator thread each time.
I'd clear the flag in a finally around everything after the first save, keeping the "framework is stopping" check, so only a real interruption leaves it set.
This is a fix for a strange behavior that I encountered when Karaf was restarted during initial KAR deployment.
It ended up with some bundles not started after the next Karaf start.
If the framework stops after a deployment has installed its bundles and saved the features state, but before or while the bundles are started, those bundles stay installed/resolved after the next start: the framework does not start them (starts during shutdown are not persisted) and the features service considers the deployment done.
Persist a deploymentPending flag while the deployment is in progress, keep it if the framework is shutting down at the end of the start phase, and resume pending deployments in BootFeaturesInstaller.
This PR was created with help from AI.