Set default server roles from env #15470
Conversation
|
Can you not @mention me in a commit message? 😆
@chrisarcand Has 👑 |
'd me with his epic commit 
| allow_any_instance_of(MiqServer).to receive(:set_assigned_roles).and_return(nil) | ||
| allow_any_instance_of(MiqServer).to receive(:sync_workers).and_return(nil) | ||
| allow_any_instance_of(MiqServer).to receive(:sync_log_level).and_return(nil) | ||
| allow_any_instance_of(MiqServer).to receive(:wait_for_started_workers).and_return(nil) |
|
LGTM, just fixup the commit so I don't get notifications everytime it's extracted to a new repo. |
bdunne
changed the title
- If no roles exist (new server), try to source them from the environment or fall back to the default in Settings
bdunne
force-pushed
the
set_default_server_roles_from_env
branch
from
8e1117b
to
780a69c
Compare
bdunne
force-pushed
the
set_default_server_roles_from_env
branch
from
780a69c
to
a3e5404
Compare
|
This direction looks good to me @bdunne and is even better now that I'm not getting trolled in the commit message ;-) It's still WIP though. |
bdunne
changed the title
|
Ok, got it working. Ready for review |
| self.role = ::Settings.server.role | ||
| end | ||
|
|
||
| def ensure_default_roles | ||
| MiqServer.my_server.add_settings_for_resource(:server => {:role => ENV["DEFAULT_ROLES"]}) if role.blank? && ENV["DEFAULT_ROLES"].present? |
There was a problem hiding this comment.
Should environment variable names be prefixed with MIQ_ or MANAGEIQ_?
There was a problem hiding this comment.
I'm not too concerned about the variable name since I hope it will be short lived, but I namespaced it to avoid conflict.
|
Checked commits bdunne/manageiq@eeabe1e~...6e84d92 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
EDIT: Updated description due to env variable rename |
| self.role = ::Settings.server.role | ||
| end | ||
|
|
||
| def ensure_default_roles | ||
| MiqServer.my_server.add_settings_for_resource(:server => {:role => ENV["MIQ_SERVER_DEFAULT_ROLES"]}) if role.blank? && ENV["MIQ_SERVER_DEFAULT_ROLES"].present? |
There was a problem hiding this comment.
@bdunne I don't see the discussion on this change, but why the move from self.role= ?
There was a problem hiding this comment.
Ohhhh I see...if you don't change it then you end up with it syncing back from config and everything is wonky. Even so, I don't like using add_settings_for_resource because it broadcasts a config change to all running servers.
There was a problem hiding this comment.
I'm wondering if it's cleaner to extract this into a separate method like maybe save_settings and call it directly...
manageiq/app/models/mixins/configuration_management_mixin.rb
Lines 13 to 16 in a13b67b
Set default server roles from
ENV["MIQ_SERVER_DEFAULT_ROLES"]if they don't already exist. This allows us to pass the default roles in the env variable to the backend manageiq container. (don't include UI/WS/etc. which will never receive traffic)For the frontend, we don't provide the env variable and it will take the defaults from
Settings.cc @jrafanie @carbonin