ServiceDeployer.deploy not seeing deployment errors

[starksm64]

The http://jira.jboss.com/jira/browse/JBAS-4684 issue is due to the ServiceDeployer.deploy method not seeing the deployment error. This is due to the AbstractController.incrementState uninstalling context with errors:

...
 finally
 {
 lockWrite();
 if (error != null)
 {
 log.error("Error installing to " + toState.getStateString() + ": " + context.toShortString(), error);
 uninstallContext(context, ControllerState.NOT_INSTALLED, trace);
 errorContexts.put(context.getName(), context);
 context.setError(error);
 return false;
 }
 }

AbstractControllerContext.uninstall(ControllerState fromState, ControllerState toState) clears the error state, and the ServiceContext problem is nulled out when the CreateDestroyLifecycleAction.uninstallAction calls getServiceContext:

 public ServiceContext getServiceContext()
 {
 try
 {
 serviceContext.proxy = getServiceProxy();
 }
 catch (Exception ignored)
 {
 }
 serviceContext.setProblem(getError());
 if (getState() == ControllerState.ERROR)
 serviceContext.state = ServiceContext.FAILED;
 return serviceContext;
 }

The context after the ServiceDeployer.deploy calls start(context), the context is in a DESTROYED state, but has no problem set:

ObjectName: jboss.web.deployment:war=/custom-context
 State: DESTROYED

Because of this, the MainDeployer.checkComplete call has no information about the failed deployment. If I just fail to null out the ServiceContext.problem then the deployment failure propagates as expected.

However, it seem like we should not be able to transition out of the ERROR state to DESTROYED?

[adrian.brock]

The http://jira.jboss.com/jira/browse/JBAS-4684 issue is due to the ServiceDeployer.deploy method not seeing the deployment error. This is due to the AbstractController.incrementState uninstalling context with errors:

...
 finally
 {
 lockWrite();
 if (error != null)
 {
 log.error("Error installing to " + toState.getStateString() + ": " + context.toShortString(), error);
 uninstallContext(context, ControllerState.NOT_INSTALLED, trace);
 errorContexts.put(context.getName(), context);
 context.setError(error);
 return false;
 }
 }

Looks to me like it is setting the error after it has done the uninstallContext()
so I don't see why this wouldn't show up in the checkComplete()?

NOTE: If this is the checkComplete(Deployment) rather than the full checkComplete()
that does not look at the contexts created by the deployment, it is a TODO item.

The context after the ServiceDeployer.deploy calls start(context), the context is in a DESTROYED state, but has no problem set:

ObjectName: jboss.web.deployment:war=/custom-context
 State: DESTROYED

Because of this, the MainDeployer.checkComplete call has no information about the failed deployment.
If I just fail to null out the ServiceContext.problem then the deployment failure propagates as expected.

That looks like a cut and paste error to me? i.e. uninstall() was copied from install()
and the nulling of the error was not removed.
We just haven't seen it before because of the setting of the error after uninstallContext()
above.

However, it seem like we should not be able to transition out of the ERROR state to DESTROYED?

That is only on the old ServiceContext (which isn't used for the checkComplete())
It is not being maintained properly because the setError() hasn't been invoked yet.
The real ServiceControllerContext should be in the ERROR state?

[adrian.brock]

I've removed the spurious nulling of error in AbstractControllerContext.uninstall()

I see you've already worked around the problem in system-jmx.
I added an extra check to make sure the state of the ServiceContext is always FAILED
when we check errors in those tests.

I also reverted your change that made ServiceContext.problem private.
The whole point of maintaining the ServiceContext is for backwards compatibilty
where users have referenced this class (it is exposed via the ServiceController
and ServiceMBeanSupport).
If this field disappears it could break user code.