Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WFCORE-6728] Utility to reload servers started by testsuite to a desired stability level #5895

Merged
merged 10 commits into from
Mar 23, 2024
Merged

[WFCORE-6728] Utility to reload servers started by testsuite to a desired stability level #5895

merged 10 commits into from
Mar 23, 2024

Conversation

kabir
Copy link
Contributor

@kabir kabir commented Mar 6, 2024 •
edited
Loading

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Mar 6, 2024
bstansberry
bstansberry reviewed Mar 11, 2024
View reviewed changes
testsuite/shared/src/main/java/org/wildfly/test/stability/StabilityServerSetupTasks.java Outdated
}

// The stability parameter for the reload opration is only registered below the default level
Assume.assumeFalse("Can't reload to a different stability when server stability level is default", currentStability == Stability.DEFAULT && stability != Stability.DEFAULT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should do a read-operation-names and look for the op and Assume.assumeTrue(opnames.contains(theopname)).

IOW don't hard code the current rules about the availability of the op into this test fixture.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need https://issues.redhat.com/browse/WFCORE-6731 and then this can do an Assume.assumeTrue(supported.contains(stability))

Copy link
Contributor Author

@kabir kabir Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bstansberry I'm not totally sure about WFCORE_6731 for this? I am able to start WildFly Core with --stability=default so I presume that means default is a legal stability level. Still, I can add a placeholder for this check, since there may well be something which isn't turned on in my version of WildFly yet enforcing this.

EDIT: I've looked in ProductConfig and it makes sense now, I can see it :-)

The issue I am trying to solve here, is that if we were to reload to default to run the test, we would not be able to reload back to community since the new reload operation is only registered at that level.

I think I will check the stability of the reload-enhanced operation definition, and check that it would still be available if the reload to the intended stability takes place.

@bstansberry
Copy link
Contributor

@kabir I changed this to a Feature Request.

Re "I'm opening this as a draft for now. If it looks good, I'll add something similar for domain mode." -- This comment might be out of date, but if not I suggest not, not for this iteration. The key use case is testing, and for that use case domain mode is a minor consideration. Better IMHO to just do the standalone bit and come back later for more. As long as you can write an analysis with a clear use case and a solution that logically can be carried forward to other use cases without incompatible change, that's a good target. The more you go beyond that the more time will be needed from others and the longer it will take to get in.

@kabir kabir force-pushed the stability-level-test-reload branch from 51d6e3a to 8cbabde Compare March 12, 2024 11:54
@kabir kabir marked this pull request as ready for review March 12, 2024 11:54
@kabir kabir force-pushed the stability-level-test-reload branch 5 times, most recently from c4e0661 to b1438fc Compare March 19, 2024 15:20
@wildfly-ci
Copy link

Core -> Full Integration Build 13615 outcome was UNKNOWN using a merge of b1438fc
Summary: Canceled (Error while applying patch; cannot find commit 365c419 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/5895/merge branch was updated and the commit selected for the ... Build time: 00:00:38

@wildfly-ci
Copy link

Core -> Full Integration Build 13368 outcome was UNKNOWN using a merge of b1438fc
Summary: Canceled (Error while applying patch; cannot find commit 365c419 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/5895/merge branch was updated and the commit selected for the ... Build time: 00:00:18

@wildfly-ci
Copy link

Core -> WildFly Preview Integration Build 13425 outcome was UNKNOWN using a merge of b1438fc
Summary: Canceled (Error while applying patch; cannot find commit 365c419 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/5895/merge branch was updated and the commit selected for the ... Build time: 00:00:20

@wildfly-ci
Copy link

Core -> Full Integration Build 13370 outcome was FAILURE using a merge of b1438fc
Summary: Exit code 1 (Step: Build core (Maven)) (new) Build time: 00:02:00

@wildfly-ci
Copy link

Core -> WildFly Preview Integration Build 13427 outcome was FAILURE using a merge of b1438fc
Summary: Exit code 1 (Step: Build core (Maven)) (new) Build time: 00:01:40

@wildfly-ci
Copy link

Core -> Full Integration Build 13616 outcome was FAILURE using a merge of b1438fc
Summary: Exit code 1 (Step: Build core (Maven)) (new) Build time: 00:02:08

@kabir kabir force-pushed the stability-level-test-reload branch from b1438fc to ed3dc9d Compare March 19, 2024 15:58
@wildfly-ci
Copy link

Core -> Full Integration Build 13617 outcome was FAILURE using a merge of ed3dc9d
Summary: Exit code 1 (Step: Build core (Maven)) Build time: 00:01:52

@wildfly-ci
Copy link

Core -> WildFly Preview Integration Build 13428 outcome was FAILURE using a merge of ed3dc9d
Summary: Exit code 1 (Step: Build core (Maven)) Build time: 00:02:15

@wildfly-ci
Copy link

Core -> Full Integration Build 13371 outcome was FAILURE using a merge of ed3dc9d
Summary: Exit code 1 (Step: Build core (Maven)) Build time: 00:02:57

@wildfly-ci
Copy link

Core -> Full Integration Build 13618 outcome was FAILURE using a merge of ed3dc9d
Summary: Exit code 1 (Step: Build core (Maven)) Build time: 00:01:29

pferraro
pferraro requested changes Mar 19, 2024
View reviewed changes
server/src/main/java/org/jboss/as/server/Bootstrap.java Outdated Show resolved Hide resolved
server/src/main/java/org/jboss/as/server/ServerEnvironment.java Outdated

ServerEnvironment recalculateForReload(RunningModeControl runningModeControl) {
if (runningModeControl.isReloaded()) {
Stability stability = runningModeControl.getReloadedStability() != null ? runningModeControl.getReloadedStability() : productConfig.getDefaultStability();
Copy link
Contributor

@pferraro pferraro Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this default to the current value of ServerEnvironment.getStability()? That way a regular reload would reflect the stability level of a previous enhanced reload.

controller/src/main/java/org/jboss/as/controller/extension/ExtensionRegistry.java Show resolved Hide resolved
kabir added 9 commits March 21, 2024 09:48
@kabir
[WFCORE-6728] New reload-enhanced operation allowing us to reload to …
149ecb2 
…a new stability level

This is not available at all stability levels, and has its own RBAC settings
@kabir
[WFCORE-6728] Change stability in ExtensionRegistry when reloading
6c9e814
@kabir
[WFCORE-6728] Clean up the ServerReload overloaded methods a bit
923ad86
@kabir
[WFCORE-6728] Make it possible to run Junit Assume from a ServerSetup…
ada7a1d 
…Task.setup() method
@kabir
[WFCORE-6728] Setup tasks to reload to a server stability level
189129f
@kabir
[WFCORE-6728] Make the stability setup tasks take/restore snapshots o…
594b8c8 
…f the server config

Otherwise, if we were working on a lower stability level, and made changes to the
model, those could be stored with s schema from a lower stability level, meaning
the reload back to the default community level will fail since it will contain
xml elements from a namespace we can't handle
@kabir
[WFCORE-6728] Tweak default if there is no reloaded stability
05006ab
@kabir
[WFCORE-6728] Refactor ExtensionRegistry to be a supplier
720f835
@kabir
[WFCORE-6728] Code review suggestions
2363b2e
@kabir kabir force-pushed the stability-level-test-reload branch from 7817dd6 to 2363b2e Compare March 21, 2024 09:49
@pferraro pferraro self-requested a review March 21, 2024 15:20
@jamezp
Copy link
Member

jamezp commented Mar 21, 2024

Just a general comment about the ServerReload stuff. We've got the https://github.com/wildfly/wildfly-plugin-tools project which has some reload helper methods. Would it make sense to move the ServerReload stuff there so it can be shared across the various projects?

@yersan yersan mentioned this pull request Mar 22, 2024
Closed
yersan
yersan approved these changes Mar 22, 2024
View reviewed changes
server/src/main/java/org/jboss/as/server/ServerEnvironment.java Show resolved Hide resolved
...hared/src/main/java/org/wildfly/test/stability/StabilityServerSetupSnapshotRestoreTasks.java
Comment on lines +113 to +116
private Set<Stability> getSupportedStabilityLevels() {
// TODO WFCORE-6731 - see https://github.com/wildfly/wildfly-core/pull/5895#discussion_r1520489808
// This information will be available in a management operation, For now just return a hardcoded set
return EnumSet.allOf(Stability.class);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this is not going to block this PR. We could address it in a follow up.

WFCORE-6731 is in place and I think ready to be merged #5908, @pferraro FYI

@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@wildfly wildfly deleted a comment from wildfly-ci Mar 23, 2024
@bstansberry bstansberry merged commit 9a10dbd into wildfly:main Mar 23, 2024
12 checks passed
@bstansberry
Copy link
Contributor

Thanks @kabir, @yersan, @jamezp and @pferraro

@kabir kabir mentioned this pull request Apr 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamezp jamezp jamezp left review comments

@bstansberry bstansberry bstansberry approved these changes

@yersan yersan yersan approved these changes

@pferraro pferraro Awaiting requested review from pferraro

Assignees
No one assigned
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kabir @bstansberry @wildfly-ci @jamezp @pferraro @yersan