-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add support for not (re)starting server after cloud-setup-management. #1162
Conversation
@@ -24,20 +24,23 @@ from cloudutils.globalEnv import globalEnv | |||
from cloudutils.serviceConfigServer import cloudManagementConfig | |||
from optparse import OptionParser | |||
if __name__ == '__main__': | |||
initLoging("@MSLOGDIR@/setupManagement.log") | |||
initLoging("/var/log/cloudstack/management/setupManagement.log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this hardcoded value fly on windows? (yes it is used as ms platform by some)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes this was an unintended change. I will put it back to the original @MSLOGDIR@ tomorrow.
This adds an option to the cloud-setup-management script to not start the management server after a successful configuration of it. The primary motivation for this is to avoid circular dependency issues on systems that use systemd. When calling cloud-setup-management from a unit with a Before= directive on a service depending on cloudstack-management, the process will deadlock because /usr/bin/service will delegate to systemd, which is waiting for the Before service to start.
Removed the unintentional change to the initLoging (sic) method. |
if self.syscfg.svo.enableService("cloudstack-management"): | ||
return True | ||
else: | ||
raise CloudRuntimeException("Failed to configure %s, please see the /var/log/cloudstack/management/setupManagement.log for detail"%self.serviceName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to be a nag @ProjectMoon but can you change the path in the log message as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change it, but it should be known that this was actually the original message. :) Does need to be fixed though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, one possible issue with this: will the serviceConfigServer.py script be processed by the *.in ant tasks, or will a new antrun task need to be added to process the files found at python/lib/cloudutils
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah you are right, and am I glad I forgot to add a 👎 ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We crossed each other here, what do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original message had the log message in the wrong place (I think it was pointing at /var/log/cloudstack-management/setupManagement.log
). I changed it to the right path on Linux systems (for Ubuntu and friends). But probably the log path can be fixed as part of this commit or in another one. But in order for the replace.properties stuff to work, there is an antrun task I believe. Not sure it would currently process files in the directory that serviceConfigServer.py is in.
lgtm |
@DaanHoogland That makes no sense. We should build RPMs and install mgt server from it and test both the new and existing features. The integration tests do not hit this code as far as I know. |
LGTM: Old default behaviour is to enable service:
And with extra option it is not enabled:
|
Add support for not (re)starting server after cloud-setup-management.This adds an option to the cloud-setup-management script to not start the management server after a successful configuration of it. The primary motivation for this is to avoid circular dependency issues on systems that use systemd. When calling cloud-setup-management from a unit with a Before= directive on a service depending on cloudstack-management, the process will deadlock because /usr/bin/service will delegate to systemd, which is waiting for the Before service to start. Executing the cloud-setup-management script with this new `--no-start` option will simply leave the management server stopped after a successful configuration. systemd can then be bypassed with `export _SYSTEMCTL_SKIP_REDIRECT=1` and using the init.d script. * pr/1162: Add support for not (re)starting server after cloud-setup-management. Signed-off-by: Remi Bergsma <github@remi.nl>
[CLOUDSTACK-9645] Followup fix for #1162: Add support for not (re)starting server after cloud-setup-management.New version against #1330, opened against master. ``` Moving stop of management in config inside if statement. It will only run the stop command when --no-start is specified. ``` * pr/1566: CLOUDSTACK-9645: Moving stop of management in config inside if statement. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This adds an option to the cloud-setup-management script to not start the management server after a successful configuration of it.
The primary motivation for this is to avoid circular dependency issues on systems that use systemd. When calling cloud-setup-management from a unit with a Before= directive on a service depending on
cloudstack-management, the process will deadlock because /usr/bin/service will delegate to systemd, which is waiting for the Before service to start.
Executing the cloud-setup-management script with this new
--no-start
option will simply leave the management server stopped after a successful configuration. systemd can then be bypassed withexport _SYSTEMCTL_SKIP_REDIRECT=1
and using the init.d script.