[WFCORE-7011] Use 'default' stability level when the remote host controller do not advertise its current stability level #6204
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
|
Just rebased on top of the current main and fixed ServerEnvironmentTestCase#testAliasNotWorkingInDefaultStability. @pferraro Could you approve from your side? I would like to release this this week to unlock pending work with the mixed domains in upstream. This one will be merged as the last PR for the release since it breaks the TS until the WildFly counterpart gets merged After that, we will continue adding a WildFly for the mixed domains and follow with #6214 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Are you sure this branch is up to date? The git diff looks odd. |
|
Hi @pferraro , it is now. We do not need to get the ProductConf stability level at the Host registration process, so most of the "weird" changes shown in the diff are to remove the need to pass that variable through all the method calls to get it at the registration phase. Then, some adjustments on some tests because I moved the default stability level for the tests to Community, which apparently makes more sense. We can continue using DEFAULT and let each test choose whatever it needs. Maybe that makes more sense. Still in draft since I have not gotten the full CI feedback yet, but great if you can take a look at it is |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@pferraro I think this is ready to go with your review, galleon full failure is unrelated |
| this.defaultStability = Stability.DEFAULT; | ||
| this.stabilities = EnumSet.of(this.defaultStability); | ||
| this.defaultStability = Stability.COMMUNITY; | ||
| this.stabilities = Collections.unmodifiableSet(EnumSet.range(Stability.DEFAULT, Stability.COMMUNITY)); |
There was a problem hiding this comment.
I would keep this logic internal to the Stability object.
e.g.
this.stabilities = EnumSet.allOf(Stability.class).stream().filter(this.defaultStability::enables).collect(Collectors.toUnmodifiableSet());There was a problem hiding this comment.
@pferraro thanks for reviewing, done
Since that was the only pending fix, I am going to move on with it so I can prepare a release today
There was a problem hiding this comment.
Well, wait .... now I am wondering, I would expect to see the mixed domain integration tests failing with this PR in, since we will be assuming EAP 7.4 legacy host will be running in "default" stability level, hence the mixed domains should have failed because WildFly will be running by default in "community" stability level.
Looking ....
There was a problem hiding this comment.
Ok, got it, I've realized today that full integration Jobs are ignoring the mixed domain tests because of a misconfiguration of those two Jobs (they are using JDK 17 but legacy JDK to launch the legacy servers is not configured). I'll fix them once we have integrated both this PR and the WildFly counterpart, so we can continue without breaking WildFly Core CI until the WildFly full gets merged.
…roller do not advertise its current stability level Jira issue: https://issues.redhat.com/browse/WFCORE-7011
This comment was marked as off-topic.
This comment was marked as off-topic.
yersan
added
ready-for-merge
|
Thanks @pferraro |
Built on top of #6201
Jira issue: https://issues.redhat.com/browse/WFCORE-7011
It is expected that mixed domain tests fail with this change. I opened it as a draft since this change still requires some discussions to decide whether we want to allow DC -- HC mixed domains under community -- default stability levels