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.

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

[WFCORE-6815][Community] Added reload-enhanced operation to reload a domain to a certain stability #5992

Merged
merged 7 commits into from
Jun 20, 2024

Conversation

yersan
Copy link
Collaborator

@yersan yersan commented May 8, 2024

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label May 8, 2024
@yersan yersan requested a review from kabir May 9, 2024 09:44
@yersan
Copy link
Collaborator Author

yersan commented May 9, 2024

@kabir This PR is based on your previous work for standalone, although it has some particularities to adapt it on Domain Mode, could you take a look and provide your feedback? thanks

Comment on lines +926 to +940
void setStability(Stability stability) {
WildFlySecurityManager.setPropertyPrivileged(ProcessEnvironment.STABILITY, stability.toString());
this.hostSystemProperties.put(ProcessEnvironment.STABILITY, stability.toString());
this.stability = stability;
}

public void checkStabilityIsValidForInstallation(Stability stability) {
checkStabilityIsValidForInstallation(productConfig, stability);
}

private void checkStabilityIsValidForInstallation(ProductConfig productConfig, Stability stability) {
if (!productConfig.getStabilitySet().contains(stability)) {
throw HostControllerLogger.ROOT_LOGGER.unsupportedStability(this.stability, productConfig.getProductName());
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Notice we also have https://issues.redhat.com/browse/WFCORE-6760 to move common functionality to ProcessEnvironment, that can be done later

Copy link
Contributor

@kabir kabir left a comment

Choose a reason for hiding this comment

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

I think it looks good

}

public static OperationDefinition getEnhancedDefinition(final LocalHostControllerInfo hostControllerInfo) {
return new DeferredParametersOperationDefinitionBuilder(hostControllerInfo, ENHANCED_OPERATION_NAME, HostModelUtil.getResourceDescriptionResolver(), ENHANCED_PRIMARY_HC_ATTRIBUTES, ENHANCED_SECONDARY_HC_ATTRIBUTES, Stability.COMMUNITY)
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't really important, but I think it would be clearer if both the calls to this constructor had the same format

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for reviewing @kabir

What do you mean about having the same format?

I understand that what you suggested is to remove the overloaded DeferredParametersOperationDefinitionBuilder constructor, so both getEnhancedDefinition and getDefinition use the same number of arguments to improve the code readability and make it more clear, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So far, just updated it to assert that closing the snapshot indeed reloaded the host controller to the original stability level

@yersan yersan requested a review from kabir May 9, 2024 16:34
@yersan yersan changed the title [WFCORE-6815] Added reload-enhanced operation to reload a domain to a certain stability [WFCORE-6815][Community] Added reload-enhanced operation to reload a domain to a certain stability May 17, 2024
@wildfly-ci

This comment was marked as outdated.

@wildfly-ci
Copy link

Core -> Full Integration Build 13857 outcome was FAILURE using a merge of 59589cb
Summary: Execution timeout (new); tests passed: 2925, ignored: 45 Build time: 10:00:58

@wildfly wildfly deleted a comment from wildfly-ci Jun 10, 2024
@yersan
Copy link
Collaborator Author

yersan commented Jun 17, 2024

@kabir Friendly remainder, this one is waiting for your approval to get it merged, please take a look, thanks!

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Jun 19, 2024
@yersan yersan merged commit a358ba9 into wildfly:main Jun 20, 2024
12 checks passed
@yersan yersan deleted the WFCORE-6815 branch June 20, 2024 07:44
@yersan
Copy link
Collaborator Author

yersan commented Jun 20, 2024

Thanks @kabir @jamezp for the reviews here and in the analysis doc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants