Add the ability to populate State from metadata

[tonyandrewmeyer]

The State could have some default components based on the Context metadata, or perhaps there could be a fill_from_context method or similar.

For example, this could add:

• A Container for each container in the metadata (although you'd probably want to be able to specify whether can_connect should be True or False, or it seems like there might be a lot of awkward dataclasses.replace afterwards)
• config that's the same as the defaults in the metadata
• A default Network (there's essentially always a Juju network, right?)
• A Storage for each storage in the metadata (to the minimum side of the multiple value if provided)
• A StoredState for each ops.StoredState attribute on the Context's charm class (not perfect since these can exist elsewhere, particularly in libs, but maybe a useful base?)

I don't think it could have relations (even if we do make it possible to have data-inaccessible relations, e.g. #1452) since there's a practical difference between a relation existing (from the point of view of the unit) and not existing, wider than just the data being available.

I don't think it could include resources, even though they're in the metadata and always (?) available, because it doesn't seem like there's any way to have a sensible default location for the content.

See this Matrix discussion for more background.

[benhoyt]

If this does make it significantly simpler for test authors it's worth considering. Going to add it to our tentative 25.10 roadmap.

[dimaqq]

Matrix copy-paste

Wed, Nov 13, 2024
Leon
Hi Team (and
Pietro Pasotti
)
Would you be able to help with a scenario test for grafana/354?

Currently I have this:

def test_reporting_disabled(ctx):
    # GIVEN the "reporting_enabled" config option is set to False
    state = State(leader=True, config={"reporting_enabled": False})
    # WHEN config-changed fires
    out = ctx.run(ctx.on.config_changed(), state)

    # THEN the config file is written WITH the [analytics] section being rendered
    simulated_pebble_filesystem = out.get_container("grafana").get_filesystem(ctx)
    grafana_config_path = simulated_pebble_filesystem / "etc/grafana/grafana-config.ini"

    config = ConfigParser()
    config.read(grafana_config_path)
    assert dict(config["analytics"]) == {
        'reporting_enabled': 'false',
        'check_for_updates': 'false',
        'check_for_plugin_updates': 'false',
    }

The error I'm getting is:

  File "/home/ubuntu/grafana-k8s-operator/.tox/scenario/lib/python3.10/site-packages/scenario/runtime.py", line 473, in exec
    raise UncaughtCharmError(
scenario.errors.UncaughtCharmError: Uncaught exception (<class 'RuntimeError'>) in operator/charm code: RuntimeError("container with name='grafana' not found. Did you forget a Container, or is the socket path '/charm/containers/grafana/pebble.socket' wrong?")

Where can I find an example for using a container?
Shouldn't it picked up automatically if not specified?
Ben Hoyt
Tony Meyer
^
Leon
Got it:

    state = State(
        leader=True,
        config={"reporting_enabled": False},
        containers=containers
    )

and:

containers = [Container(name="grafana", can_connect=True), Container(name="litestream", can_connect=True)]

Would it make sense:

auto populate "containers" from metadata, if not given
set can_connect to True by default
?
Also, is there a simpler way of getting a file out of the simulated pebble filesystem?
Tony Meyer
There could perhaps be helpers or an option to populate the state with the containers, relations, storage, etc that's in the metadata, with some sort of default content. We've preferred being explicit so far.

I can see an argument that the charm always has the containers in the metadata, and that they're just can_connect==False until they are ready (I don't think a charm should be able to tell the difference between "container has not been created by K8s" and "Pebble isn't available"). So maybe that's different than non-peer relations where the relation might not actually exist even though it's defined.

I guess I would say open a feature ticket and we'll consider it. Edit: I opened one.

For can_connect defaulting to True, we're stuck with the default until v8 and would really like to stick to the promise that the breaking v7 changes would be it for a good amount of time.

Would True be a better default? It's maybe the more common result in tests? I see a lot of tests around the container becoming available or handling error states where it's more 50/50, but then there are some where the container isn't the focus of the test but needs to be there, and for those True is more common. It'll be easier to judge this when more tests exist across repos.

I don't remember considering this when we were doing the v7 API review, and it was False in earlier versions too.
Pietro Pasotti
might have more context around why it's the default.

Tony Meyer
Unable to load event that was replied to, it either does not exist or you do not have permission to view it.
If you set up Mounts in your Container then you'll have access to the source you provided and that's probably simpler. For example, you could wrap the run inside a TemporaryDirectory context block and use that as the source for a Mount at /etc/grafana/ and then get the file directly from that temporary directory.

You could also use the Context as a context manager and then do something like:

mgr.charm.unit.get_container('grafana').pull(...)

I'm not sure whether you'd consider that simpler or not.

I think doing it the way you are is fine, though. If you find that you're having to write really similar code all the time, then let us know, because we could have more helpers, like a get_file that shortcuts things, if there's a clear need for them.

Pietro Pasotti
All that Tony said.
RE the reason why the default of all State objects is to be as barren as possible, that was an explicit design choice. The reason for it is that I wanted to catch first and foremost the 'unhappy paths', ones where the container isn't there, the relations aren't there, etc...
So that as you fill in the missing pieces the charm needs in order to work, you learn how things work.
Also, where does the 'defaulting' stop? Would we attempt to guess the contents of databags, if we have some schemas lying around?
In general I think I tried to write scenario to be as unsmart as possible when it comes to assuming the content of the mocks. It's easy enough to write a little helper function to populate a state with some more charm-specific defaults.