cannot sequentially create multiple cross model relations from the same app

Bug #2048870 reported by Pietro Pasotti
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical Juju
Won't Fix
High
Nicolas Vinuesa

Bug Description

when you have multiple cross-model relations you need to first create all offers (and consume them), and only then create the relations with `juju relate`.

Creating an offer, relating it, and then creating another offer gives an error such as:
ERROR cannot update application offer "gagent": application endpoint "logging-consumer" has active consumers

Remark from Joe Phillips:
> The bug comes from [here](https://github.com/juju/juju/blob/fc069f19f894c4f7c777ed9a290cd64a706c24dd/state/applicationoffers.go#L679). I don't think the determination by set difference is correct.

Steps to reproduce:

deploy cos-lite in a kubernetes model

deploy in a different (machine?) controller any charm supporting a cos-agent subordinate such as https://github.com/canonical/intro-to-charming-mc/tree/part-3-observability

deploy grafana-agent and attach it to the charm
follow the steps in https://discourse.charmhub.io/t/using-the-grafana-agent-machine-charm/8896#step-4-relate-grafana-agent-to-cos-lite-prometheus-loki-and-grafana to create one of the CMRs and then another

--> bug.

Reproducible in a setup with:

juju snap version 3.1.7
juju machine model agent version 3.2.3
juju kubernetes model agent version 3.1.6

Ian Booth (wallyworld)
Changed in juju:
milestone: none → 3.3.2
importance: Undecided → High
status: New → Triaged
Changed in juju:
milestone: 3.3.2 → 3.3.4
Changed in juju:
assignee: nobody → Nicolas Vinuesa (nvinuesa)
Ian Booth (wallyworld)
Changed in juju:
milestone: 3.3.4 → 3.3.5
Harry Pidcock (hpidcock)
Changed in juju:
milestone: 3.3.5 → 3.3.6
Changed in juju:
milestone: 3.3.6 → 3.4.4
Revision history for this message
Nicolas Vinuesa (nvinuesa) wrote :

@ppasotti I might be misunderstanding your reproducer scenario, but aren't you essentially (I'm simplifying a lot):
```
juju offer app1:endpoint1
juju consume ...
juju integrate ...
juju offer app1:endpoint2
```
?

Because if that's the case, then the last operation is basically trying to update the offer (by default named) `app1` to include the endpoint2 and remove endpoint1, this fails because endpoint1 is related. https://juju.is/docs/juju/manage-offers#heading--create-an-offer

Please correct me if I'm wrong.

Revision history for this message
Pietro Pasotti (ppasotti) wrote :

that's essentially what I'm trying to do, yes.
I was expecting the semantics of `juju offer app1:endpoint2` to be: "add this offer" rather than "replace the previous offer with this one".

So that explains why it fails.
Still unclear to me is: how do I add an offer to an application that already has a bound endpoint?

Just like I can do `juju relate foo:a bar; juju relate foo:b baz`
I would expect to be able to do `juju cross-model-relate foo:a othermodel.bar; juju relate foo:b othermodel.baz`

Revision history for this message
Nicolas Vinuesa (nvinuesa) wrote :

Ok so at least that explains why it's failing and indeed what the code is currently doing is coherent with the docs.
That being said, I see your pain point and we could discuss changing the semantics for offers a little bit. Just for clarification, currently `juju offer app:endpoint` will *create* the offer for the application app, and *if* the "app" offer already exists, then it updates it so that the endpoints are the ones passed by the command line args (in this example only `endpoint`).

A quick workaround I'm thinking for you could be to add all the previous endpoints in an additive fashion:
1) juju offer app:endpoint1
2) juju offer app:endpoint1,endpoint2
3) juju offer app:endpoint1,endpoint2,endpoint3
...
(I agree this is not exactly the semantics you are looking for)

Revision history for this message
Nicolas Vinuesa (nvinuesa) wrote :

Maybe you'd like something like:
juju add-offer app:endpoint
juju update-offer app:endpoint(s)
?
This is more explicit than our current "upsert" implementation.

Revision history for this message
Pietro Pasotti (ppasotti) wrote :

Not sure I see the need for two separate commands for initializing an offer and extending it. How about:

`juju add-offer foo:x` -> if foo has no offers yet, create an offer on endpoint x. If foo already has offers, append 'x' to the list of offered endpoints

Also I'd rather have `juju offer` be called `juju set-offer`, to hint at the fact that you're expected to provide an exclusive list of endpoints to be activated, and the command will deactivate all that are not mentioned in the list.

We have a similar convention in ops, where `add/remove-port` offers the delta semantics, and `set-port` gives you the replace semantics.

Revision history for this message
Nicolas Vinuesa (nvinuesa) wrote :

As discussed with Pietro, this behavior is actually what's documented and the code is coherent with that.
Since the `offers` command behaves as an upsert and this might be misleading to some users, further discussions will happen around `offers` and (maybe) some new commands. If discussions advance a spec will be created for this.
For the moment, this bug closed in favor of those possible changes.

Changed in juju:
status: Triaged → Won't Fix
Revision history for this message
John A Meinel (jameinel) wrote :

There is one thing that we could do more clearly, which is if you did:

$ juju offer A:one
$ juju relate other/A B
$ juju offer A:two
A:one in use, did you want to add A:two: `juju offer A:one,two` ?

IOW, just improve our error message to help the user understand that what they asked for might not be what they thought it was.

Revision history for this message
Pietro Pasotti (ppasotti) wrote :

that sounds great too

Revision history for this message
Nicolas Vinuesa (nvinuesa) wrote :

Even though we said we won't change the behavior of `juju offer` for the moment, I landed this PR to improve the error messages at least:
https://github.com/juju/juju/pull/17497

Revision history for this message
Pietro Pasotti (ppasotti) wrote :

:100-points:

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.