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-5343]: Proposal for YAML support #384

Merged
merged 1 commit into from
Jun 28, 2024
Merged

Conversation

ehsavoie
Copy link
Contributor

Supersedes #381

@ehsavoie ehsavoie mentioned this pull request Mar 26, 2021
Closed
@ehsavoie ehsavoie force-pushed the WFCORE-5343 branch 3 times, most recently from a6c28a8 to 006e55f Compare March 30, 2021 13:39

== Overview

The aim of this proposal is to be able to inject external configuration in a YAML format to WildFly in order to customize an existing configuration.
Copy link

Choose a reason for hiding this comment

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

We see overlap between this feature and https://issues.redhat.com/browse/WFCORE-5324.

Can you elaborate in proposal what does it add compared to https://issues.redhat.com/browse/WFCORE-5324. Is it just different format? I assume underneath both cli and yaml ways end in DMR so behaviour, starting times and limitations would be same.

Can you elaborate on that as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is different in that the operations from the YAML files are executed on boot in addition to the one from the XML file. So this means that we should have better performance, we don't require a reload of the server and we don't change the resulting XML configuration since we are in "read-only" mode on that part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also this isn't a script: no if/else etc.
Also you can reuse the same command line in a loop without having to have the logic in your script to manage existing resources.
Last but not least, this is for all versions of WildFly: bootable and bare metal.

Copy link

Choose a reason for hiding this comment

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

Can I achieve better performance and no reload requirement and idempotency with CLI as well? Or is it consequence of limited scope of yaml files? Still I assume CLI way must be superset of yaml way, because in the end yaml must use same capabilities as CLI. Am I missing something?

So I have cli way which is applicable everywhere; bootable jar, bare metal, openshift, domain. And I can express anything in CLI.

I will have yaml file with much more limited usage (just bootable jar, bare metal, just subset of CLI capabilities).

Why should I use yaml file? When I will be writing yaml files I will need to try cli command before transform it into yaml snippet anyway. Isn't it better to directly compose cli script?

I agree yaml file is easy to read. But something similar can be achieved with formatting of CLI commands. ( I know it is not yaml :) )

/authentication=classic/login-module=Remoting:add( \
    code=Remoting, \
    flag=optional, \
    module-options=[ \
        password-stacking=useFirstPass \
    ] \
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would work on OpenShift ;-) Just domain is not supported.
You won't test using CLI script, in the same way you don't test using CLI to write or configure a feature in Galleon.
CLI script requires a reload: so boot-time is longer. Like I wrote the cost is almost just the cost of parsing the YAML.
In your case you are missing the test to check if the resource already exists. Also you would have to create all the tree, while here we would create them for you.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mchoma, yaml syntax, in the context of bootable JAR, is to help transitioning from Thorntail to WildFly Bootable JAR. The object model it exposed can be seen as simpler than CLI syntax. CLI implies a start in admin and a reload. We apply CLI commands (that are more than DMR) on top of a running server. Yaml will be more efficient. Having both is powerful. The order of application is: 1) yaml files 2) CLI script. With CLI you can call some management operations not exposed by YAML. I suspect that yaml will be much more used than CLI with bootable JAR.
I don't know of its usage outside of bootable JAR. If yaml is usable outside of Bootable JAR, we could have an issue with Openshift s2i images. The YAML being applied first, the server would be not yet fully configured by the CLI script generated by our launch scripts (that currently relies on the same mechanism as the one used in WFCORE-5324).

@ehsavoie ehsavoie marked this pull request as ready for review April 26, 2021 10:02
@ehsavoie ehsavoie force-pushed the WFCORE-5343 branch 2 times, most recently from 80a9f2a to d74a824 Compare May 6, 2021 13:48
@rpelisse
Copy link

rpelisse commented Sep 16, 2021

I agree yaml file is easy to read. But something similar can be achieved with formatting of CLI commands. ( I know it is not yaml :) )

That's a big issue for integration into automation tools (such as Ansible). The CLI is not idempotent and thus we have customers writing pages and pages of CLI script while they could express their configuration using this feature.

I don't know of its usage outside of bootable JAR.

