Merge lp:~smoser/curtin/trunk.lp1520400 into lp:~curtin-dev/curtin/trunk

[Scott Moser]

initial fix for reading skew_data

I picked these from cloud-init revno 1147 that made similar changes. One
thing I don't yet have here is the retries that went in there.
Essentially you have to retry to put the skew in place. In cases where
MAAS is running curtin, cloud-init will have already done that for us
and we'll just read the offset from skew_data_file.

So this should be sufficient for MAAS, which is really the only place
that OAuth is currently used.

There is actually one other set of changes that i'd like to get.
Currently we set the oauth skew, and then use it, but if something
(systemd-timed) updated the clock during these things happening then our
skew could be come wrong. This is described more in bug 1531233.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:315
https://server-team-jenkins.canonical.com/job/curtin-ci/87/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/84/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/87/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I opened bug 1531233 to describe the solution I want to have in place at some point.

[Ryan Harper (raharper)]

You can still dump in json. We have an existing yaml load/read that does
exactly what you want.

On Tue, Jan 5, 2016 at 1:46 PM, Scott Moser <email address hidden> wrote:

>
>
> Diff comments:
>
> > === modified file 'curtin/url_helper.py'
> > --- curtin/url_helper.py 2016-01-04 18:03:45 +0000
> > +++ curtin/url_helper.py 2016-01-05 18:06:23 +0000
> > @@ -160,7 +160,7 @@
> > def read_skew_file(self):
> > if self.skew_data_file and os.path.isfile(self.skew_data_file):
> > with open(self.skew_data_file, mode="r") as fp:
> > - return json.load(fp.read())
> > + return json.load(fp)
> > return None
> >
>
> since we author and consume, i'd prefer json. json is more a machine
> format. also as it is right now, url_helper is very stand alone. it doesnt
> depend on any other curtin things.
>
> > def update_skew_file(self, host, value):
>
>
> --
> https://code.launchpad.net/~smoser/curtin/trunk.lp1520400/+merge/281682
> Your team curtin developers is requested to review the proposed merge of
> lp:~smoser/curtin/trunk.lp1520400 into lp:curtin.
>

[Scott Moser (smoser)]

yes, but that means:
 from curtin import config
 config.load_config(file)

the issues with that are:
a.) it means yaml has to be available to use curtin.url_helper (where json
is standard library). This is admittedly not that big of a deal.

b.) 'load_config' does not imply reading json, it implies reading a
config. this seems like a happenstance use of that function ("I know that
config is yaml and yaml is superset of json and my content is json so i
can use load_config". The single 'json.load' seems sane to me.

[Ryan Harper (raharper)]

OK. The next user of json loading needs to util.load_json() so we have it one place.

review: Approve

=== modified file 'curtin/url_helper.py'
--- curtin/url_helper.py	2016-01-04 18:03:45 +0000
+++ curtin/url_helper.py	2016-01-05 18:06:23 +0000
@@ -160,7 +160,7 @@
     def read_skew_file(self):
         if self.skew_data_file and os.path.isfile(self.skew_data_file):
             with open(self.skew_data_file, mode="r") as fp:
-                return json.load(fp.read())
+                return json.load(fp)
         return None
 
     def update_skew_file(self, host, value):
@@ -168,6 +168,8 @@
         if not self.skew_data_file:
             return
         cur = self.read_skew_file()
+        if cur is None:
+            cur = {}
         cur[host] = value
         with open(self.skew_data_file, mode="w") as fp:
             fp.write(json.dumps(cur))