Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ingress support #117

Merged
merged 9 commits into from
Sep 16, 2022
Merged

Add ingress support #117

merged 9 commits into from
Sep 16, 2022

Conversation

rbarry82
Copy link
Contributor

Neither ingress-per-unit nor ingress-per-app do precisely what
Grafana needs in order to keep sqlite in a reasonable state, but
the TraefikRoute libraries, as long as we ignore the work done by
the actual traefik-route-k8s charm entirely, provide an interface
via which we can send "raw" Traefik config snippets to ingress.

web-external-url was already an available config option used by
the load tests, so it is reasonably exercised and tested already,
and matches the existing expected semantics for ingress on other
charms.

Issue

Ingress!

Solution

If a relation is established to Traefik without web-external-url
being set, the Grafana leader will configure Traefik with a route
which points to {model}-{app.name} publicly, which will use the
{app}-{unit_num}-{endpoints} binding Juju gives us in order to
forward external traffic to the leader.

If web-external-url is set, use that, with the same internal
endpoints as above.

On leadership election, rewrite the route so it points at the
correct leader. Similarly, if a config change sets web-external-url,
use that.

Since TraefikRouteRequirer has a non-optional Relation as part
of the constructor, also configure ingress if a relation is
established during the middle of a charm lifecycle after setting
a private variable in the library.

Testing Instructions

Relate to traefik, make sure it's accessible.

Release Notes

Add ingress support to Grafana

Neither ingress-per-unit nor ingress-per-app do precisely what
Grafana needs in order to keep sqlite in a reasonable state, but
the TraefikRoute libraries, as long as we ignore the work done by
the actual `traefik-route-k8s` charm entirely, provide an interface
via which we can send "raw" Traefik config snippets to ingress.

`web-external-url` was already an available config option used by
the load tests, so it is reasonably exercised and tested already,
and matches the existing expected semantics for ingress on other
charms.

If a relation is established to Traefik without `web-external-url`
being set, the Grafana leader will configure Traefik with a route
which points to `{model}-{app.name}` publicly, which will use the
`{app}-{unit_num}-{endpoints}` binding Juju gives us in order to
forward external traffic to the leader.

If `web-external-url` is set, use that, with the same internal
endpoints as above.

On leadership election, rewrite the route so it points at the
correct leader. Similarly, if a config change sets `web-external-url`,
use that.

Since `TraefikRouteRequirer` has a non-optional `Relation` as part
of the constructor, _also_ configure ingress if a relation is
established during the middle of a charm lifecycle after setting
a private variable in the library.
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
tests/unit/test_charm.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
Copy link
Contributor

@sed-i sed-i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces a few crutches that should ideally be streamlined.
It's not pleasant to look at, but not blocking either.

src/charm.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
tests/unit/test_charm.py Show resolved Hide resolved
@rbarry82
Copy link
Contributor Author

rbarry82 commented Sep 1, 2022

I still don't get it.
We are under if self.ingress.is_ready() so shouldn't we return the ingress address instead of fqdn?

Not here. For traefik_route, ingress.is_ready() is actually self.ingress._relation is not None. It does not know or have an ingress address. It accepts a raw config, nothing more or less.

For the specific Grafana use case, it's inverted, and Grafana "knows" the ingress address as either web_external_url or generate something which looks like what ingress_per_app would provide, except that Grafana itself must generate the specific contorted configuration to make "ingress per app" actually route to a specific unit, but only one and not the others, and that unit must be the leader, which is also the sqlite replication primary. It's a very, very targeted use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants