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

Expose goal-state #434

Closed
stub42 opened this issue Oct 27, 2020 · 4 comments
Closed

Expose goal-state #434

stub42 opened this issue Oct 27, 2020 · 4 comments

Comments

@stub42
Copy link
Contributor

stub42 commented Oct 27, 2020

The output of the goal-state Juju hook environment tool is not exposed by the Operator Framework, requiring charms to wrap it themselves and mock it in tests. The Operator Framework should provide access to it.

Known use cases are #206, and my charm which needs to know the unit names of its expected peers. But rather than trying to be prescient and second-guessing all the possible use cases, we could just expose the data that Juju exposes. The test harness could just return empty data by default, and provide a method to populate it (so goal-state in tests by default would look like the environment is being destroyed, which is as good a default as any and the easiest to implement).

@chipaca
Copy link
Contributor

chipaca commented Nov 12, 2020

If we had a way to get the number of pending units, and you just had to wait for that to be zero to get the names, would that be enough?

@facundobatista
Copy link
Contributor

Hey @stub42!

We had yet another conversation about this topic. We will not be providing access to Juju's goal-state, mainly because probably juju will drift away from providing such an information, in favor on the more consistent information of "pending units" (which is better for this kind of distributed systems).

We're discussing in #165 if having "pending units" is enough or not (and in which cases it is not, and how to deal with that).

So, if this issue is enough covered there, we may close it and concentrate the discussion.

Thanks!

@stub42
Copy link
Contributor Author

stub42 commented Nov 16, 2020

For the postgresql-k8s charm use case, a 'pending units' count would be enough. But certainly more awkward and less developer friendly, since I now need to implement and test a loop spanning multiple hooks to get the same information that goal-state already provides. And as @jameinel points out on a related bug, the logic can hang if a unit is removed at the wrong time, requiring hooking into update-charm (and a ~5 minute delay) specifically to catch the situation (if you go ahead with 'pending units' count, the Operator Framework should fire an event when it detects a change, forward compatible with whatever hook Juju eventually provides to maintain the count).

The other use case I have is exposed by charmhelpers.is_doomed(). Sometimes, a unit needs to know it is being torn down. This occurs in relation-departed hooks, where a node needs to know if it is the local unit departing the cluster or if it is the remote unit. Maybe this is now exposed elsewhere by Juju. Also occurs in Juju 2.7 and earlier, where a lead unit needs to be smart enough to not do anything stupid if it is being torn down and no longer has a complete view of the model (per https://bugs.launchpad.net/bugs/1469731)

It seems a lot of effort for no gain. Juju drifting away from providing information that was explicitly exposed because charms needed it and charm authors requested it makes Juju less useful. 'goal-state' provides a view at what might be, and is useful despite the fact that a goal is not a guarantee or because charms are not notified when the goal posts move.

@jnsgruk
Copy link
Member

jnsgruk commented Sep 28, 2021

Mostly fixed in #597 with caveats, please see that PR for details!

@jnsgruk jnsgruk closed this as completed Sep 28, 2021
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

No branches or pull requests

4 participants