[Freeipa-devel] [PATCH] 1074 limit service list

[Simo Sorce]

On Tue, 2012-12-04 at 14:03 -0500, Rob Crittenden wrote:
> Only touch the service list in the server installer and ipactl.

Nack, comments inline.


> diff --git a/install/tools/ipactl b/install/tools/ipactl
> index
> f931a27257aaca987db46c7295cbb4708a6801f7..2a60b9eaf4e1ffb536fd389d17ff747c99492a35 100755
> --- a/install/tools/ipactl
> +++ b/install/tools/ipactl
> @@ -175,6 +175,9 @@ def get_config_from_file():
>  
>      svc_list = []
>  
> +    if not os.path.exists(ipaservices.get_svc_list_file()):
> +        return svc_list
> +

This break the fallback we have in ipa_stop()
We expect an exception or a non empty list there.

>      try:
>          f = open(ipaservices.get_svc_list_file(), 'r')
>          svc_list = json.load(f)
> @@ -469,7 +472,7 @@ def main():
>          else:
>              raise e
>  
> -    api.bootstrap(context='cli', debug=options.debug)
> +    api.bootstrap(context='ipactl', debug=options.debug)
>      api.finalize()
>  
>      if '.' not in api.env.host:
> diff --git a/ipapython/platform/base.py b/ipapython/platform/base.py
> index
> 8385e1038c0609ae06a7a4a25d844de48360f19e..d1d42b3259d735d88df4e9c1698d5f8781dd1124 100644
> --- a/ipapython/platform/base.py
> +++ b/ipapython/platform/base.py
> @@ -136,12 +136,15 @@ class PlatformService(object):
>      def __init__(self, service_name):
>          self.service_name = service_name
>  
> -    def start(self, instance_name="", capture_output=True,
> wait=True):
> +    def start(self, instance_name="", capture_output=True, wait=True,
> +        update_list=True):

Can we call this something like 'store_action' or 'remember_action' ?
'update_list' is quite opaque as name.
Or maybe at least qualify: 'update_stop_list'

>          """
>          When a service is started record the fact in a special file.
>          This allows ipactl stop to always stop all services that have
>          been started via ipa tools
>          """
> +        if not update_list:
> +            return
>          svc_list = []
>          try:
>              f = open(SVC_LIST_FILE, 'r')
> @@ -159,10 +162,12 @@ class PlatformService(object):
>          f.close()
>          return
>  
> -    def stop(self, instance_name="", capture_output=True):
> +    def stop(self, instance_name="", capture_output=True,
> update_list=True):
>          """
>          When a service is stopped remove it from the service list
> file.
>          """
> +        if not update_list:
> +            return
>          svc_list = []
>          try:
>              f = open(SVC_LIST_FILE, 'r')
> diff --git a/ipapython/platform/systemd.py
> b/ipapython/platform/systemd.py
> index
> bb6c009299adc9ca8488308afffdd767975fc2ae..359b593594f2db2b1a1810abbd71deebbf33677e 100644
> --- a/ipapython/platform/systemd.py
> +++ b/ipapython/platform/systemd.py
> @@ -91,13 +91,21 @@ class SystemdService(base.PlatformService):
>  
>      def stop(self, instance_name="", capture_output=True):
>          ipautil.run(["/bin/systemctl", "stop",
> self.service_instance(instance_name)], capture_output=capture_output)
> -        super(SystemdService, self).stop(instance_name)
> +        if 'context' in api.env and api.env.context in ['ipactl',
> 'installer']:

Will this trigger also when ipa-client-install is run ?
We have a patch on the list to restart sssd via ipa-client-install.
sssd *should* not end in the stop-list though.

> +            update_list = True
> +        else:
> +            update_list = False
> +        super(SystemdService,
> self).stop(instance_name,update_list=update_list)
>  
>      def start(self, instance_name="", capture_output=True,
> wait=True):
>          ipautil.run(["/bin/systemctl", "start",
> self.service_instance(instance_name)], capture_output=capture_output)
> +        if 'context' in api.env and api.env.context in ['ipactl',
> 'installer']:
> +            update_list = True
> +        else:
> +            update_list = False
>          if wait and self.is_running(instance_name):
> 
> self.__wait_for_open_ports(self.service_instance(instance_name))
> -        super(SystemdService, self).start(instance_name)
> +        super(SystemdService, self).start(instance_name,
> update_list=update_list)
>  
>      def restart(self, instance_name="", capture_output=True,
> wait=True):
>          # Restart command is broken before systemd-36-3.fc16
> 
In general looks good, otherwise.

Simo.
> 
-- 
Simo Sorce * Red Hat, Inc * New York