You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by ProjectMoon <gi...@git.apache.org> on 2015/12/03 19:42:23 UTC

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

GitHub user ProjectMoon opened a pull request:

    https://github.com/apache/cloudstack/pull/1162

    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.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/greenqloud/cloudstack pr-no-start

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1162.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1162
    
----
commit 1f2357dc22f78acc8a5e6a1cb3e9269488aa499f
Author: jeff <je...@greenqloud.com>
Date:   2015-12-03T18:36:10Z

    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.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46671989
  
    --- Diff: python/lib/cloudutils/serviceConfigServer.py ---
    @@ -135,9 +135,14 @@ def checkHostName():
                     self.syscfg.svo.disableService("tomcat6")
             except:
                 pass
    -            
    +
             self.syscfg.svo.stopService("cloudstack-management")
    -        if self.syscfg.svo.enableService("cloudstack-management"):
    -            return True
    +
    +        if self.syscfg.env.noStart == False:
    +            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)
    --- End diff --
    
    We crossed each other here, what do you mean?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46672120
  
    --- Diff: python/lib/cloudutils/serviceConfigServer.py ---
    @@ -135,9 +135,14 @@ def checkHostName():
                     self.syscfg.svo.disableService("tomcat6")
             except:
                 pass
    -            
    +
             self.syscfg.svo.stopService("cloudstack-management")
    -        if self.syscfg.svo.enableService("cloudstack-management"):
    -            return True
    +
    +        if self.syscfg.env.noStart == False:
    +            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)
    --- End diff --
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46671835
  
    --- Diff: python/lib/cloudutils/serviceConfigServer.py ---
    @@ -135,9 +135,14 @@ def checkHostName():
                     self.syscfg.svo.disableService("tomcat6")
             except:
                 pass
    -            
    +
             self.syscfg.svo.stopService("cloudstack-management")
    -        if self.syscfg.svo.enableService("cloudstack-management"):
    -            return True
    +
    +        if self.syscfg.env.noStart == False:
    +            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)
    --- End diff --
    
    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`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:

    https://github.com/apache/cloudstack/pull/1162#issuecomment-161926917
  
    Removed the unintentional change to the initLoging (sic) method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46670911
  
    --- Diff: python/lib/cloudutils/serviceConfigServer.py ---
    @@ -135,9 +135,14 @@ def checkHostName():
                     self.syscfg.svo.disableService("tomcat6")
             except:
                 pass
    -            
    +
             self.syscfg.svo.stopService("cloudstack-management")
    -        if self.syscfg.svo.enableService("cloudstack-management"):
    -            return True
    +
    +        if self.syscfg.env.noStart == False:
    +            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)
    --- End diff --
    
    Sorry to be a nag @ProjectMoon but can you change the path in the log message as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46671185
  
    --- Diff: python/lib/cloudutils/serviceConfigServer.py ---
    @@ -135,9 +135,14 @@ def checkHostName():
                     self.syscfg.svo.disableService("tomcat6")
             except:
                 pass
    -            
    +
             self.syscfg.svo.stopService("cloudstack-management")
    -        if self.syscfg.svo.enableService("cloudstack-management"):
    -            return True
    +
    +        if self.syscfg.env.noStart == False:
    +            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)
    --- End diff --
    
    I will change it, but it should be known that this was actually the original message. :) Does need to be fixed though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46671886
  
    --- Diff: python/lib/cloudutils/serviceConfigServer.py ---
    @@ -135,9 +135,14 @@ def checkHostName():
                     self.syscfg.svo.disableService("tomcat6")
             except:
                 pass
    -            
    +
             self.syscfg.svo.stopService("cloudstack-management")
    -        if self.syscfg.svo.enableService("cloudstack-management"):
    -            return True
    +
    +        if self.syscfg.env.noStart == False:
    +            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)
    --- End diff --
    
    yeah you are right, and am I glad I forgot to add a :-1: ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46613321
  
    --- Diff: client/bindir/cloud-setup-management.in ---
    @@ -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")
    --- End diff --
    
    Will this hardcoded value fly on windows? (yes it is used as ms platform by some)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1162


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/1162#issuecomment-162644589
  
    LGTM:
    
    Old default behaviour is to enable service:
    ```
    [root@cs1 x86_64]# cloudstack-setup-management --tomcat7 
    Starting to configure CloudStack Management Server:
    Configure Firewall ...        [OK]
    Configure CloudStack Management Server ...[OK]
    CloudStack Management Server setup is Done!
    [root@cs1 x86_64]# ls -la /etc/systemd/system/multi-user.target.wants/cloudstack-management.service
    lrwxrwxrwx. 1 root root 53 Dec  7 20:03 /etc/systemd/system/multi-user.target.wants/cloudstack-management.service -> /usr/lib/systemd/system/cloudstack-management.service
    ```
    
    And with extra option it is not enabled:
    
    ```
    [root@cs1 x86_64]# systemctl disable cloudstack-management
    rm '/etc/systemd/system/multi-user.target.wants/cloudstack-management.service'
    [root@cs1 x86_64]# ls -la /etc/systemd/system/multi-user.target.wants/cloudstack-management.service
    ls: cannot access /etc/systemd/system/multi-user.target.wants/cloudstack-management.service: No such file or directory
    [root@cs1 x86_64]# cloudstack-setup-management --tomcat7 --no-start
    Starting to configure CloudStack Management Server:
    Configure Firewall ...        [OK]
    Configure CloudStack Management Server ...Configured successfully, but not starting management server.
    [OK]
    CloudStack Management Server setup is Done!
    [root@cs1 x86_64]# ls -la /etc/systemd/system/multi-user.target.wants/cloudstack-management.service
    ls: cannot access /etc/systemd/system/multi-user.target.wants/cloudstack-management.service: No such file or directory
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/1162#issuecomment-162341391
  
    lgtm
    @remibergsma can you schedule a regression?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1162#discussion_r46618985
  
    --- Diff: client/bindir/cloud-setup-management.in ---
    @@ -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")
    --- End diff --
    
    Ah yes this was an unintended change. I will put it back to the original @MSLOGDIR@ tomorrow.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/1162#issuecomment-162342893
  
    @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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---