Skip to content

Commit

Permalink
Merge pull request #692 from cderici/merge-3.0-compatibility
Browse files Browse the repository at this point in the history
#692

#### Description

This PR introduces the mechanism to have both the old and the new clients generated from `2.9` and `3.0` schema respectively. Based on the `server_version'` information within the connection, correct facade is picked from the correct client among the correct client group. With this, `pylibjuju` should be able to work with both `juju 2.9` and `juju 3.0`.

This PR also merges the `juju-3.0-compatibility` branch onto the `master`.

Most important changes include:
- we have both the clients generated from `juju 3.0` schema, and from the `juju 2.9` schema. The `facade` code is updated to generate the `_client` code to have both sets of clients and facades inside, and whenever a facade is needed, to dynamically pick the correct facade from the correct client based on the version set in the underlying connection with juju.
- `client.py` module is no longer a regular Python module, it's a class instance that acts like a module within Python runtime. Based on the juju version, it pulls all the bindings (`_client`s and the `_definition`s) dynamically from respective correct modules, which are named `_client` and `_definitions` for `juju 3.0`, and `_2_9_client` and `_2_9_definitions` for `juju 2.9`. So whenever in the code we pull a binding from the client, like `client.CharmOrigin`, the `CharmOrigin` object we get comes from the correct `_definitions` module.

#### QA Steps

All the integration tests should pass here on CI, and also on a `juju 3.0` controller.

So bootstrap from juju's `develop` branch (or `snap install juju --channel=latest/beta`) and run the following in libjuju:

```sh
make test
```

#### Notes & Discussion

- Everything on the CI tests should pass except the intermittent stuff we already know about, and also some charmhub stuff that we observed before I started working on this.

- There will be some tests failing for sure, I couldn't check all the tests yet, so I'm labeling this as `do not merge` until we see all green (or almost all green).
  • Loading branch information
jujubot authored Jul 27, 2022
2 parents fbbaf78 + e516285 commit 9349e45
Show file tree
Hide file tree
Showing 91 changed files with 256,064 additions and 2,719 deletions.
25 changes: 17 additions & 8 deletions juju/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,9 @@ async def get_actions(self, schema=False):
:return dict: The charms actions, empty dict if none are defined.
"""
actions = {}
entity = [{"tag": self.tag}]
entity = {"tag": self.tag}
action_facade = client.ActionFacade.from_connection(self.connection)
results = (
await action_facade.ApplicationsCharmsActions(entities=entity)).results
results = (await action_facade.ApplicationsCharmsActions(entities=[entity])).results
for result in results:
if result.application_tag == self.tag and result.actions:
actions = result.actions
Expand Down Expand Up @@ -563,7 +562,10 @@ async def set_config(self, config):
else:
raise JujuApplicationConfigError(config, [k, v])

await app_facade.Set(application=self.name, options=str_config)
return await app_facade.SetConfigs(args=[{
"application": self.name,
"config": str_config,
}])

async def reset_config(self, to_default):
"""
Expand All @@ -577,7 +579,10 @@ async def reset_config(self, to_default):
log.debug(
'Restoring default config for %s: %s', self.name, to_default)

return await app_facade.Unset(application=self.name, options=to_default)
return await app_facade.UnsetApplicationsConfig(args=[{
"application": self.name,
"options": to_default,
}])

