-
Notifications
You must be signed in to change notification settings - Fork 123
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
JUJU_VERSION not set during metrics hooks #372
Comments
https://bugs.launchpad.net/juju/+bug/1891337 is about fixing this in Juju |
I think doing #365 will address this, won't it? |
My concern is that Install will see a JUJU_VERSION of 2.8 above a threshold
but then collect-metrics runs and sees a version of 0.0 which ends up below
a threshold and confuses the charm.
…On Wed, Aug 12, 2020, 18:52 chipaca ***@***.***> wrote:
I think doing #365 <#365>
will address this, won't it?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#372 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABRQ7LYVQWJQRORYJJV4OTSAKUDHANCNFSM4P4YVI5A>
.
|
Should this still be open? #365 was merged already. |
I would certainly want us to test whether it works the way we want. The
issue is that you might see Juju 2.8+ and go with server-side storage, but
then a metrics hook fires without JUJU_VERSION and you think 'oh, I must
need local storage because my version is 0.0.0' and then the next hook that
fires sees that there is local storage, and ignores all of the state that
it had previously recorded in server-side storage.
…On Wed, Aug 26, 2020 at 9:32 AM Dylan Stephano-Shachter < ***@***.***> wrote:
Should this still be open? #365
<#365> was merged already.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#372 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABRQ7O7G33HLNIRZCQ2U6LSCUMGFANCNFSM4P4YVI5A>
.
|
You could fall back to querying the machine agent, like charmhelpers does. |
Can close this for now -- now always running "new Juju" (2.8 or whatever it is). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As seen by @camille-rodriguez :
During 'meter-status-changed'
https://paste.ubuntu.com/p/WZfMRkvgyn/
Metrics hooks run in a Limited context which seems to define its own set of vars:
https://github.com/juju/juju/blob/develop/worker/uniter/runner/context/context.go#L946
vs
https://github.com/juju/juju/blob/develop/worker/meterstatus/context.go#L44
Ideally this also gets fixed in Juju, but in the short term, we don't want to break because of a meter-status-changed or a collect-metrics hook getting triggered.
The text was updated successfully, but these errors were encountered: