cf-charmers team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

Review: Approve

LGTM with a change to the UAA DB Relation class as described inline. The timeoouts are still scary and we need a better long term answer, but better to land this so the GUI works.

Diff comments:

> === modified file 'cloudfoundry/contexts.py'
> --- cloudfoundry/contexts.py	2014-10-27 19:48:56 +0000
> +++ cloudfoundry/contexts.py	2014-10-29 18:29:23 +0000
> @@ -1,5 +1,6 @@
>  import os
>  import yaml
> +import time
>  
>  from charmhelpers.core import host
>  from charmhelpers.core import hookenv
> @@ -31,6 +32,12 @@
>                                                 rid=rid, unit=unit))
>          return list(units)
>  
> +    def get_first(self, key):
> +        return self[self.name][0][key]
> +
> +    def get_all(self, key):
> +        return [u[key] for u in self[self.name]]
> +
>  
>  class StoredContext(dict):
>      """
> @@ -114,7 +121,7 @@
>  
>  class UAARelation(RelationContext):
>      name = 'uaa'
> -    interface = 'http'
> +    interface = 'uaa'
>      required_keys = ['login_client_secret', 'admin_client_secret', 'cc_client_secret', 'cc_token_secret',
>                       'service_broker_client_secret', 'servicesmgmt_client_secret', 'port']
>      port = 8081
> @@ -168,6 +175,19 @@
>          }
>  
>  
> +class UAADBRelation(RelationContext):
> +    name = 'uaa-db'
> +    interface = 'uaa-db'
> +    required_keys = MysqlRelation.required_keys

Wouldn't this be a subclass of MysqlRelation, this isn't a great contract with the other class, but inheritance is.

> +
> +    @classmethod
> +    def send_data(cls, job_name):
> +        # using send_data instead of provide_data to delay it until data_ready
> +        data = MysqlRelation()['db'][0]

Again, a violation of OOP.

> +        for rid in hookenv.relation_ids(cls.name):
> +            hookenv.relation_set(rid, data)
> +
> +
>  class LoginRelation(RelationContext):
>      name = 'login'
>      interface = 'http'
> @@ -370,6 +390,7 @@
>      @classmethod
>      def send_data(cls, job_name):
>          # using send_data instead of provide_data to delay it until data_ready
> +        time.sleep(120)  # hack to ensure migrations are done  :(

I wonder how they detect this in BOSH land. If we knew that migrations wrote a row in a table or something that we could probe that would help here. On a large deployment with many rows 120 might not be enough but in the common case its more than enough. I'd want to know that this isn't greatly blowing out the waterfall chart in the common case if we can detect a migration in a better way (and I'm not sure we can today).

>          data = MysqlRelation()['db'][0]
>          for rid in hookenv.relation_ids(cls.name):
>              hookenv.relation_set(rid, data)
> 
> === modified file 'cloudfoundry/services.py'
> --- cloudfoundry/services.py	2014-09-25 15:23:46 +0000
> +++ cloudfoundry/services.py	2014-10-29 18:29:23 +0000
> @@ -271,11 +271,14 @@
>          'jobs': [
>              {'job_name': 'uaa',
>               'mapping': {'db': mapper.uaadb},
> -             'provided_data': [contexts.UAARelation],
> +             'provided_data': [contexts.UAARelation,
> +                               contexts.UAADBRelation],
>               'required_data': [contexts.MysqlRelation,
>                                 contexts.NatsRelation,
> -                               contexts.UAARelation.remote_view]
> -             }
> +                               contexts.UAARelation.remote_view],
> +             'data_ready': [
> +                 contexts.UAADBRelation.send_data,
> +             ]},
>          ]
>      },
>  
> 


-- 
https://code.launchpad.net/~johnsca/charms/trusty/cloudfoundry/trunk-webadmin/+merge/240033
Your team Cloud Foundry Charmers is subscribed to branch lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.