[09:09] morning [10:32] hola bthomas [10:33] ¡Muy buenos días a todos! [10:33] facubatista: 👋❗ [10:33] hola Chipaca :) [10:34] नमस्ते facubatista : looks like you beat me to saying hola first today :-). So I am going back to saying नमस्ते [10:34] Mui bein [10:34] bthomas, yes, I read *your* "morning" and reacted to that :) [10:35] :-) [10:57] facubatista: did you forget to push something to charmcraft#180? not seeing the changes you talk about there [10:57] PR charmcraft#180: Support to create a library, first step in all the lifecycle [11:00] Chipaca, I didn't finish my "reaction to the review" [11:00] (this is a github problem, in launchpad I send the responses to the comments all at the end) [11:01] facubatista: ␇ shame ␇ [11:35] facubatista: have you seen simple-term-menu? [11:36] Chipaca, nop [11:44] Chipaca, there I pushed my changes as per your review, note that I answered one of the items with an open question [11:44] Chipaca, the '--lib-name' one [11:59] Chipaca, so I'm happy to land this, but let's talk before about --lib-name [11:59] facubatista: ok. Why --lib-name and --charm-name for things that can only take one and not the other? is there a case of a command that might take both? [12:00] facubatista: note how --charm-file is now being a bother when we want to use it for a bundle or a charm, for example [12:02] Chipaca, no case for a command taking both [12:03] Chipaca, so both 'create-lib' and 'list-lib' can get a '--name' (the first one meaning a lib name, the second one meaning a charm name) [12:04] facubatista: are they optional? [12:05] yes [12:05] k [12:05] sorry [12:05] facubatista: what would the help lok like [12:05] list-lib is [12:06] ah [12:06] facubatista: if it's not optional it shouldn't be an option (and you know this :) ) [12:06] create-lib has a mandatory "name" parameter, the help is "The name of the library file (e.g. 'db')." [12:06] ok [12:06] right [12:06] yes, that's already so [12:06] good [12:07] list-lib has an optional "--charm-name", the help is "The name of the charm." [12:07] facubatista: now, about list-lib, is --charm-name the right thing? [12:07] the right name for the option i mean [12:07] for create-lib's argument, 'name' makes the most sense, as what else would it be [12:08] ack, changing it for create-lib [12:08] facubatista: this is my opinion, not an order :-p [12:08] pad everything with IMHOs [12:09] facubatista: i could also argue against myself 🙂 [12:09] but i'd rather if you did [12:09] for list-lib is less clear [12:09] facubatista: for list-lib, --charm-name seems … i dunno, overly verbose? [12:09] OTOH I can see the argument of --name being ambiguous [12:09] but why not --charm ? [12:11] Chipaca, I like --charm [12:11] "get me all the libraries for this charm" [12:11] s/get/list/ [12:12] Good morning! | Buenos días! | शुभ प्रभात! [12:14] hola JoseMasson :) [12:15] facubatista: OTOH [12:15] facubatista: what is the label of the table that list-lib produces under which you put the charm's name? [12:15] facubatista: the option should match that label [12:15] facubatista: probably(tm) [12:16] s/table/column of the table/ [12:17] Chipaca, I don't put the charm name in the table (there's no point, all rows would just repeat that) [12:18] facubatista: but doesn't list-lib list all libs across charms? [12:26] facubatista: looking at the draft i see it's only ever for one charm [12:26] Chipaca, no, list-lib list all libs for a charm [12:26] right [12:26] facubatista: but i'm a bit confused because if i do create-lib, the lib i created is not in list-lib [12:27] Chipaca, yes it is, the lib created is for your charm [12:28] list-lib by default lists all libs for your charm, so [12:28] facubatista: i get 'Noting found.' [12:28] mmm... well, did you *publish* it? [12:29] no [12:29] anyway, I need to run or I won't get to the bank [12:29] run run [12:29] we can talk later about this, thanks! [12:29] i need to have lunch soon or i won't have time for it [12:43] PR operator#446 opened: Minor clarification for deferred event doc string [12:43] ^ minor doc string update [12:55] bthomas: s/being/begin/ [12:56] bthomas: but [12:56] bthomas: that last statement probably confuses more than it helps [12:56] bthomas: if you're not already thinking it's some sort of magic continuation, it'll throw you for a loop [13:00] Chipaca: Maybe it is just me but I did have to think about if event.defer() continued from the same point or not. I can remove the second statement but if you have a alternative phrasing I would be interested to know. [13:02] bthomas: I think it's worthwhile rephrasing the whole thing to make it crystal clear what it does (and what it does not). Let me think about it for a bit. [13:02] Awesome. Just edit the github PR when you do makeup your mind. [13:51] * bthomas -> lunch [14:08] * facubatista is back [14:21] * bthomas too [14:22] hey all! [14:23] DominikF and I currently working on a charm where pip ends up pulling in binaries (specifically via pynacl) into the charms venv which makes the charm architecture specific. Is their a recommendation on how to deal with this problem? In the end charms should be architecture-independent, right? [14:26] no, charms will be arch-dependent [14:26] that's fine [14:26] (well, not everything is in place to make them work 'right' all the time, but if your world is all amd64 it'll be fine until we get everything in place) [14:27] morphis: ^ [14:36] Chipaca: hm, so the store will be able to distribute different revisions depending on the architecture same as we have for snaps? [14:36] morphis: yup [14:36] ah ok [14:36] Chipaca: how much of this is already in place? [14:37] morphis: the store is multi-arch-aware but nothing else is afaik [14:37] morphis: for now everything is assumed to be arch: all [14:37] so we have to workaround with shipping two different venv's until that is fixed [14:38] morphis: you're wanting to support multiple arches? [14:38] yes, we have to support both arm64 and x86 with the same charm [14:39] bummer [14:39] but, yes [14:39] it wouldn't be too much of an issue just complicates the build process a bit [14:40] morphis: is this for running on juju >=1.8? [14:40] oh yeah, assume the latest juju [14:40] morphis: ok [14:40] facubatista: ^^^ please read from morphis' "hey all" down [14:41] Chipaca, facubatista: happy to sync on a call on this [14:49] would, love to be included in that call [14:49] * Chipaca bans DominikF [14:49] * Chipaca hides [14:49] 🙂 [14:51] 😂 [14:51] reading [14:54] morphis, what you could do (as a hack, until all the rest is in place and fully automatic (*)), is to `pip install your-dep` on the "install" hook, and postpone the import of that after that [14:54] facubatista: i think doing the multi-venv is better [14:54] otherwise your charm suddenly needs The Internet [14:55] and set up gets slower, and all the bad things we know about [14:55] yes, it needs the internet + gcc, ... [14:55] exactly [14:55] and libnacl is scary [14:55] deps-wise [14:55] (iirc) [14:55] (*) fully automatic: in the future you'll declare what archs you need, and "charmcraft pack" will pack your charm creating with the libs installed under those archs, having N results, which you could publish in the store for each arch, and the consumer will just get what it needs [14:56] facubatista: does the store support arch: many for charms? [14:56] how would charmcraft deal with the cross compilations aspects? [14:56] multi? [14:56] arch: [amd64, i386] [14:56] morphis, similar to how snapacraft does it today [14:56] probably using multipass, if you do it in your machine [14:56] so you either offload to a remote arm machine or you run on arm [14:57] morphis: launchpad builders or equivalent, yes' [14:57] ok [14:57] morphis, and eventually we would have launchpad builders [14:57] that [14:57] that's in the fullness of time, which is a way aways 🙂 [14:57] hehe [14:57] ok, I think for now we will manually perform what charmcraft is doing to generate the multi venvs [14:58] facubatista: for now, if you have a way of doing a 'charmcraft build' on x86 and amd64, you can then bring them together [14:58] morphis: ^ [14:58] Chipaca, how? [14:58] morphis: and if it's only for new jujus, nuke hooks/* and put the arch thing in dispatch [14:58] morphis: note that it's called venv, but it's not a venv, just a lib [14:59] should be straightforward to rename it and pivot in the dispatch (which is a shell script) [14:59] anyway, i'm off (more meetings) [14:59] Chipaca, ah, got it [14:59] morphis, do you understand what Chipaca is proposing? else I can help you [14:59] Chipaca: yes, we can easily do a uname -m check in the dispatch script and switch the venv/lib there [14:59] facubatista: yes I do, will ping you if we run into any problems [14:59] morphis, wonderful [15:00] thanks facubatista & Chipaca ! [15:00] :) [15:04] Hello everyone! [15:08] Hello ionutbalutoiu! [15:51] ionutbalutoiu: 👋 [16:05] facubatista: in charms.foo.v0.bar, is v0 the API version of bar? [16:06] Chipaca, yes [16:06] hello ionutbalutoiu [16:07] facubatista: new_library.py.j2 has a syntax error unless I'm misunderstanding something [16:07] facubatista: is that on purpose? [16:08] … and you merged it, o no :) [16:09] facubatista: you're missing tests, obviously, because otherwise you would've caught the error [16:09] facubatista: see the tests for 'init', maybe? [16:09] or _something_ :) [16:24] Chipaca, will add a test for it be a valid Python file [17:07] Chipaca facubatista between 6pm and 7pm UK time I will be going out to pickup dinner. But should I have any question after that will at least one of you be around ? [17:10] bthomas: I'm going to pop out now, go for a run, and be back at about 19:30 [17:10] thanks [17:10] bthomas: ie in 2 hours and a bit [17:10] no problem [17:10] (time to get changed + 2 hour run + shower) [17:10] nice [17:24] Prometheus altermanager changes have quite a few merge conflicts. I am working through them now. [17:24] bthomas, it's better to always talk in UTC-0, because of DST and everything I don't know that is 6pm uk time [17:24] facubatista: agreed. I think I am UTC+1 zone. [17:25] I'll be off between 19:00z and 21:30z [17:39] I just fixed the merge conflicts for the prometheus alertmanager changes from dylan. I will review and approve so I can merge after a short tea break. I will need a second review and approval to be able to merge this pull request https://github.com/canonical/prometheus-operator/pull/5 [17:39] PR prometheus-operator#5: Alertmanager support [17:39] * bthomas goes to make some tea [17:54] * bthomas is back [18:09] I am off to get some groceries and dinner. Will be back in an hour. [18:09] I need one more approval on ^ pull request. Will merge when back. [18:09] * bthomas goes [18:31] reviews appreciated! https://github.com/canonical/charmcraft/pull/181 [18:31] PR charmcraft#181: Support for publishing libraries [18:59] * facubatista bbl [19:46] * bthomas is back [19:46] Five guys are also under lockdown. This is depressing. :( [19:55] * Chipaca is _almost_ back [20:11] Chipaca: All you have to do is click OK :-) https://github.com/canonical/prometheus-operator/pull/5 [20:11] PR prometheus-operator#5: Alertmanager support [20:11] bthomas, just approved [20:12] Ah thanks justinclark [20:12] wait I still see merging is blocked [20:13] Never mind I just did the second approve. :-) [20:13] bthomas: no README? [20:14] wait, wrong project [20:15] bthomas: how's the alertmanager README etc coming? [20:16] Chipaca: I will create another pull request with README for alertmanager-operator [20:16] Cleaning commings to merge alertmanager changes to prometheus operator [20:16] ok,fair [20:17] bthomas: do you need me to review https://github.com/canonical/alertmanager-operator/pull/1 ? [20:17] PR alertmanager-operator#1: MVP for alertmanager charm [20:19] Chipaca: Any input you can give on alertmanager-operator PR will be helpfull. I have already reviewed. I can approve too due to paucity of time. If you want to make any suggested edits can I review and merge. I will then create another PR with readme. [20:23] Merged changes into prometheus. Building prometheus to check nothing broke [20:27] bthomas: reviewed the https://github.com/canonical/alertmanager-operator/pull/1, +1 with notes [20:27] PR alertmanager-operator#1: MVP for alertmanager charm [20:27] * Chipaca digs into his guiso de lentejas [20:28] (a spanish dhal) [20:31] Chipaca: Thank you. justinclark Chipaca One more quick fix. With this fix prometheus runs and works ok. Please see commit message explaining need for fix. https://github.com/canonical/prometheus-operator/pull/12 [20:31] PR prometheus-operator#12: Fix incorrect label for stored state [20:32] Going to merge alertmanager-operator build and test now. If it works and can form relations with prometheus I will create another pull request with README for alertmanager operator [20:39] bthomas, just reviewed [20:39] thanks justinclark [20:39] Need to make the same change in the tests [20:40] just deployed alertmanager and prometheus and related them. No errors. Trying to figure out if the relation actually worked. [20:40] justinclark: thanks will edit and push changes to test in same commit . You may need to reapprove if you do that. Alternatively if you just make and edit suggestions I can accept and it will work, without the need for reapproval. [20:42] Chipaca: from juju debug logs I can see that when I added alertmanager and prometheus relation. The relation changed event triggered for alertmanager and there were no errors. Is this good enough ? [20:51] jldev: ^ i think that is what you meant? [20:51] unless he meant in the alertmanager logs itself [20:54] bthomas, I'm fixing tests now and will push to that PR [20:55] jldev: Chipaca just looking at logs. There are no errors but no information regarding alertmanager joining. [20:55] thanks justinclark [20:56] * bthomas googles how to check alertmanager prometheus interaction [20:56] If it's alright with you, I'm also going to change grafana-dashsource to grafana-datasource if you don't think it'll muddy the PR too much :) [20:56] bthomas ^ [20:57] justinclark: I have no preference either way. Check if jldev has any preference. [20:57] justinclark: please keep commits separate . Not great to be missing alertmanager commits with grafana commits [20:59] Will do. That is the name we agreed on, but didn't know if I should sneak the commit into this PR or not. [21:00] jldev: Chipaca Also I can confirm that alertmanager unit gets added as a target to the prometheus configuration file /etc/prometheus/prometheus.yml . So this may be further indication that the relation worked. [21:00] I am not sure what else to check. [21:01] justinclark: I am done testing alertmanager and prometheus relation for now so will start on README, if you were planning to fix tests. If not I will fix tests. Let me know. [21:02] I will fix tests. Feel free to focus on the README [21:02] awesome thanks [21:02] justinclark: you are an angel :-) [21:30] :) [21:30] Readme is baking, will be ready soon [21:32] It looks like one issue may have to do with the harness. When we update the app relation data rather than unit relation data, the event.unit is None in the *-relation-changed event. Is this the expected behavior? [21:32] justinclark: I think it is. Chipaca facubatista may be better informed here. [21:32] yes that is expected behaviour [21:33] Okay cool. Then the tests just need to be rewritten. [21:33] event.unit is None for app data changes [21:33] * facubatista is back [21:38] * Chipaca goes for a cuppa (and some digestives why not) [21:43] Chipaca: when you are back https://github.com/canonical/alertmanager-operator/pull/2 [21:43] PR alertmanager-operator#2: Updated readme [21:44] * Chipaca is back [21:44] justinclark: I am done with readme. Do you need help with tests ? [21:44] bthomas, just finishing up. [21:44] awesome [21:44] bthomas: ooh, typo [21:45] I am going to hammer another coffin name while Chipaca rips my README to shreds :-) [21:45] * bthomas goes for a puff [21:46] There is definitely still some logic that doesn't make sense given the alertmanager changes. [21:47] bthomas: 1st suggestion, to make it easier to write markdown text with lots of links, embrace the 'reference' style using [21:48] ah, if you're gone, i'll make the suggestions in code [21:53] * bthomas is back [21:53] * bthomas picks up the pieces of the README :-) [21:53] * bthomas googles reference style in Markdown [21:54] * bthomas reads https://www.simondcoll.com/references-markdown-zotero/ [21:57] bthomas: ah, just pushed my +1 with suggestions [21:57] Thanks Chipaca [22:01] The alertmanager charm is setting the relation data "addrs" to be a list, and prometheus still assumes it's a dictionary in _alertmanager_relation_departed. [22:02] justinclark: that should be in the test only. Though there were merge conflicts like that, they should have been resolved for the charm. [22:02] I am merging readme now [22:04] I see - the relation-departed is still in the charm when it should have been replaced with relation broken [22:05] justinclark: My understanding was relation departed removes on particular alertmanager but relation broken clears the entire list. [22:07] That's not how it would work in the current iteration. addrs is being stored in _stored.alertmanagers which is a list [22:07] relation-departed is popping as if it were still a dictionary [22:09] justinclark: you are right . That merging was not right. The correct thing to do is search for event.unit.name in the list and remove it if found. [22:10] I see. I'll add a quick fix. [22:13] I'm actually not sure why we need departed. [22:13] justinclark: looks like that may not have been a merge issue. It is a syntax/semantics issue. List pop method requires an index not name. [22:14] justinclark: Departed is when one unit goes out of the relation. When they relation itself is terminated then it is said to be broken. [22:14] At least that is how I understand it. [22:16] How would we know which unit to remove? [22:17] Alertmanager is sending the k8s hostnames, but those can be shuffled around. I'm not sure we'd be able to identify which hostname should be removed just based on the relation information [22:17] From my reading of Dylans code I understand he was saving the alertmanager hostnames names in a list. [22:17] When an alertmanager relation departs. I think you get the unit name as part of the event [22:18] e.g. alertmanager/2 could be tied to alertmanager-1.xxx.endpoints [22:18] Maybe I'm overcomplicating it in my head [22:18] hang on, I am double checking [22:20] justinclark: If you look at alertmanager charm.py [22:20] he is saving the alertmanager unit hostname in the application relation data [22:21] so prometheus should be storing them in its stored state list [22:22] Seems like there is a need to translate between unit name and host name [22:22] * bthomas digs [22:22] bthomas, justinclark, i'm still awake but going to go read stuff and slowly crash; mention me if you need me and i'll pop by [22:23] Thanks Chipaca [22:23] of course my internet cuts off in 40 minutes to help me sleep so after that you're on your own 🙂 [22:23] Chipaca: we will try not to disturb you unless the sky begins to fall. Good night. [22:23] (bah, if the world is on fire, you can @ me on mattermost -- my phone still works) [22:24] bthomas, I know it's late for you too. Don't feel the need to stay up. I was just a bit perplexed by the logic. [22:25] justinclark: looks like the logic of removing alertmanager units from the prometheus unit was not completely implemented. Little wonder that pull request on prometheus repository was still in a draft stage. I actually convert it to a pull request so I could merge and test. [22:25] justinclark: it may be faster if we pair on this. I can hang on for another hour or so without falling off my chair. Your call ? [22:26] If you like I can get on to hangouts. [22:27] Maybe we wait for Dylan in the morning? He may already have thoughts about this since he's been working on it. [22:28] justinclark: Do you know when this should be done by ? I know for sure before next tuesday. Is jldev around ? [22:29] I know he asked that Graylog be done by tomorrow. I would suspect the same is true for alertmanager/prometheus [22:30] I am messaging him on mm now. [22:35] waiting for response from jldev [22:36] ack [22:39] I have dinner plans with the wife soon. I think it might be best to wait until tomorrow. I'm sure we can have it done before standup if we pair and bring Dylan into the convo [22:40] justinclark: OK. Looks like jldev is away and I am getting weary. [22:40] sleepy programmers are not good programmers [22:40] I can also get back to it after dinner if it's absolutely critical that it's done today. [22:40] Haha have a good rest of the night bthomas. [22:41] justinclark: your call. If jldev is around catchup with him. It all depends on what kind of a demo we are trying to do. On the prometheus repository there is still a branch for monitoring k8s that can feed in data to prometheus which can be related to grafana and you can get some pretty graphs. [22:42] Last thing I will try and do now is rebase that monitoring k8s branch on latest master to keep it updated. [22:43] This will help since it provides a means to feed data into prometheus for demo purposes. Since we still need to add an alerting rules and scrapping rules relation to prometheus. [22:44] * bthomas is going to EOD now [22:51] * bthomas goes