mthaddon | Chipaca: yep, have added an approving review overnight (had deliberately held off til then to avoid confusion about whether it's ready to land before you guys have had a chance to review it) | 08:19 |
---|---|---|
Chipaca | mthaddon: 👍 | 08:19 |
bthomas | Chipaca: could you recommend some well made operator charm, so I can check that there is nothing wrong with my juju/microk8s setup ? | 09:04 |
Chipaca | hmm | 09:06 |
bthomas | :) | 09:06 |
Chipaca | looking for one that's public and doesn't need a whole lot of other stuff around it | 09:07 |
bthomas | 👍 | 09:08 |
Chipaca | bthomas: https://code.launchpad.net/~mattermost-charmers/charm-k8s-mattermost/+git/charm-k8s-mattermost | 09:08 |
bthomas | Thanks Chipaca : will give it a shot | 09:09 |
Chipaca | bthomas: requires a custom image, instructions in the README | 09:09 |
bthomas | will read | 09:09 |
bthomas | The mattermost charm uses https://code.launchpad.net/~stub/interface-pgsql/+git/operator as a submodule . And the interface-pgsql uses opslib as a submodule. In the opslib/pgsql sub-directory of the interface-pgsql there is a link to ".." which creates a cyclic inode structure. | 10:11 |
bthomas | Even though I am able to clone the mattermost charm using my launchpad credentials cloning it recursively fails | 10:12 |
bthomas | Even submodule update fails. | 10:12 |
Chipaca | bthomas: pgsql uses opslib as a submodule? | 10:24 |
Chipaca | not sure i understand what that means | 10:24 |
Chipaca | i guess i could check out that git repo again | 10:25 |
bthomas | Chipaca: interface-pgsql has a pgsql/opslib subdirectory in that subdirectory there is a link to ".." | 10:27 |
Chipaca | bthomas: I don't see that | 10:32 |
Chipaca | bthomas: maybe you updated the mod beyond what the charm specifies? | 10:33 |
bthomas | When I tried to build the mattermost charm, even though it built the charm it gave an error saying pgsql was not found. Upon digging further I found that it used a git submodule. There is no mention of this in the readme. | 10:34 |
bthomas | So I am not sure if it is ok to use the built mattermost charm without fixing the pgsql issue. | 10:34 |
Chipaca | bthomas: how did you get the submodule? | 10:35 |
bthomas | Chipaca: even if I do git clone git+ssh://balbir-thomas@git.launchpad.net/~stub/interface-pgsql/+git/operator interface-pgsql I can see the link | 10:35 |
Chipaca | bthomas: right, but the charm doesn't say 'grab the dev tip' | 10:36 |
Chipaca | bthomas: or does it/ | 10:38 |
bthomas | Chipaca: Not sure. But the standard way to pull git submodules fails with mattermost charm. For example if I do "git clone --recursive git+ssh://balbir-thomas@git.launchpad.net/charm-k8s-mattermost". Tis fails to clone submodules. | 10:39 |
bthomas | Building mattermost charm without submodule produces an error, even though charm gets built. | 10:40 |
Chipaca | bthomas: did you try cloning with the https url? | 10:40 |
Chipaca | bthomas: it's possible you're missing the git-in-launchpad recommended git config; the .gitmodules file depends on that | 10:40 |
bthomas | Chipaca: Just did "git clone --recursive https://balbir-thomas@git.launchpad.net/charm-k8s-mattermost" with same result. I was not aware of git-in-launchpad. | 10:41 |
* bthomas googling git-in-launchpad | 10:42 | |
Chipaca | bthomas: https://help.launchpad.net/Code/Git | 10:42 |
bthomas | Thanks Chipaca : will read | 10:42 |
Chipaca | bthomas: the 'configuring git' section | 10:42 |
bthomas | Chipaca: you were right. It was the git config. Thanks. | 11:07 |
Chipaca | bthomas: \o/ | 11:07 |
gnuoy | Is it expected that when I try and push a charmcraft built charm to the charmstore it'll fail with "ERROR open mycharm.charm/bundle.yaml: not a directory" ? | 11:34 |
mthaddon | Chipaca: do you think you'll have time to review https://code.launchpad.net/~axino/charm-k8s-gunicorn/+git/charm-k8s-gunicorn/+merge/389554 soon? Junien has another MP that he's working on that depends on that but want to make sure there's not too much rebasing needed | 12:03 |
Chipaca | gnuoy: pushing it using the 'charm' tool? yes that won't work (unpack to a directory and push that) | 12:22 |
gnuoy | ack, thanks | 12:23 |
Chipaca | gnuoy: 'charmcraft upload' pushes to staging using the new APIs, if that's what you want though | 12:24 |
Chipaca | and that one does take the .charm (and not a directory) | 12:24 |
gnuoy | That is not what I want, but good to know, thanks. | 12:24 |
Chipaca | mthaddon: i've still got one from deej to review, and then one from justin, and then that one | 12:26 |
mthaddon | Chipaca: could i ask for this one to go above deej's? It's a day older, and there's another MP that depends on it, whereas deej's doesn't have another one depending on it | 12:27 |
Chipaca | mthaddon: i'll swap it with that one then | 12:27 |
mthaddon | perfect, thx | 12:27 |
justinclark | bthomas, did you get Mattermost running? I quickly tried deploying yesterday based on the README and it failed for me. | 13:34 |
justinclark | https://git.launchpad.net/~mattermost-charmers/charm-k8s-mattermost/tree/README.md | 13:34 |
bthomas | justinclark: Yes mattermost was running. It was idle because i did not provide it a database connection. But it was not my purpose to use it. I just wanted to check if it can be deployed and the container runs. | 13:35 |
justinclark | Ah okay. Thanks bthomas. It was probably the database for me too but got sucked into learning about cross model relations rather than going further with mattermost. | 13:37 |
Chipaca | justinclark: where was the charm you wanted reviewed? | 13:55 |
justinclark | https://github.com/justinmclark/grafana-charm-base/blob/master/src/charm.py | 13:55 |
justinclark | Thanks Chipaca! | 13:55 |
justinclark | Just FYI, jam suggested a change yesterday in an issue I opened. That is still WIP. Here's the issue: https://github.com/canonical/operator/issues/392 | 13:57 |
justinclark | Pushed some other changes I made last night Chipaca. The charm deploys to microk8s just fine by itself, but it is quite uninteresting without relating a data-source for the dashboards. That still needs to be tested | 14:02 |
Chipaca | :) | 14:03 |
Chipaca | ok | 14:03 |
Chipaca | justinclark: you're using 'charmcraft build'? | 14:03 |
justinclark | Yes. | 14:03 |
Chipaca | justinclark: ok. You should probably embrace it more then :) | 14:03 |
Chipaca | justinclark: right now you're getting ops via a git submodule | 14:04 |
Chipaca | justinclark: instead of requirements.txt i mean | 14:04 |
Chipaca | justinclark: and the 'fixpath' hack is not needed | 14:04 |
justinclark | Chipaca: ah yes that needs to be a more prominent TODO. I used an out of date example when first starting and just didn't bother changing it as testing was working as expected. | 14:08 |
Chipaca | justinclark: the 'configured' and 'started' storedstate things were red flags but they're not used at all so that's alright (just drop them) | 14:09 |
Chipaca | justinclark: you're using f-strings which don't work on python 3.5 (which you'll get in xenial) so you might want to drop those too | 14:09 |
Chipaca | justinclark: and you're setting state around a pod.set_spec which _i think_ (but am waiting to hear back from juju people) doesn't make sense | 14:10 |
Chipaca | because statuses are immediate but set_spec is not | 14:11 |
Chipaca | IIRC at least :) | 14:11 |
Chipaca | jam: if you're around already, maybe you can answer that? (haven't heard back from #juju) | 14:11 |
jam | morning Chipaca. set_spec is not immediate, it doesn't happen until the hook exits | 14:11 |
jam | (in 2.8) | 14:11 |
Chipaca | jam: right, that one i know | 14:12 |
Chipaca | jam: but setting status is immediate, right? | 14:12 |
jam | Chipaca: status = Blah is immediate, yes | 14:12 |
Chipaca | so, yeah, that pattern is broken | 14:12 |
jam | so you can show progress during an operation | 14:12 |
Chipaca | this one: https://github.com/justinmclark/grafana-charm-base/blob/master/src/charm.py#L350 | 14:12 |
Chipaca | jam: right but using it show progress for something you're scheduling to happen later won't dtrt | 14:13 |
Chipaca | in their defense, set_spec() does _sound_ immediate | 14:13 |
jam | Chipaca: correct. except that Juju filters Active status for the case of 'the pod hasn't actually started yet', and overrides it until the application pod has finished starting. | 14:13 |
Chipaca | maybe we should've called it set_spec_sometime_in_the_future_at_your_leisure_please_no_rush | 14:13 |
Chipaca | jam: oh that's sneaky | 14:14 |
jam | Chipaca: relation-set sounds immediate, too | 14:14 |
jam | Chipaca: this is a case where, Mark found out that 'leader-set' was immediate, which breaks the 'transactionality' that all other relation-get/relation-set code has. | 14:14 |
jam | So we went on a hunt for other things that violated that | 14:15 |
jam | and found pod-spec-set. | 14:15 |
jam | So we pushed for it to be changed | 14:15 |
jam | but having worked with charmers for much longer | 14:15 |
jam | I don't feel like pod-spec-set is an interaction *with other charms* it is an interaction with the charm and its application | 14:15 |
jam | so I no longer feel it should be transactional | 14:16 |
jam | (we don't put a transaction around 'systemctl start foo' | 14:16 |
jam | however, we do need to deal with how it is currently, and not how it might theoretically work | 14:16 |
jam | Also, the new work gets rid of pod-spec-set for charms running as sidecars | 14:16 |
Chipaca | yep | 14:17 |
Chipaca | jam: does the 'pod hasn't actually started yet' override thing mean that the code i linked to would currently dtrt w/2.8? | 14:19 |
jam | Chipaca: I don't think it would stay in the "maintenance with 'setting pod spec'" but otherwise yes | 14:22 |
jam | (eg, it uses Juju's 'pod not started yet' message, though I don't know the exact words) | 14:22 |
Chipaca | k | 14:22 |
* justinclark thanks Chipaca and jam for feedback :) | 14:29 | |
* justinclark pushes python 3.5 under the rug and blows things up | 14:31 | |
Chipaca | :) | 14:31 |
Chipaca | justinclark: one silly thing I'd also do in your charm is make the log.error call for missing fields mention exactly what was missing, instead of everything that's required | 15:00 |
justinclark | Chipaca: will do. Can you elaborate on the 'fixpath hack'? | 15:07 |
justinclark | I'm not sure what that hack is. | 15:08 |
Chipaca | justinclark: your charm.py does 'import fixpath' | 15:08 |
Chipaca | justinclark: fixpath.py just adds lib to sys.path | 15:08 |
Chipaca | justinclark: this was needed before charmcraft took on the job of setting up the environment for the charm | 15:08 |
Chipaca | justinclark: it's a hack, which is why you need to tell pyflake to ignore it | 15:09 |
justinclark | I see. Thanks. | 15:13 |
Chipaca | EOWing here | 19:13 |
Chipaca | bye all! see you tuesday | 19:13 |
jam | have a great weekend ChanServ | 19:41 |
jam | dang, gone already :) | 19:41 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!