[09:55:45] jbond42: ah hah. i wasn't sure if it was legal to include a profile inside a function. [09:56:40] <_joe_> kormat: uhhh it doesn't look great tbh in terms of code clarity [09:56:57] <_joe_> the idea was you can easily understand what's installed on a system by going [09:56:58] _joe_: i welcome your input :) https://gerrit.wikimedia.org/r/c/operations/puppet/+/667547/ [09:57:07] <_joe_> site.pp -> role -> profiles [09:57:45] <_joe_> is the profile in question just a bunch of params? [09:58:33] _joe_: it's literally a single `lookup()` [09:59:03] `modules/profile/manifests/mariadb/mysql_role.pp` [09:59:26] <_joe_> I would suggest a different approach, I'll comment on the patch [09:59:44] great, thanks [10:08:14] kormat: i don't think functions where possible in the puppet DSL when the style guide was created/last revised so more like they have not been considered however i think it makes senses for a function to depends on its module init class e.g. foo::do_stuff can depend on and include foo, this split becomes a bit more murky with profiles thought. fyi my personal prefrence in this case would be to [10:08:20] pass role as a parameter. im not a big fan of ... [10:08:22] ... the mariadb::mysql_role profile in genral and would see that getting subsumed in a more generic mariadb base profile somewhere down the line once oter refactors have taken place (we have spoken about this before im not sure if i have managed to convince you yet ;)). so from that point i think it also makes more senses to have it as a parameter. [10:10:33] * kormat waits patiently for _joe_'s review to completely contradict everything ;) [10:11:15] <_joe_> kormat: I have one q: some functions take 'role' as a param, others don't [10:11:18] <_joe_> why is that? [10:11:28] _joe_: yeah that's the main discussion point with jbond42 [10:11:46] <_joe_> I asked if there was a reason [10:11:50] i started without, then thought using a param was better, [10:11:54] and apparently didn't convert them all [10:12:09] some functions don't need a role, some do, depends on the logic, too. [10:12:10] <_joe_> ok no, I agree with jbond42 on the fundamentals [10:12:29] alright. make everywhere that needs it use a role param, no problem [10:13:17] <_joe_> I have proposed an alternative, but I feel the right direction would be to input the role into the function [10:19:37] _joe_: "I'd avoid early optimizations" - i.e. leave it as it is? [10:19:44] <_joe_> yes [10:19:48] ๐Ÿ‘ [10:20:22] <_joe_> I think we can live with 10 lookups for the same param, given our lookup backends do cache the yaml contents [10:21:11] <_joe_> so it's just a function call + in-memory lookup, not that worse than what puppet does to find the value of a parameter [10:25:06] cool. CR updated addressing all comments [10:25:22] it just wouldn't be a kormat puppet CR unless it ran into _some_ style corner-cases ;) [10:25:34] :) [10:36:02] <_joe_> "you know what's worse than puppet? kormat puppet!" [10:36:12] ๐ŸŽ–๏ธ [10:39:02] fyi all just released a new package of wmf-laptop (thanks dcaro https://gerrit.wikimedia.org/r/c/operations/debs/wmf-sre-laptop/+/667887) [10:39:32] ๐Ÿ‘ [10:40:20] jbond42: should we be concerned? :) [10:41:25] :) of course not [10:41:44] ๐Ÿคจ [10:59:02] PSA: swift codfw is going to be repooled in 10/15 mins (cfr T267338) [10:59:03] T267338: Depool codfw swift cluster - https://phabricator.wikimedia.org/T267338 [12:16:40] have we hit issues with restbase trying to speak ipv4 when it should be speaking ipv6 before? some of the restbase-dev hosts have started alerting and the errors are coming up as "connect ECONNREFUSED 127.0.0.1:6500" [12:16:57] the rb configs have the entries defined as localhost though [13:34:13] hnowlan: i could be wrong, but i think I recall there being a discussion about something like this in here, i think it bad to do with envoy [13:34:28] Had* [13:35:11] Iโ€™d have to check my irc scrollback to be for sure though [13:41:10] Zppix: nice, I'll have a look [13:41:17] seems like there is an issue: https://phabricator.wikimedia.org/T276323 [13:43:37] Hopefully finding solution isnt too difficult [13:44:56] I think I had a conversation about it at some point and if I remember correctly this is not an official foundation page but rather something a volunteer copies over from another source. [13:45:07] sobanski: ECHAN [14:51:32] at 15:30 i'm gonna disable puppet on appservers to do a test rollout of a new mtail change, any objections? [14:51:47] 1530 utc that is [14:53:26] hnowlan: I don't think liw is in there but the MediaWiki train window still open [14:53:31] In here** [14:54:31] RhinosF1: ack, I'll wait until the window is closed [14:55:56] hnowlan: I'm not sure how it'll affect it so best ask them tbh. [15:04:00] <_joe_> hnowlan: no reason to wait, sorry I didn't see it before [15:04:02] <_joe_> go on [15:07:33] jbond42: okay to merge "P:pki::root_ca: add ability to generate intermidiate certificates" ? [15:08:49] jayme: yes sorry [15:09:05] jbond42: np, merged [15:09:10] thx [15:30:32] hnowlan: train looks to have completed for this window. Best to ask liw first though. [15:44:08] godog: I'm looking at this email from freenode about wmf-logmsgbot; did you already take care of that? (It looks like something you've been working on) [15:44:47] andrewbogott: _joe_ mentioned it in another channel, it's safe to ignore [15:44:56] did we cause any trouble? [15:45:08] rzl: cool, thanks [15:49:03] mtail change looks okay, reenabling puppet [15:55:58] andrewbogott: yeah what rzl said [15:58:33] I only noticed because I have the vague feeling that I created that account years ago. [16:15:28] andrewbogott: _joe_ created it while troubleshooting T276299 [16:15:28] T276299: dbctl not sending !log to IRC - https://phabricator.wikimedia.org/T276299 [16:15:38] ok :) [16:15:48] <_joe_> yeah sorry I forgot to reply to the email [16:16:05] sorry i just realized i pinged you [16:18:08] I have clarified the task [16:19:48] I think I may look into seeing how easily we can make the tcpircbot role not set the username, but let it be set in hiera [16:22:10] s/role/profile [16:24:34] <_joe_> Zppix: I think our energies would be better spent rewriting tcpircbot without all of its peculiarities [16:24:51] oh. [16:25:24] <_joe_> that was a fancy way to say it has more bugs than lines of code [16:26:26] +1 [16:26:28] Yeah, does seem to have quite a few issues, plus it probably couldnt hurt to bring it "up-to-speed" so to speak (SASL, etc) [16:27:08] to whoever ends up rewriting it, I would recommend https://github.com/bd808/python-ib3/ which makes sasl etc easy [16:27:30] a good project for a gsoc? [16:27:41] I couldnt see why not [16:29:14] how large should gsoc projects be? [16:30:02] 10 weeks, 18 hours a week [16:31:38] <_joe_> Majavah: that seems nice, but I don't trust the author [16:31:41] <_joe_> :} [16:34:22] heh [17:56:21] puppet on alert1001 is broken [17:56:30] Duplicate declaration: Monitoring::Check_prometheus[mediawiki-error-rate] [20:44:31] where do I run puppet for changes to conftool-data to take effect? https://gerrit.wikimedia.org/r/c/operations/puppet/+/668190 [20:52:20] legoktm: it's done automatically by the puppet-merge [20:52:32] towards the end, after the sync of all puppetmasters [20:53:30] it runs conftool-merge [20:53:48] see https://wikitech.wikimedia.org/wiki/Conftool#The_tools [20:55:31] you need sudo -i [20:55:52] or will not find the credentials to connect to etcd [20:56:22] beware that the weight will need to be set manually IIRC [20:56:31] afterwards [21:13:05] volans: ack, thanks. Should I get in the habit of running `sudo -i puppet-merge` then? [21:13:48] yep I do that all the time (thanks to ctrl+r :-P ) [21:16:52] <_joe_> no you don't need sudo -i anymore [21:17:10] <_joe_> it's been fixed like 1 year ago [21:20:15] > 2021-03-03 21:20:05 [INFO] conftool::load: Creating node with tags eqiad/docker-registry/docker-registry/registry1003.eqiad.wmnet [21:20:19] it worked without -i [21:28:26] oh nice, I didn't get the memo :D [21:28:35] and wikitech is not updated either fwiw [21:39:57] I won't update it because I'm off the clock but I'll nag someone else to do it :-P [21:40:01] <_joe_> legoktm: remember to also set the weight to non-zero [21:40:16] I did, set it to 10 [22:37:05] If i replace a host in smokeping. then run puppet.. i see it change the smokeping config, but I do not see a change in the actual HTML on smokeping.wikimedia.org. it's not behind caching layer though. hmm, do I have to hard restart it or just wait? I will assume I am not causing an alert email anymore though if my old host goes down now, since /etc/smokeping config was edited. I think i was [22:37:11] wondering the same last time [22:38:28] of course.. i just had to wait a minute longer and refresh and there it is now. ignore :) [22:38:58] probably it needs to not only restart the smokeping daemon that does the pinging, but also, it needs to complete a full ping cycle to record results where the cgi script can see them [22:40:17] that could match the timing, *nod* [23:40:54] hm..logstash is doing something not so useful to the http.client_ip field. it is seeing aa.bb.cc.dd as four terms and giving it a keyword index like 'http.client_ip.keyword'. That's not too bad by itself, but it seems the "Unique count" panel doesn't see the normal 'http.client_ip' field, only the keyword variant [23:41:54] any advice on how to make this work, e.g. by telling it (declarative in puppet) to e.g. only use space-separated terms for this field, or maybe there's another way to aggregate this that I don't know of? [23:43:10] i suppose if/when we hash it, that will solve itself [23:56:27] Krinkle: that is by design. http.client_ip is indexed as natural language (which is not aggregate-able) but http.client_ip.keyword is captured as a unique keyword