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-1456] Allow CLI class to support offline mode - REVERTED #1495

Merged
merged 1 commit into from
Apr 13, 2016

Conversation

jfdenise
Copy link
Contributor

This fix allows to use the class in offline mode and connected mode. Furthermore these changes cover the issue reported in WFCORE-1417 ( CLI was not usable after a failing connect and error was wrongly reported).
CLI class has been refactored in order to reduce code replication and remove some formatting issues.

The existing API and expected behaviour has been preserved:
CLI cli = CLI.newInstance();
cli.connect(....);

The new behaviour:
CLI cli = CLI.newInstance();
cli.cmd("embed-server");
cli.connect(...);
cli.disconnect();
cli.cmd("embed-server"); || cli.connect(...);
cli.disconnect();
...

@aloubyansky
Copy link
Contributor

Looks good. Thanks Jeff!

@kabir kabir merged commit 30bf6c8 into wildfly:master Apr 13, 2016

// Enable management
executeCommand(cli, "reload --admin-only=false");

Copy link
Contributor

@luck3y luck3y Apr 14, 2016

Choose a reason for hiding this comment

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

@jfdenise Sorry for coming in late on this. I think you may need to add a method to poll for the reload to be actually completed here, as it seems to be racing against itself. If I run in IDEA, the test always seems to pass, but running with mvn on the command line, it always seems to fail:

$ cd testsuite/standalone/
$ mvn test -Dtest=org.jboss.as.test.integration.management.cli.CLITestCase

What appears to be happening is that its trying to connect before the reload has completed.

If you take a look in ReloadHandler at the ensureServerRebootCompleted method, you'll see how that polls. We've recently added a new attribute runtime-configuration-state that should have the value "ok" when the reload is complete, which you can probably use here.

Some quick code if it helps: https://gist.github.com/luck3y/d563c3820c3e22c116327f4c5737bd58.

On a side note, I've often wondered about the immediate return of reload, it would seem more friendly if there was a way to have it block (perhaps as an optional parameter) until it was actually complete.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are aware of the issue and @jfdenise is going to follow up with a fix. Apparently, instead of the reload command, what gets executed is :reload operation, so the logic for making sure the controller is up in the reload handler is not run.
Sorry about that. And thanks for the tips!

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks @aloubyansky and @jfdenise! Also, that makes a lot of sense now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this test helped discover the CLI class behaviour, I am working on a patch to make CLI class to behave properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bug that is tracking the CLI command/operation mix: https://issues.jboss.org/browse/WFCORE-1471

@kabir kabir changed the title [WFCORE-1456] Allow CLI class to support offline mode [WFCORE-1456] Allow CLI class to support offline mode - REVERTED Apr 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants