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

Only start evmserverd after db and messaging config #195

Conversation

agrare
Copy link
Member

@agrare agrare commented Oct 11, 2022

When configuring the application there are possibly two steps, setting up the database and setting up messaging.

Each sub-section was stopping/starting/restarting evmserverd on its own so if you went through and configured the database and messaging we were starting evmserver after the database was setup, restarting it after clearing default messaging config (side-note I feel like we should start from a cleared state when we build the appliance), then restarting it again after completing messaging setup.

This changes the behavior to stop evmserverd if changes are going to be made to either the database or messaging, then starting it again after it completes.

Depends on:

@miq-bot miq-bot added the wip label Oct 11, 2022
@agrare agrare force-pushed the only_start_evmserverd_after_db_and_messaging_config branch from 2d9bae0 to 41aa311 Compare October 13, 2022 15:40
@Fryguy
Copy link
Member

Fryguy commented Oct 13, 2022

Merged #194 , so this can be rebased.

@agrare agrare force-pushed the only_start_evmserverd_after_db_and_messaging_config branch 5 times, most recently from 674f480 to b5a09c1 Compare October 14, 2022 14:51
@agrare agrare changed the title [WIP] Only start evmserverd after db and messaging config Only start evmserverd after db and messaging config Oct 14, 2022
@agrare agrare added enhancement and removed wip labels Oct 14, 2022
no_changes = database_action == "no_changes" && messaging_action == "no_changes"

# Stop evmserver while we make changes to the database and/or messaging configuration
unless no_changes
Copy link
Member

Choose a reason for hiding this comment

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

The double negative is killing me 😆

@@ -393,6 +393,14 @@ Static Network Configuration

messaging_action = ask_with_menu("Configure Messaging", messaging_options)

no_changes = database_action == "no_changes" && messaging_action == "no_changes"
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps, instead we make it a positive variable?

Suggested change
no_changes = database_action == "no_changes" && messaging_action == "no_changes"
changes_requested = database_action != "no_changes" || messaging_action != "no_changes"

Copy link
Member

Choose a reason for hiding this comment

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

Or

Suggested change
no_changes = database_action == "no_changes" && messaging_action == "no_changes"
changes_requested = !(database_action == "no_changes" && messaging_action == "no_changes")

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 done

@agrare agrare force-pushed the only_start_evmserverd_after_db_and_messaging_config branch from b5a09c1 to ce9ab16 Compare October 17, 2022 13:05
agrare and others added 2 commits October 17, 2022 09:06
Currently when configuring the application evmserverd is started after
setting up the database, it is then restarted twice after configuring
messaging (once for unconfiguring then once after configuring).

This is very inefficient and when we move to require messaging to be
configured in order to start evmserverd then the first start won't ever
complete anyway.
@agrare agrare force-pushed the only_start_evmserverd_after_db_and_messaging_config branch from ce9ab16 to 4d634e3 Compare October 17, 2022 13:06
@miq-bot
Copy link
Member

miq-bot commented Oct 17, 2022

Checked commits agrare/manageiq-appliance_console@2f0b91d~...4d634e3 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
10 files checked, 0 offenses detected
Everything looks fine. 🏆

@Fryguy Fryguy merged commit 5f9346d into ManageIQ:master Oct 17, 2022
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 18, 2022
Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
- Don't start evmserverd until messaging is configured (ManageIQ#196)
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 19, 2022
Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
- **BREAKING** Don't start evmserverd until messaging is configured (ManageIQ#196)
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 19, 2022
Breaking
- Don't start evmserverd until messaging is configured (ManageIQ#196)

Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 19, 2022
Breaking
- Don't start evmserverd until messaging is configured (ManageIQ#196)

Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 19, 2022
Breaking
- Don't start evmserverd until messaging is configured (ManageIQ#196)

Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
@agrare agrare deleted the only_start_evmserverd_after_db_and_messaging_config branch October 20, 2022 13:09
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 20, 2022
Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
- Don't start evmserverd until messaging is configured **BREAKING** (ManageIQ#196)
- Simplify messaging options by saving in yml files (ManageIQ#197)
agrare added a commit to agrare/manageiq-appliance_console that referenced this pull request Oct 20, 2022
Fixed
- Don't require pressing any key twice for message configuration (ManageIQ#193)

Added
- Report messaging configuration on summary info page (ManageIQ#190)

Changed
- Refactor EvmServer operations (ManageIQ#194)
- Only start evmserverd after all application configuration is done (ManageIQ#195)
- **BREAKING** Don't start evmserverd until messaging is configured **BREAKING** (ManageIQ#196)
- Simplify messaging options by saving in yml files (ManageIQ#197)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants