[Bug]: Pass a 'series' kwarg to Model.deploy() error when series is specified

[arturo-seijas]

Description

Hi,
All of our iur integration tests are failing in CI with the following error although the series parameter is specified when deploying.

juju.errors.JujuError: Couldn't determine series for charm at /home/runner/work/indico-operator/indico-operator/.tox/integration/tmp/pytest/testing0/charms/indico_ubuntu-20.04-amd64.charm. Pass a 'series' kwarg to Model.deploy().

If we pin the version to 3.0.2 the error isn't raised.
It look like this issue was introduced in this PR.

Also, it is my understanding that, given I'm deploying kubernetes charms, the series should be irrelevant.

Thank you

Urgency

Annoying bug in our test suite

Python-libjuju version

3.0.3

Juju version

2.9.35

Reproduce / Test

# Deploy relations first to speed up overall execution
    hello_kubecon_app_name = "hello-kubecon"
    await ops_test.model.deploy(hello_kubecon_app_name)

    # Build and deploy ingress
    charm = await ops_test.build_charm(".")
    application = await ops_test.model.deploy(
        charm, application_name=app_name, series="focal", trust=True
    )
    await ops_test.model.wait_for_idle()

[mthaddon]

Specifically, we used to be doing this:

series = res.origin.series or series

But we're now doing this:

series = res.origin.series if self.connection().is_using_old_client \
            else series

These two aren't equivalent if you're using the old client, it's no longer falling back to series.

[juanmanuel-tirado]

I have checked the value of series using both 2.9 and 3.0 controllers and the value matches the input argument when using 2.9 or 3.0 controllers. The final call to the Juju API client uses the given input argument. I'm checking what's going on there.

[jameinel]

For Juju 3.0, I would expect that you should be passing a Base not a Series. Likely we should move the Lib to Base, and have a compatibility where if you supply one or the other we translate in the short term. (eg, we can have pylibjuju 3.0 only take a Base, but translate it to a series for juju 2.9) John =:->

…

On Tue, Oct 25, 2022 at 10:34 AM Juan M. Tirado ***@***.***> wrote: I have checked the value of series using both 2.9 and 3.0 controllers and the value matches the input argument when using 2.9 or 3.0 controllers. The final call to the Juju API client uses the given input argument. I'm checking what's going on there. — Reply to this email directly, view it on GitHub <#758 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABRQ7IEV7EPE36CADXCIMDWE6LSNANCNFSM6AAAAAARNVSOKI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[juanmanuel-tirado]

There is something else going on there because this has skipped our CI checks.

[jnsgruk]

Related to #759

[mthaddon]

I have checked the value of series using both 2.9 and 3.0 controllers and the value matches the input argument when using 2.9 or 3.0 controllers. The final call to the Juju API client uses the given input argument. I'm checking what's going on there.

Have you tried this with a local charm? This is what's failing for us specifically on juju 2.9 with python-libjuju 3.0.3.

[juanmanuel-tirado]

There is an ongoing problem with the Base object definition. We are unfortunately taking the former 2.9 client version instead of the new 3.0.3. I could have a correct deployment by forcing this manually.

[cderici]

There is an ongoing problem with the Base object definition. We are unfortunately taking the former 2.9 client version instead of the new 3.0.3. I could have a correct deployment by forcing this manually.

This is incorrect. I just double checked and confirmed that the pylibjuju indeed gets the correct object from correct client modules for both 2.9.35, 2.9.36 and 3.0.0.

@jameinel For Juju 3.0, we do indeed pass a Base to the Juju API. For local ones, that's constructed here, and for charmhub ones it comes from the Charms facade baked into the CharmOrigin (here). Both are passed to the ApplicationFacade.Deploy as charm_origin.

The confusion is about the series parameter itself. So Juju 2.9.35 uses ApplicationFacade v14 and Juju >=2.9.36 use ApplicationFacade v15. Deploy in v14 accepts the series parameter as well as taking the base --inside the CharmOrigin--. Deploy in v15 doesn't accept the series parameter and only takes the base. What pylibjuju needs to do is to stop passing the series parameter entirely, as @jnsgruk suggested in #759, however, that would break compatibility with the earlier AppFacade versions that do expect the series (and maybe even not expect the base).

To take the base ([track/risk] + OS) as an argument from the user is a libjuju design decision that we can have a discussion with @juanmanuel-tirado and @jameinel.

For this particular issue, @arturo-seijas is correct. I do seem to have introduced a logic that ends up ignoring the series argument when it's passed. Fix is coming, I'm on it 👍