Merge lp:~nuclearbob/utah/bug1087679 into lp:utah

[Max Brustkern]

This branch changes SSHMixin to use config.user when connecting for file transfers. I found this error when I used a different user while working on lp:1087679 I think we may be able to fix that issue with just a preseed change, but I'm still testing that. I know we can fix it with a preseed change and this patch, and I think this patch is worth having anyway, since it fixes a potential problem with different configuration settings.

[lp:~nuclearbob/utah/bug1087679]

775. By Max Brustkern on 2012-12-07

Added support for rewriting ~-based paths in config options

776. By Max Brustkern on 2012-12-07

Properly quoted option names

[Max Brustkern (nuclearbob)]

I've added support to config for rewriting ~-based paths. The idea behind this is that we can specify ~/.ssh/utah as the sshprivatekey, and then we won't have to worry about what user is running the job. I'm testing this now.

review: Needs Information

[lp:~nuclearbob/utah/bug1087679]

777. By Max Brustkern on 2012-12-07

Corrected global variable capitalization

778. By Max Brustkern on 2012-12-07

Added support for None types

779. By Max Brustkern on 2012-12-07

Fixed pep8 warning

[Javier Collado (javier.collado)]

I like the idea of expanding user path automatically.

Looking at the `config.py` I see that `os.path.expanduser` is still used for:
- sshprivatekey
- sshpublickey
- sshknonhosts
- vmpath

Please remove those calls since the user path is going to be expanded anyway
later.

review: Needs Fixing

[lp:~nuclearbob/utah/bug1087679]

780. By Max Brustkern on 2012-12-10

Removed things that get expanded later from the list of things to expand the user path on

[Max Brustkern (nuclearbob)]

Those are out now.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

What I meant is that it made sense to me to remove all the calls to
`os.path.expanduser` in the `LOCALDEFAULTS` dictionary to have just a common
place where `os.path.expanduser` was called.

However, what I see is those calls are still there and the keys affected have
been removed from `PATHOPTIONS`. This removes some duplication as well, but I'm
not sure why this is the best option. I guess it's probably based on the way
`LOCALDEFAULTS` is processed. Is that the case?

review: Needs Information

[Max Brustkern (nuclearbob)]

Okay, I think I understand better now. LOCALDEFAULTS contains all the keys that can't be written to the config file during package build time, because they depend on local system things (such as the expansion of user paths.) Would it make more sense to remove the expanduser from their definitions and put them back in PATHOPTIONS? That would make sense to me.

[Javier Collado (javier.collado)]

Yes, that's what I was trying to ask for in my first comment.

[lp:~nuclearbob/utah/bug1087679]

781. By Max Brustkern on 2012-12-10

Moved expanduser around per Javier's suggestion

[Max Brustkern (nuclearbob)]

Okay, that should be better now.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

Looks good to me now. Thanks.

review: Approve

=== modified file 'utah/config.py'
--- utah/config.py	2012-12-10 13:01:20 +0000
+++ utah/config.py	2012-12-10 19:02:18 +0000
@@ -143,16 +143,15 @@
     # Hostname (used for logfile among other things)
     hostname=socket.gethostname(),
     # SSH private key to use for provisioned machine logins
-    sshprivatekey=os.path.join(os.path.expanduser('~'), '.ssh', 'utah'),
+    sshprivatekey=os.path.join('~', '.ssh', 'utah'),
     # SSH public key to install on provisioned machines
-    sshpublickey=os.path.join(os.path.expanduser('~'), '.ssh', 'utah.pub'),
+    sshpublickey=os.path.join('~', '.ssh', 'utah.pub'),
     # SSH known hosts file
-    sshknownhosts=os.path.join(os.path.expanduser('~'), '.ssh',
-                               'known_hosts'),
+    sshknownhosts=os.path.join('~', '.ssh', 'known_hosts'),
     # User to install and use on provisioned machines
     user=getpass.getuser(),
     # Path for VM files
-    vmpath=os.path.join(os.path.expanduser('~'), 'vm'),
+    vmpath=os.path.join('~', 'vm'),
 )
 
 # These depend on other config options, so they're added last.
@@ -231,6 +230,31 @@
                 sys.stderr.write(conffile + ' is not a valid JSON file')
 
 
+# Support ~-based paths on variables that use paths
+PATHOPTIONS = (
+    'dlcommand',
+    'image',
+    'initrd',
+    'isodir',
+    'kernel',
+    'logpath',
+    'nfsconfigfile',
+    'preseed',
+    'pxedir',
+    'wwwdir',
+    'xml',
+    'sshprivatekey',
+    'sshpublickey',
+    'sshknownhosts',
+    'vmpath',
+    'logfile',
+    'debuglog',
+)
+for option in PATHOPTIONS:
+    if CONFIG[option] is not None:
+        CONFIG[option] = os.path.expanduser(CONFIG[option])
+
+
 # Allow variables to be accessed via config.variable
 locals().update(CONFIG)
 

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-12-06 16:30:34 +0000
+++ utah/provisioning/provisioning.py	2012-12-10 19:02:18 +0000
@@ -742,6 +742,7 @@
 
         self.activecheck()
         self.ssh_client.connect(self.name,
+                                username=config.user,
                                 key_filename=config.sshprivatekey)
         sftp_client = self.ssh_client.open_sftp()
         failed = []
@@ -775,6 +776,7 @@
 
         self.activecheck()
         self.ssh_client.connect(self.name,
+                                username=config.user,
                                 key_filename=config.sshprivatekey)
         sftp_client = self.ssh_client.open_sftp()
         if os.path.isdir(target):
@@ -799,6 +801,7 @@
         """
         self.activecheck()
         self.ssh_client.connect(self.name,
+                                username=config.user,
                                 key_filename=config.sshprivatekey)
         sftp_client = self.ssh_client.open_sftp()
         myfiles = []
@@ -871,6 +874,7 @@
         self.logger.info('Checking for ssh availability')
         try:
             self.ssh_client.connect(self.name,
+                                    username=config.user,
                                     key_filename=config.sshprivatekey)
         except socket.error as err:
             raise UTAHProvisioningException(str(err), retry=True)