I'm part of an initiative to better integrate middleware products into Ansible (one use case being helping customers to replace their automation based on JON by Ansible) and this feature would help a lot. Currently, we use another tool, called JCliff, to help Ansible managed Wildfly or EAP. It's quite cumbersome and far less efficient than this feature.

Note: using JCliff forces us to deliver an extension for Ansible and maintain the associated code. With this feature, we can get ride off it and get a better performance and integration than we have currently.

@mchoma
Copy link

mchoma commented Sep 16, 2021

The CLI is not idempotent

Can be with if conditions. But is more talkative for sure.

this feature would help a lot.

Will you expose in ansible yaml as is? Wouldn't there be any "facade" in ansible playbooks which can hide CLI talkativness? Isn't it problem for you yaml syntax is subset of CLI? Wouldn't you end in situation you will have to use CLI underneath anyway in some cases? My point is don't you need yaml have same power as CLI when used in Ansible?

@rpelisse
Copy link

rpelisse commented Sep 16, 2021

@mchoma I built a prototype based on @ehsavoie work and I have yet to find a use case where I need to use the CLI directly (apart from app deployment). I simply deploy the YAML configuration as a template, so I don't really need a facade. I actually find the direct mapping between CLI syntax and the YAML very comfortable. I also think people using Ansible and used to its YAML will be happy to the template has a close syntax.

But is more talkative for sure.

I think you have largely underestimated the issue here. If I take my prototype, here is what I configured :

  • adding a new JDBC driver (postgresql)
  • deploying a new datasource
  • configuring mod_cluster
  • adding a JMS queue
  • set some system properties

Pretty basic, right? With this feature, all of this is configured in one place within Ansible : the template file producing the YAML configuration. The config is loaded and executed when the server boots in a matter of milliseconds.

If the configuration evolves after the installation was performed, Ansible will detect the changes in the resulting file produced by the template, so Ansible will trigger a server restart to apply the change.

Now, let's do the same with CLI. First, I need to wrap CLI into a somewhat reusable Ansible task (akin to that), then I need to call this generic task for each item... This requires several exchanges between Ansible and the server and takes way more time than what this feature offer. And let's not even discussed what happens if one query fails and dealing with the configuration that requires the server to be reloaded...

OK, let's say you have done of all of that and that is somehow working for the installation part. Now someone comes and change the configuration. Then you need to make CLI scripts able to detect that there is a differences between the requested configuration and the state of the server....

So it's not more talkative. It's pages of custom scripts versus one configuration file. Bottom line, it's small feature for the appserver, but it's huge help for integrating Wildfly into Ansible (and other tool like Puppet or Chef).

@ehsavoie
Copy link
Contributor Author

Also I would say that the YAML is less verbose than a cli script as you only describe some configuration instead of describing the operations to be applied to create the final configuration. It is a very different perspective.

Copy link
Member

@xstefank xstefank left a comment

Choose a reason for hiding this comment

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

LGTM

To create a resource we will look into the YAML subtree for the attributes that are defined, and add an `add` operation for it.
If the resource is created then we look for it attributes that are writeable in the YAML subtree and add a `write-attribute` operation for each of them. That means that we need to have every element in a list since we are not using `list-add` in this case.
Then we process the sub-resources if any exist.
We should be able to get the effective XML file (using the `read-config-as-xml` operation) to validate the resut of our YAML configuration.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
We should be able to get the effective XML file (using the `read-config-as-xml` operation) to validate the resut of our YAML configuration.
We should be able to get the effective XML file (using the `read-config-as-xml` operation) to validate the result of our YAML configuration.

@zhantemirov
Copy link

The AD is complete enough for the Jira's current state (Planning).
Maybe, it would be useful to have having one little thing (backward compatibility) described - Darran has touched this topic in his commentary.

@ehsavoie WDYT?

Otherwise LGTM.

@ehsavoie
Copy link
Contributor Author

@zhantemirov updated as not supported

Copy link

@mnovak1 mnovak1 left a comment

Choose a reason for hiding this comment

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

LGTM, just typo/formatting problems.

@yersan yersan merged commit e43b04e into wildfly:main Jun 28, 2024
1 check passed
@yersan
Copy link
Contributor

yersan commented Jun 28, 2024

Thanks to everyone involved in this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants