[13:41:33] https://www.irccloud.com/pastebin/twV806so/ [13:41:42] Can anyone explain the above error? [13:42:36] show your patch too? [13:43:00] oh boy, ok, but fair warning it's a new chart so it's... a lot [13:43:11] https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/1248148 [13:43:19] i.e. as diffs go, it's all green :) [13:46:28] urandom: there is a way to make the review more human [13:46:40] let me fetch you an example [13:46:41] * urandom is listening [13:50:00] ok cant find one now, so create the new chart with only the defaults (even use some random as port), and add 'vanilla' or 'default' in the title [13:50:23] create a new commit where you will edit values.yam, so changes are easier to review side by side [13:51:06] fwiw the reason for that specific error is that charts/hoarde/templates/deployment.yaml only defines a container from mesh.deployment.container which is empty if Values.mesh.enabled is true, but that defaults to false and those failing tests don't overwrite it [13:51:55] correction: empty if Values.mesh.enabled is false [13:52:43] urandom: for example https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/1150760 && https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/1150762 with the latter being the one to actually review [13:55:50] taavi: thanks [13:55:52] effie: thanks [13:56:00] * urandom reaches for aspirin [13:57:00] urandom: so you decided to go with a new chart rather than using the python-webapp one? [13:57:02] may I ask why? [13:57:26] (sorry I bailed on our meeting, I'm a little swamped considering I'm still not recovered from the summit plague) [13:57:41] Raine: sure, it's because I don't understand what I'm doing, and I'm learning it all the hard way :) [13:57:57] Raine: specifically, I wasn't sure how I was meant to configure my service [13:58:33] The aqs-http-gateway (the one used for services most similar to this one), differs in ways that address that, so I started there [13:58:39] ok, I see [13:59:08] you mean the cassandra connection, right? [14:05:15] Raine: well that, yes, but also the tables/cache set [14:05:24] Raine: https://gitlab.wikimedia.org/repos/sre/hoarde/-/blob/main/sample-config.yaml?ref_type=heads#L19 [14:05:38] the latter is why I turned away from the aqs-http-gateway chart [14:07:04] I found an example of using the environment to configure services using python-webapp, but doing that here would be terrible I think [14:07:46] comparing the charts, that seemed like the biggest difference, that the aqs-http-gateway chart templated a config for cassandra (and druid) connections [14:08:26] this service does need the cassandra config, does not need the druid config, but *also* needs that table stanza above [14:13:00] so basically the problem is that you need a config file, not just a pile of env vars, and the python-webapp chart doesn't do that? [14:23:06] Raine: right; if it does, I don't understand how to do that [14:27:29] effie: (and for posterity sake): https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/1249962 & https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/1249963 [14:30:43] urandom: it does not [14:30:48] and I think we should fix that [14:31:16] but currently it does not [14:31:23] Raine: is there a way to do that in a generally useful way? [14:32:32] in principle, just having a config files dict that dumps some strings into ConfigMap templates would work [14:33:32] I'd probably make an "`app_config_files` or something like that"-named value in yaml, that's a dict of filename to (multiline) string [14:33:48] *in the values.yaml [14:34:06] and then a template that renders that in a ConfigMap and another one that mounts that ConfigMap [14:36:52] would having that (plus $someone helping you figure out the external_services for cassandra) enable you to use the python-webapp chart? [14:38:31] alternatively (I might be missing context): Could the aqs chart be refactored to a generic "things that are cassandra gateways" chart? [14:40:48] I wasn't aware you're taking care of this Raine since you where not on the reviewers list for the related changes (or am I missinterpreting?) [14:41:09] I'm not, I'm rather swamped [14:41:14] okidoke [14:41:34] I'm just wondering whether a new chart really is the right answer [14:41:42] sounded like it because "bailed on meeting" :) [14:42:06] yeah, general note would be that generalizing/refactoring an existing chart is prefered over generating a new one [14:42:22] jayme: that was about the (future) serviceops office hours I should start at some point [14:42:35] ah, I see [14:42:36] (but currently don't have the capacity for) [14:42:49] (because the offsite bug is super persistent) [14:42:57] (we should probably store our data in it) [15:31:34] re: above (use of python-webapp or refactoring the aqs chart), ¯\_(ツ)_/¯ [15:32:27] I wasn't sure how to do either, which is why I ended up here (sort of a Cunningham's Law in desperation approach) [15:33:40] I thought about adding a section to that config for my `tables` config stanza, similar to the druid one, where it would just be empty when it didn't make sense (everything else but hoarde) [15:34:01] "that config" being the aqs-http-gateway generated one [15:34:27] but it seemed desperate even for me [16:19:03] desperate in what way? The rational behind this is that each additional chart adds a maintenance burden that more often then never serviceops has to carry. I think the question here is more if it makes sense (from a logical/user standpoint) to use the AQS chart [16:19:53] if it does, because those two things have more in common than not, I would say it's worth it to add a knob to support the usecase (and maybe rename the chart to something more generic) [16:21:11] jayme: IIUC, there is either the option of using the AQS chart, or the python-webapp chart, both would need some modifications, but the python-webapp chart is newer and more general, so it would be more useful to put work into that one, WDYT? [16:25:27] yeah, makes sense as well. My understanding was that there is a bunch of stuff already present in AQS chart that is required for this [16:25:50] true [21:16:31] jayme: I meant the approach, the manner in which I was talking about extending aqs-http-gateway was bad (but easy/I understood enough to make it happen) [21:20:16] urandom: I thought about adding a section to the (aqs-http-gateway) config for my `tables` config stanza, similar to the druid one, where it would just be empty when it didn't make sense (everything else but hoarde) <-- if you think that approach is better than creating another chart, I could do that. If a cleaner approach is needed, I could use some guidance about what that might be