/srv/irclogs.ubuntu.com/2020/11/12/#smooth-operator.txt

bthomasmorning09:09
facubatistahola bthomas10:32
facubatista¡Muy buenos días a todos!10:33
Chipacafacubatista: 👋❗10:33
facubatistahola Chipaca :)10:33
bthomasनमस्ते facubatista : looks like you beat me to saying hola first today :-). So I am going back to saying नमस्ते10:34
bthomasMui bein10:34
facubatistabthomas, yes, I read *your* "morning" and reacted to that :)10:34
bthomas:-)10:35
Chipacafacubatista: did you forget to push something to charmcraft#180? not seeing the changes you talk about there10:57
mupPR charmcraft#180: Support to create a library, first step in all the lifecycle <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/180>10:57
facubatistaChipaca, I didn't finish my "reaction to the review"11:00
facubatista(this is a github problem, in launchpad I send the responses to the comments all at the end)11:00
Chipacafacubatista: ␇ shame ␇11:01
Chipacafacubatista: have you seen simple-term-menu?11:35
facubatistaChipaca, nop11:36
facubatistaChipaca, there I pushed my changes as per your review, note that I answered one of the items with an open question11:44
facubatistaChipaca, the '--lib-name' one11:44
facubatistaChipaca, so I'm happy to land this, but let's talk before about --lib-name11:59
Chipacafacubatista: 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?11:59
Chipacafacubatista: note how --charm-file is now being a bother when we want to use it for a bundle or a charm, for example12:00
facubatistaChipaca, no case for a command taking both12:02
facubatistaChipaca, 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:03
Chipacafacubatista: are they optional?12:04
facubatistayes12:05
Chipacak12:05
facubatistasorry12:05
Chipacafacubatista: what would the help lok like12:05
facubatistalist-lib is12:05
Chipacaah12:06
Chipacafacubatista: if it's not optional it shouldn't be an option (and you know this :) )12:06
facubatistacreate-lib has a mandatory "name" parameter, the help is "The name of the library file (e.g. 'db')."12:06
Chipacaok12:06
Chipacaright12:06
Chipacayes, that's already so12:06
Chipacagood12:06
facubatistalist-lib has an optional "--charm-name", the help is "The name of the charm."12:07
Chipacafacubatista: now, about list-lib, is --charm-name the right thing?12:07
Chipacathe right name for the option i mean12:07
Chipacafor create-lib's argument, 'name' makes the most sense, as what else would it be12:07
facubatistaack, changing it for create-lib12:08
Chipacafacubatista: this is my opinion, not an order :-p12:08
Chipacapad everything with IMHOs12:08
Chipacafacubatista: i could also argue against myself 🙂12:09
Chipacabut i'd rather if you did12:09
facubatistafor list-lib is less clear12:09
Chipacafacubatista: for list-lib, --charm-name seems … i dunno, overly verbose?12:09
ChipacaOTOH I can see the argument of --name being ambiguous12:09
Chipacabut why not --charm ?12:09
facubatistaChipaca, I like --charm12:11
facubatista"get me all the libraries for this charm"12:11
facubatistas/get/list/12:11
JoseMassonGood morning! | Buenos días! | शुभ प्रभात!12:12
facubatistahola JoseMasson :)12:14
Chipacafacubatista: OTOH12:15
Chipacafacubatista: what is the label of the table that list-lib produces under which you put the charm's name?12:15
Chipacafacubatista: the option should match that label12:15
Chipacafacubatista: probably(tm)12:15
Chipacas/table/column of the table/12:16
facubatistaChipaca, I don't put the charm name in the table (there's no point, all rows would just repeat that)12:17
Chipacafacubatista: but doesn't list-lib list all libs across charms?12:18
Chipacafacubatista: looking at the draft i see it's only ever for one charm12:26
facubatistaChipaca, no, list-lib list all libs for a charm12:26
facubatistaright12:26
Chipacafacubatista: but i'm a bit confused because if i do create-lib, the lib i created is not in list-lib12:26
facubatistaChipaca, yes it is, the lib created is for your charm12:27
facubatistalist-lib by default lists all libs for your charm, so12:28
Chipacafacubatista: i get 'Noting found.'12:28
facubatistammm... well, did you *publish* it?12:28
Chipacano12:29
facubatistaanyway, I need to run or I won't get to the bank12:29
Chipacarun run12:29
facubatistawe can talk later about this, thanks!12:29
Chipacai need to have lunch soon or i won't have time for it12:29
mupPR operator#446 opened: Minor clarification for deferred event doc string <Created by balbirthomas> <https://github.com/canonical/operator/pull/446>12:43
bthomas^ minor doc string update12:43
Chipacabthomas: s/being/begin/12:55
Chipacabthomas: but12:56
Chipacabthomas: that last statement probably confuses more than it helps12:56
Chipacabthomas: if you're not already thinking it's some sort of magic continuation, it'll throw you for a loop12:56
bthomasChipaca: 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:00
Chipacabthomas: 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
bthomasAwesome. Just edit the github PR when you do makeup your mind.13:02
* bthomas -> lunch13:51
* facubatista is back14:08
* bthomas too14:21
morphishey all!14:22
morphisDominikF 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:23
Chipacano, charms will be arch-dependent14:26
Chipacathat's fine14:26
Chipaca(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:26
Chipacamorphis: ^14:27
morphisChipaca: hm, so the store will be able to distribute different revisions depending on the architecture same as we have for snaps?14:36
Chipacamorphis: yup14:36
morphisah ok14:36
morphisChipaca: how much of this is already in place?14:36
Chipacamorphis: the store is multi-arch-aware but nothing else is afaik14:37
Chipacamorphis: for now everything is assumed to be arch: all14:37
morphisso we have to workaround with shipping two different venv's until that is fixed14:37
Chipacamorphis: you're wanting to support multiple arches?14:38
morphisyes, we have to support both arm64 and x86 with the same charm14:38
Chipacabummer14:39
Chipacabut, yes14:39
morphisit wouldn't be too much of an issue just complicates the build process a bit14:39
Chipacamorphis: is this for running on juju >=1.8?14:40
morphisoh yeah, assume the latest juju14:40
Chipacamorphis: ok14:40
Chipacafacubatista: ^^^ please read from morphis' "hey all" down14:40
morphisChipaca, facubatista: happy to sync on a call on this14:41
DominikFwould, love to be included in that call14:49
* Chipaca bans DominikF 14:49
* Chipaca hides14:49
Chipaca🙂14:49
DominikF😂14:51
facubatistareading14:51
facubatistamorphis, 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 that14:54
Chipacafacubatista: i think doing the multi-venv is better14:54
Chipacaotherwise your charm suddenly needs The Internet14:54
Chipacaand set up gets slower, and all the bad things we know about14:55
morphisyes, it needs the internet + gcc, ...14:55
Chipacaexactly14:55
Chipacaand libnacl is scary14:55
Chipacadeps-wise14:55
Chipaca(iirc)14:55
facubatista(*) 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 needs14:55
Chipacafacubatista: does the store support arch: many for charms?14:56
morphishow would charmcraft deal with the cross compilations aspects?14:56
Chipacamulti?14:56
Chipacaarch: [amd64, i386]14:56
facubatistamorphis, similar to how snapacraft does it today14:56
facubatistaprobably using multipass, if you do it in your machine14:56
morphisso you either offload to a remote arm machine or you run on arm14:56
Chipacamorphis: launchpad builders or equivalent, yes'14:57
morphisok14:57
facubatistamorphis, and eventually we would have launchpad builders14:57
facubatistathat14:57
Chipacathat's in the fullness of time, which is a way aways 🙂14:57
morphishehe14:57
morphisok, I think for now we will manually perform what charmcraft is doing to generate the multi venvs14:57
Chipacafacubatista: for now, if you have a way of doing a 'charmcraft build' on x86 and amd64, you can then bring them together14:58
Chipacamorphis: ^14:58
facubatistaChipaca, how?14:58
Chipacamorphis: and if it's only for new jujus, nuke hooks/* and put the arch thing in dispatch14:58
Chipacamorphis: note that it's called venv, but it's not a venv, just a lib14:58
Chipacashould be straightforward to rename it and pivot in the dispatch (which is a shell script)14:59
Chipacaanyway, i'm off (more meetings)14:59
facubatistaChipaca, ah, got it14:59
facubatistamorphis, do you understand what Chipaca is proposing? else I can help you14:59
morphisChipaca: yes, we can easily do a uname -m check in the dispatch script and switch the venv/lib there14:59
morphisfacubatista: yes I do, will ping you if we run into any problems14:59
facubatistamorphis, wonderful14:59
morphisthanks facubatista & Chipaca !15:00
facubatista:)15:00
ionutbalutoiuHello everyone!15:04
JoseMassonHello ionutbalutoiu!15:08
Chipacaionutbalutoiu: 👋15:51
Chipacafacubatista: in charms.foo.v0.bar, is v0 the API version of bar?16:05
facubatistaChipaca, yes16:06
facubatistahello ionutbalutoiu16:06
Chipacafacubatista: new_library.py.j2 has a syntax error unless I'm misunderstanding something16:07
Chipacafacubatista: is that on purpose?16:07
Chipaca… and you merged it, o no :)16:08
Chipacafacubatista: you're missing tests, obviously, because otherwise you would've caught the error16:09
Chipacafacubatista: see the tests for 'init', maybe?16:09
Chipacaor _something_ :)16:09
facubatistaChipaca, will add a test for it be a valid Python file16:24
bthomasChipaca 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:07
Chipacabthomas: I'm going to pop out now, go for a run, and be back at about 19:3017:10
bthomasthanks17:10
Chipacabthomas: ie in 2 hours and a bit17:10
bthomasno problem17:10
Chipaca(time to get changed + 2 hour run + shower)17:10
bthomasnice17:10
bthomasPrometheus altermanager changes have quite a few merge conflicts. I am working through them now.17:24
facubatistabthomas, it's better to always talk in UTC-0, because of DST and everything I don't know that is 6pm uk time17:24
bthomasfacubatista: agreed. I think I am UTC+1 zone.17:24
facubatistaI'll be off between 19:00z and 21:30z17:25
bthomasI 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/517:39
mupPR prometheus-operator#5: Alertmanager support <Created by dstathis> <https://github.com/canonical/prometheus-operator/pull/5>17:39
* bthomas goes to make some tea17:39
* bthomas is back17:54
bthomasI am off to get some groceries and dinner. Will be back in an hour.18:09
bthomasI need one more approval on ^ pull request. Will merge when back.18:09
* bthomas goes18:09
facubatistareviews appreciated! https://github.com/canonical/charmcraft/pull/18118:31
mupPR charmcraft#181: Support for publishing libraries <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/181>18:31
* facubatista bbl18:59
* bthomas is back19:46
bthomasFive guys are also under lockdown. This is depressing. :(19:46
* Chipaca is _almost_ back19:55
bthomasChipaca: All you have to do is click OK :-) https://github.com/canonical/prometheus-operator/pull/520:11
mupPR prometheus-operator#5: Alertmanager support <Created by dstathis> <https://github.com/canonical/prometheus-operator/pull/5>20:11
justinclarkbthomas, just approved20:11
bthomasAh thanks justinclark20:12
bthomaswait I still see merging is blocked20:12
bthomasNever mind I just did the second approve. :-)20:13
Chipacabthomas: no README?20:13
Chipacawait, wrong project20:14
Chipacabthomas: how's the alertmanager README etc coming?20:15
bthomasChipaca: I will create another pull request with README for alertmanager-operator20:16
bthomasCleaning commings to merge alertmanager changes to prometheus operator20:16
Chipacaok,fair20:16
Chipacabthomas: do you need me to review https://github.com/canonical/alertmanager-operator/pull/1 ?20:17
mupPR alertmanager-operator#1: MVP for alertmanager charm <Created by dstathis> <https://github.com/canonical/alertmanager-operator/pull/1>20:17
bthomasChipaca: 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:19
bthomasMerged changes into prometheus. Building prometheus to check nothing broke20:23
Chipacabthomas: reviewed the https://github.com/canonical/alertmanager-operator/pull/1, +1 with notes20:27
mupPR alertmanager-operator#1: MVP for alertmanager charm <Created by dstathis> <https://github.com/canonical/alertmanager-operator/pull/1>20:27
* Chipaca digs into his guiso de lentejas20:27
Chipaca(a spanish dhal)20:28
bthomasChipaca: 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/1220:31
mupPR prometheus-operator#12: Fix incorrect label for stored state <Created by balbirthomas> <https://github.com/canonical/prometheus-operator/pull/12>20:31
bthomasGoing 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 operator20:32
justinclarkbthomas, just reviewed20:39
bthomasthanks justinclark20:39
justinclarkNeed to make the same change in the tests20:39
bthomasjust deployed alertmanager and prometheus and related them. No errors. Trying to figure out if the relation actually worked.20:40
bthomasjustinclark: 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:40
bthomasChipaca: 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:42
Chipacajldev: ^ i think that is what you meant?20:51
Chipacaunless he meant in the alertmanager logs itself20:51
justinclarkbthomas, I'm fixing tests now and will push to that PR20:54
bthomasjldev: Chipaca just looking at logs. There are no errors but no information regarding alertmanager joining.20:55
bthomasthanks justinclark20:55
* bthomas googles how to check alertmanager prometheus interaction20:56
justinclarkIf 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
justinclarkbthomas ^20:56
bthomasjustinclark: I have no preference either way. Check if jldev has any preference.20:57
bthomasjustinclark: please keep commits separate . Not great to be missing alertmanager commits with grafana commits20:57
justinclarkWill do. That is the name we agreed on, but didn't know if I should sneak the commit into this PR or not.20:59
bthomasjldev: 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
bthomasI am not sure what else to check.21:00
bthomasjustinclark: 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:01
justinclarkI will fix tests. Feel free to focus on the README21:02
bthomasawesome thanks21:02
bthomasjustinclark: you are an angel :-)21:02
justinclark:)21:30
bthomasReadme is baking, will be ready soon21:30
justinclarkIt 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
bthomasjustinclark: I think it is. Chipaca facubatista may be better informed here.21:32
Chipacayes that is expected behaviour21:32
justinclarkOkay cool. Then the tests just need to be rewritten.21:33
Chipacaevent.unit is None for app data changes21:33
* facubatista is back21:33
* Chipaca goes for a cuppa (and some digestives why not)21:38
bthomasChipaca: when you are back https://github.com/canonical/alertmanager-operator/pull/221:43
mupPR alertmanager-operator#2: Updated readme <Created by balbirthomas> <https://github.com/canonical/alertmanager-operator/pull/2>21:43
* Chipaca is back21:44
bthomasjustinclark: I am done with readme. Do you need help with tests ?21:44
justinclarkbthomas, just finishing up.21:44
bthomasawesome21:44
Chipacabthomas: ooh, typo21:44
bthomasI am going to hammer another coffin name while Chipaca rips my README to shreds :-)21:45
* bthomas goes for a puff21:45
justinclarkThere is definitely still some logic that doesn't make sense given the alertmanager changes.21:46
Chipacabthomas: 1st suggestion, to make it easier to write markdown text with lots of links, embrace the 'reference' style using21:47
Chipacaah, if you're gone, i'll make the suggestions in code21:48
* bthomas is back21:53
* bthomas picks up the pieces of the README :-)21:53
* bthomas googles reference style in Markdown21:53
* bthomas reads https://www.simondcoll.com/references-markdown-zotero/21:54
Chipacabthomas: ah, just pushed my +1 with suggestions21:57
bthomasThanks Chipaca21:57
justinclarkThe 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:01
bthomasjustinclark: that should be in the test only. Though there were merge conflicts like that, they should have been resolved for the charm.22:02
bthomasI am merging readme now22:02
justinclarkI see - the relation-departed is still in the charm when it should have been replaced with relation broken22:04
bthomasjustinclark: My understanding was relation departed removes on particular alertmanager but relation broken clears the entire list.22:05
justinclarkThat's not how it would work in the current iteration. addrs is being stored in _stored.alertmanagers which is a list22:07
justinclarkrelation-departed is popping as if it were still a dictionary22:07
bthomasjustinclark: 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:09
justinclarkI see. I'll add a quick fix.22:10
justinclarkI'm actually not sure why we need departed.22:13
bthomasjustinclark: 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:13
bthomasjustinclark: 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
bthomasAt least that is how I understand it.22:14
justinclarkHow would we know which unit to remove?22:16
justinclarkAlertmanager 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 information22:17
bthomasFrom my reading of Dylans code I understand he was saving the alertmanager hostnames names in a list.22:17
bthomasWhen an alertmanager relation departs. I think you get the unit name as part of the event22:17
justinclarke.g. alertmanager/2 could be tied to alertmanager-1.xxx.endpoints22:18
justinclarkMaybe I'm overcomplicating it in my head22:18
bthomashang on, I am double checking22:18
bthomasjustinclark: If you look at alertmanager charm.py22:20
bthomashe is saving the alertmanager unit hostname in the application relation data22:20
bthomasso prometheus should be storing them in its stored state list22:21
bthomasSeems like there is a need to translate between unit name and host name22:22
* bthomas digs22:22
Chipacabthomas, justinclark, i'm still awake but going to go read stuff and slowly crash; mention me if you need me and i'll pop by22:22
justinclarkThanks Chipaca22:23
Chipacaof course my internet cuts off in 40 minutes to help me sleep so after that you're on your own 🙂22:23
bthomasChipaca: we will try not to disturb you unless the sky begins to fall. Good night.22:23
Chipaca(bah, if the world is on fire, you can @ me on mattermost -- my phone still works)22:23
justinclarkbthomas, 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:24
bthomasjustinclark: 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
bthomasjustinclark: 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:25
bthomasIf you like I can get on to hangouts.22:26
justinclarkMaybe we wait for Dylan in the morning? He may already have thoughts about this since he's been working on it.22:27
bthomasjustinclark: Do you know when this should be done by ? I know for sure before next tuesday. Is jldev around ?22:28
justinclarkI know he asked that Graylog be done by tomorrow. I would suspect the same is true for alertmanager/prometheus22:29
bthomasI am messaging him on mm now.22:30
bthomaswaiting for response from jldev22:35
justinclarkack22:36
justinclarkI 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 convo22:39
bthomasjustinclark: OK. Looks like jldev is away and I am getting weary.22:40
bthomassleepy programmers are not good programmers22:40
justinclarkI can also get back to it after dinner if it's absolutely critical that it's done today.22:40
justinclarkHaha have a good rest of the night bthomas.22:40
bthomasjustinclark: 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:41
bthomasLast thing I will try and do now is rebase that monitoring k8s branch on latest master to keep it updated.22:42
bthomasThis 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:43
* bthomas is going to EOD now22:44
* bthomas goes22:51

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!