WFCORE-4535, WFCORE-4542, WFCORE-4541, WFCORE-4613 Overhaul discovery subsystem

[pferraro]

https://issues.jboss.org/browse/WFCORE-4535
https://issues.jboss.org/browse/WFCORE-4541
https://issues.jboss.org/browse/WFCORE-4542
https://issues.jboss.org/browse/WFCORE-4613

In the process of investigating solutions for https://issues.jboss.org/browse/WFLY-12222 involving the discovery subsystem, I discovered that this subsystem never really worked correctly.
The most significant issues include:

• Inconsistent capability definitions (service value vs runtime API) between resource registration and those registered at runtime (WFCORE-4535)
  • Consequently, capability constraints were not correctly enforced for providers referenced by aggregate-provider
• Add operation handlers did not populate the discovery provider implementations they created
• "services" write attribute handler for "static-provider" throws StackOverflowErrors (WFCORE--4541).

I've re-written the add operation handlers and capability logic to address the issues above.
Subsystem capabilities are documented here: wildfly/wildfly-capabilities#37

[bstansberry]

Why the last commit? Spelling out the dependencies instead of just inheriting everything transitively is kind of nice IMHO.

This seems good; I want to look again at the main commit re the change in write-attribute behavior; now just sets reload-required. Probably fine if the custom runtime API thing was not really usable.

[pferraro]

Why the last commit? Spelling out the dependencies instead of just inheriting everything transitively is kind of nice IMHO.

Unless this module was excluding all transitive dependencies from wildfly-controller (it only excludes transitive dependencies from org.wildfly.core), I don't see much point to spelling out the dependencies, if they are already part of the dependency hierarchy. Also, I don't see any other subsystem modules that use wildcard exclusions, so I was just trying to be consistent.

If you prefer I leave them, I'll comply. :)

I want to look again at the main commit re the change in write-attribute behavior; now just sets reload-required.

We can also support the allow-resource-service-restart operation header.

[pferraro]

@bstansberry Any other comments?

[bstansberry]

@pferraro This is ok with me. The old [xxx]ProviderAddHandler.modifyRegistration stuff probably worked, but the capability runtime API behind it was never properly published, as described in WFCORE-4535. Nor were the capabilities ever documented (https://issues.jboss.org/browse/WFLY-12225). Also it seems that WFCORE-4541 means that write-attribute for a static-provider could not have worked.

StaticProviderAddHanlder.modifyRegistration could be made to work for write-attribute without allow-resource-service-restart, since the MutableDiscoveryProvider could be looked up and modified from MSC. But I agree that in the aggregrate-provider case, MSC services are more practical and that means a service restart is required. I don't like allow-resource-service-restart so wouldn't like it for the aggregrate-provider case. And for the static-provider case 'allow-resource-service-restart' would not be accurate as no service would have to be restarted. JMHO but I think it's cleaner to keep both types consistent and require reload, as you do here.

[rhusar]

FWIW I have been working with this branch for clustering subsystems integration work and haven't found any major problems besides a minor tangential issue https://issues.jboss.org/browse/WFCORE-4613 about missing validation (not sure if originated in this branch).

[pferraro]

@rhusar The missing validation was a pre-existing problem. I've added a commit that addresses WFCORE-4613 to this PR.

[wildfly-ci]

Core - Full Integration Build 8944 outcome was UNKNOWN using a merge of cb22323
Summary: Canceled (Tests passed: 5108, ignored: 127; exit code 143 (Step: Build & test full (Maven)) (new)) Build time: 02:01:30

[rhusar]

retest this please

[rhusar]

Looks great now.