Skip to content

Commit 434a080

Browse files
authored
Merge pull request #887 from cderici/avoid-parsing-endpoint-for-overlay-offers
#887 #### Description This is an addendum PR on #817, that attempts to fix #816. So the main thing here is that we don't accept endpoints that are not in `"<application-name>:<endpoint-name>[,...]"` format, like: ```sh $ j offer grafana ERROR endpoints must conform to format "<application-name>:<endpoint-name>[,...]" ``` unless, it's coming from a bundle overlay, such as: ```yaml description: Another overlay to create an offer applications: grafana: offers: dashboards: endpoints: - dashboards ``` Pylibjuju already has the necessary mechanism that passes along the application name and offer name etc from a bundle/overlay ([here](https://github.com/juju/python-libjuju/blob/ad62156b3bd18c949cd7f65a6b528cbaf023e121/juju/bundle.py#L1041)), this PR is a small change that adjusts the `controller.create_offer` (that actually calls the `ApplicationOffersFacade.Offer`) to not parse the endpoint to get the `offer_name` and `application_name` (that the function parameters are overriding anyways). Fixes #816 #### QA Steps #817 already adds a test that demonstrates the error, this PR extends that to also check for the offer in the model. So the following should pass: ``` tox -e integration -- tests/integration/test_model.py::test_deploy_bundle_with_multiple_overlays_with_include_files ```
2 parents ad62156 + d8f3fde commit 434a080

5 files changed

Lines changed: 42 additions & 15 deletions

File tree

juju/bundle.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ async def run(self, context):
10431043
def __str__(self):
10441044
endpoints = ""
10451045
if self.endpoints is not None:
1046-
endpoints = ":{}".format(self.endpoints.join(","))
1046+
endpoints = ":{}".format(",".join(self.endpoints))
10471047
return "create offer {offer_name} using {application}{endpoints}".format(offer_name=self.offer_name,
10481048
application=self.application,
10491049
endpoints=endpoints)

juju/controller.py

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -728,24 +728,32 @@ async def create_offer(self, model_uuid, endpoint, offer_name=None, application_
728728
consumers.
729729
730730
@param endpoint: holds the application and endpoint you want to offer
731-
@param offer_name: over ride the offer name to help the consumer
731+
@param offer_name: override the offer name to help the consumer
732+
@param application_name: overrides the application name in the endpoint
732733
"""
733-
try:
734-
offer = parse_offer_endpoint(endpoint)
735-
except OfferParseError as e:
736-
log.error(e.message)
737-
raise
738734

739-
if offer_name is None:
740-
offer_name = offer.application
735+
# If we have both the offer_name and the application_name
736+
# then we're coming from bundle/overlays, so no need to parse the endpoint
737+
# Also we accept endpoints without a colon (:) in the overlays
738+
if offer_name and application_name:
739+
o_name = offer_name
740+
a_name = application_name
741+
eps = {endpoint: endpoint}
742+
else:
743+
try:
744+
offer = parse_offer_endpoint(endpoint)
745+
except OfferParseError as e:
746+
log.error(e.message)
747+
raise
741748

742-
if application_name is None:
743-
application_name = offer.application
749+
o_name = offer_name if offer_name else offer.application
750+
a_name = application_name if application_name else offer.application
751+
eps = {name: name for name in offer.endpoints}
744752

745753
params = client.AddApplicationOffer()
746-
params.application_name = application_name
747-
params.endpoints = {name: name for name in offer.endpoints}
748-
params.offer_name = offer_name
754+
params.application_name = a_name
755+
params.endpoints = eps
756+
params.offer_name = o_name
749757
params.model_tag = tag.model(model_uuid)
750758

751759
facade = client.ApplicationOffersFacade.from_connection(self.connection())

juju/remoteapplication.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,11 @@ class ApplicationOffer(model.ModelEntity):
4444
@property
4545
def tag(self):
4646
return tag.application(self.name)
47+
48+
@property
49+
def offer_name(self):
50+
return self.safe_data['offer-name']
51+
52+
@property
53+
def application_name(self):
54+
return self.safe_data['application-name']
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
description: Another overlay to create an offer
2+
applications:
3+
grafana:
4+
offers:
5+
dashboards:
6+
endpoints:
7+
- dashboards

tests/integration/test_model.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,17 @@ async def test_deploy_bundle_with_multiple_overlays_with_include_files(event_loo
321321
bundle_yaml_path = TESTS_DIR / 'integration' / 'bundle' / 'bundle.yaml'
322322
overlay1_path = OVERLAYS_DIR / 'test-overlay2.yaml'
323323
overlay2_path = OVERLAYS_DIR / 'test-overlay3.yaml'
324+
overlay3_path = OVERLAYS_DIR / 'test-overlay4.yaml'
324325

325-
await model.deploy(str(bundle_yaml_path), overlays=[overlay1_path, overlay2_path])
326+
await model.deploy(str(bundle_yaml_path), overlays=[overlay1_path, overlay2_path, overlay3_path])
326327

327328
assert 'influxdb' not in model.applications
328329
assert 'test' not in model.applications
329330
assert 'memcached' not in model.applications
330331
assert 'grafana' in model.applications
332+
assert 'grafana' in model.application_offers
333+
assert 'grafana' == model.application_offers['grafana'].application_name
334+
assert 'dashboards' == model.application_offers['grafana'].offer_name
331335

332336

333337
@base.bootstrapped

0 commit comments

Comments
 (0)