async def set_constraints(self, constraints):
"""Set machine constraints for this application.
Expand Down Expand Up @@ -619,7 +624,7 @@ async def refresh(
if switch is not None and revision is not None:
raise ValueError("switch and revision are mutually exclusive")

client_facade = client.ClientFacade.from_connection(self.connection)
charms_facade = client.CharmsFacade.from_connection(self.connection)
resources_facade = client.ResourcesFacade.from_connection(
self.connection)
app_facade = self._facade()
Expand All @@ -644,11 +649,15 @@ async def refresh(
if charm_url == self.data['charm-url']:
raise JujuError('already running charm "%s"' % charm_url)

# TODO (caner) : this needs to be revisited and updated with the charmhub stuff
origin = client.CharmOrigin(source="charm-store",
risk=channel,
)
# Update charm
await client_facade.AddCharm(
await charms_facade.AddCharm(
url=charm_url,
force=force,
channel=channel
charm_origin=origin,
)

# Update resources
Expand Down
12 changes: 6 additions & 6 deletions juju/bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def __init__(self, model, trusted=False, forced=False):
model.connection())
self.ann_facade = client.AnnotationsFacade.from_connection(
model.connection())
self.machine_manager_facade = client.MachineManagerFacade.from_connection(
model.connection())

# Feature detect if we have the new charms facade, otherwise fallback
# to the client facade, when making calls.
Expand Down Expand Up @@ -617,10 +619,8 @@ async def run(self, context):
self.application, charm, overrides=self.resources)
elif Schema.CHARM_HUB.matches(url.schema):
c_hub = charmhub.CharmHub(context.model)
info = await c_hub.info(url.name, channel=self.channel)
if info.errors.error_list.code:
raise JujuError("unable to resolve the charm {} with channel {}".format(url.name, channel))
origin.id_ = info.result.id_
id_, _ = c_hub.get_charm_id(url.name)
origin.id_ = id_
resources = await context.model._add_charmhub_resources(
self.application, charm, origin, overrides=self.resources)
else:
Expand Down Expand Up @@ -726,7 +726,7 @@ async def run(self, context):
if Schema.CHARM_STORE.matches(url.schema):
entity_id = await context.charmstore.entityId(self.charm, channel=self.channel)
log.debug('Adding %s', entity_id)
await context.client_facade.AddCharm(channel=self.channel, url=entity_id, force=False)
await context.charms_facade.AddCharm(channel=self.channel, url=entity_id, force=False)
identifier = entity_id
origin = client.CharmOrigin(source="charm-store", risk="stable")

Expand Down Expand Up @@ -837,7 +837,7 @@ async def run(self, context):

# Submit the request.
params = client.AddMachineParams(**params)
results = await context.client_facade.AddMachines(params=[params])
results = await context.machine_manager_facade.AddMachines(params=[params])
error = results.machines[0].error
if error:
raise ValueError("Error adding machine: %s" % error.message)
Expand Down
31 changes: 31 additions & 0 deletions juju/charmhub.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,42 @@
from .client import client
from .errors import JujuError
from juju import jasyncio

import requests
import json


class CharmHub:
def __init__(self, model):
self.model = model

def request_charmhub_with_retry(self, url, retries):
for attempt in range(retries):
_response = requests.get(url)
if _response.status_code == 200:
return _response
jasyncio.sleep(5)
raise JujuError("Got {} from {}".format(_response.status_code, url))

def get_charm_id(self, charm_name):
conn, headers, path_prefix = self.model.connection().https_connection()

url = "http://api.snapcraft.io/v2/charms/info/{}".format(charm_name)
_response = self.request_charmhub_with_retry(url, 5)
response = json.loads(_response.text)
return response['id'], response['name']

def is_subordinate(self, charm_name):
conn, headers, path_prefix = self.model.connection().https_connection()

url = "http://api.snapcraft.io/v2/charms/info/{}?fields=default-release.revision.subordinate".format(charm_name)
_response = self.request_charmhub_with_retry(url, 5)
response = json.loads(_response.text)
return 'subordinate' in response['default-release']['revision']

# TODO (caner) : we should be able to recreate the channel-map through the
# api call without needing the CharmHub facade

async def info(self, name, channel=None):
"""info displays detailed information about a CharmHub charm. The charm
can be specified by the exact name.
Expand Down
12 changes: 8 additions & 4 deletions juju/client/_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

from juju.client._definitions import *

from juju.client import _client2, _client1, _client3, _client4, _client5, _client8, _client7, _client9, _client10, _client6, _client12, _client11, _client13, _client15, _client16, _client17, _client18

from juju.client import _client2, _client1, _client3, _client4, _client5, _client8, _client7, _client9, _client10, _client6, _client12, _client11, _client13, _client15, _client16, _client17, _client18, _client14


CLIENTS = {
Expand All @@ -23,11 +24,11 @@
"15": _client15,
"16": _client16,
"17": _client17,
"18": _client18
"18": _client18,
"14": _client14
}



def lookup_facade(name, version):
"""
Given a facade name and version, attempt to pull that facade out
Expand All @@ -45,7 +46,6 @@ def lookup_facade(name, version):
"{}".format(name))



class TypeFactory:
@classmethod
def from_connection(cls, connection):
Expand Down Expand Up @@ -259,6 +259,10 @@ class EntityWatcherFacade(TypeFactory):
pass


class EnvironUpgraderFacade(TypeFactory):
pass


class ExternalControllerUpdaterFacade(TypeFactory):
pass

Expand Down
Loading

0 comments on commit 9349e45

Please sign in to comment.