#81 install: server-install: abort installation on permission issue
Merged 3 years ago by puiterwijk. Opened 3 years ago by puiterwijk.

@@ -41,7 +41,7 @@ 

  cherrypy.log.screen = False


  # Regular logging

- LOGFILE = '/var/log/ipsilon-install.log'

+ LOGFILE = os.environ.get('LOGFILE', '/var/log/ipsilon-install.log')

  logger = logging.getLogger()



@@ -57,8 +57,7 @@ 


          lh = logging.FileHandler(LOGFILE)

      except IOError, e:

-         print >> sys.stderr, 'Unable to open %s (%s)' % (LOGFILE, str(e))

-         lh = logging.StreamHandler(sys.stderr)

+         sys.exit('Unable to open %s (%s)' % (LOGFILE, str(e)))

      formatter = logging.Formatter('[%(asctime)s] %(message)s')



file modified

@@ -207,6 +207,11 @@ 


      def setup_idp_server(self, profile, name, addr, port, env):

          http_conf_file = self.setup_http(name, addr, port)

+         logfile = os.path.join(self.testdir, name, 'logs', 'install.log')

+         if env:

+             env['LOGFILE'] = logfile

+         else:

+             env = {'LOGFILE': logfile}

          cmd = [os.path.join(self.rootdir,


                 '--config-profile=%s' % profile]

no initial comment

Why not sys.exit(message)?

Should there be a rollback of things already done? I don't know, maybe not.

Why not sys.exit(message)?
Because I was unaware of that version of sys.exit, will update the PR.

Should there be a rollback of things already done? I don't know, maybe not.
Well, openlogs() is called inside main(), before we even load the plugins, and way before we even parse any arguments.
At this point in the code, we have no output whatsoever yet.

"output" being any change on the system.

Well, there is one output: Unable to open file...

That being said change looks fine to me nonetheless

I would remove:

      print >> sys.stderr, 'Unable to open %s (%s)' % (LOGFILE, str(e))
      lh = logging.StreamHandler(sys.stderr)

And add:

     sys.exit('Unable to open %s (%s)' % (LOGFILE, str(e))

This will print the message to stderr and return 1.

Right, so I realized that while this patch will fix that case, it will break the test suite, because that does not run as root.
The means that the test suite cannot write to the installer log file, which is perfectly fine since it can still continue the testrun.

I need to figure out a way to pass a different logging path without parsing the standard arguments, perhaps with environment variables. (we can't use standard arguments because those get parsed after we loaded all plugins, and we want to also add the loading of plugins to the installer log so we can't move the logging to before that).


3 years ago

I just rebased this patch to use an environment variable to pass the logfile location.

The reason for this is that at the moment we open the logs, we have not yet parsed the arguments, and we couldn't even, because part of the logs are what plugins we have loaded.


3 years ago

Pull-Request has been merged by puiterwijk

3 years ago