[09:00:36] mutante / jhathaway > you probably want git grep instead if grepping inside our puppet repo (or any other repo) [09:04:57] <_joe_> Emperor: git grep (which I love and use continuously) is a double-edged sword at times [09:05:40] oh? I find it remarkably devoid of footguns for a tool starting git... [09:10:50] <_joe_> Emperor: fair enough, but also, sometimes .gitignore can trip you [09:11:06] <_joe_> esp when my muscle memory makes me use 'git grep' on repos in production [09:12:49] true [09:13:02] (it has runes for that but I always forget the need for them) [09:15:08] <_joe_> yeah :P [11:20:44] <_joe_> godog: fyi I'm going to depool thanos-fe1001 and apply the envoy patch there, do some tests, repool it, run puppet everywhere else [11:20:53] <_joe_> I just disabled puppet ofc before merging [11:21:16] _joe_: ack, thank you for the heads up. I'm off to lunch in 10min FWIW but I can stay if needed [11:21:46] <_joe_> godog: can one server stay depooled for some time in case of need? [11:22:02] <_joe_> if the answer is yes, you can go; else, let's reconvene after your lunch :) [11:22:04] _joe_: yeah there's plenty of capacity in the frontends [11:22:16] <_joe_> ack perfect [11:22:29] <_joe_> if in any doubt I'll wait for you being back before proceeding further [11:28:51] cheers [13:21:01] volans: using `logging.debug` or `logging.getLogger(__name__).debug` appears to produce the exact same result [13:22:27] kormat: https://docs.python.org/3/library/logging.html#logger-objects [13:22:55] cit. should NEVER [13:23:11] volans: sure. i'm not instantiating any loggers. [13:24:11] i do see this: [13:24:13] > organise your loggers on a per-module basis using the recommended construction logging.getLogger(__name__) [13:24:27] the point is that if you create a logger instance with getLogger [13:24:34] it get's namespaced with the name you provide [13:24:49] and then it can be handled as you want and that name can be part of your logs if you choose so in teh logging setup [13:26:26] ack [13:26:36] i'll fix this in a separate CR, as it's not just in this file. [13:27:12] to clarify, if you do logging.debug() directly, it goes all to the root logger [13:27:18] equivalent to getLogger() (without name) [13:27:24] sure [13:27:53] but will be all in the same single logger for all, loosing the namespace part that I thought you wanted in the log [13:29:32] and you can pick in https://docs.python.org/3/library/logging.html#logrecord-attributes all the things that you want to log [13:30:16] yah, already doing that. but this means i can move from %`(filename)s:%(lineno)d` to `%(name)s:%(lineno)d` which is cleaner [13:30:38] there is also funcName [13:30:43] whatever suits best your needs [13:32:35] volans: i also have Opinions about @property's in python [13:33:12] sure :) I think I added YMMV :) [13:33:28] well, just one, really: @property should be reserved for when you need to retain API compatibility in a class where what used to be a member is now a method [13:34:12] ~2 jobs ago i was dealing with a large (and horrible) python codebase. i was really confused at first until i realised that what looked like a simple attribute access was actually a huge amount of functionality hidden behind @property. grr. [13:34:14] or because you want it to be RO [13:34:52] i tend to use getters for that case, too. [13:54:24] you mean we shouldn't have db topology changes happening on db.master attribute assignments? ;/ [14:08:01] question_mark: for example! ;) [14:09:55] but that would be So Cool [14:12:11] narrator: it would not. [14:14:17] we could have entire spicerack cookbooks behind that [14:14:33] wmf.primary_data_center = codfw # tadaa [14:15:11] wonder why that assignment is a bit slow though [14:17:04] arturo: very helpful https://ral-arturo.org/2021/04/01/ip-token.html, thanks a lot :) [14:17:33] elukey: 🎉 [14:17:48] no, this is mediawiki, you want to use services like `$services->getDatabaseTopologyManagerFactory()->newForSection( 's1' )->setPrimary( $services->getDatabaseInstanceStoreFactory()->newForSection( 's1' )->getByName( 'db1234 ) );` [14:18:48] arturo: in my case I removed a virtual disk, the interface name changed and I didn't have the sysctl options set for the new one :) [14:18:49] I'm sure the DBAs would love if we moved db topology management to a mediawiki extension [14:19:15] elukey: and ifup failed? [14:19:21] arturo: exactly yes [14:19:41] elukey: cool. Now I'm curious, do you have a puppet patch? [14:19:48] arturo: I grepped for ens6 in /etc/, found the settings and set them for ens13 [14:20:26] arturo: not really, I moved one node (ml-serve-ctrl1002) from devicemapper to overlay2, and I removed the old /dev/vdb disk not used anymore [14:20:46] and once the node came back up, the interface was named ens13 [14:21:32] to be honest, the `ip token set` thing is nice and annoying 50% :-P [14:22:11] you saved me some debugging, and it was nice summary of the problem :) [14:22:26] taavi: 🥀 [14:24:27] elukey: nice, thanks. Some kernel devels contacted me as well, and later on they improved error reporting on that particular function. So later kernels should be more clear about what's wrong: https://elixir.bootlin.com/linux/latest/source/net/ipv6/addrconf.c#L5717 [14:25:16] arturo: nicee! [14:25:25] yeah it would have been way easier with those logs :D [14:26:01] now I need to do the same on other 3 nodes :P [14:26:17] good luck ^^U [15:03:34] one of the things I like about netplan is it makes it easy to do things like "build me a bond with all the devices that use the mlx5_core driver" [15:26:15] https://wikitech.wikimedia.org/wiki/Conftool#The_tools has an example query "confctl --object-type service select cluster=cache_text,name=varnish-be get" Which produces an error message 'CRITICAL:conftool:Object type service is not available in the current schema' ; if I remove the "--object type service" it merely returns no data (if I change it to varnish-fe then I get hits). Is that '--object-type service' just obsolete? Should I [15:26:15] replace it with a 'discovery' query like "confctl --object-type discovery select 'dnsdisc=swift.*' get" ? [15:26:51] <_joe_> Emperor: heh that example needs updating [15:26:56] <_joe_> yes [15:27:14] <_joe_> sorry I forgot we removed the service objects from the schema some time ago [15:27:23] I'd just fixed the previous example query which at least didn't error (but did return no output) [15:27:43] I'll use my discovery example [15:29:08] <_joe_> yeah or you can use the query to get which is the master mw datacenter, confctl --object-type mwconfig select 'name=WMFMasterDatacenter,scope=common' get [15:29:32] <_joe_> (or omit the scope) [15:31:28] I'll stick that in too [16:05:18] taavi or anyone else, can you confirm that deployment-puppetmaster04.deployment-prep.eqiad.wmflabs doesn't have its config within puppet (I'm trying to install cergen on that host, and wanna make sure I do it the right way) [16:06:21] not sure what exactly you mean with "config within puppet", but it's using the generic cloud vps project puppetmaster role (role::puppetmaster::standalone) [16:07:17] Sorry, I mean its config isn't managed within the puppet operations repo. But from what you just wrote, I guess it is? [16:07:52] I just don't see its hostname in site.pp , but I'm pretty new to puppet and the foundation [16:08:02] cloud vps instances aren't in site.pp [16:09:06] those are managed via horizon instead [16:16:21] usually roles include a line in the motd telling you about it, apparently this one doesn't.. will be fixed with https://gerrit.wikimedia.org/r/c/operations/puppet/+/759266/ [16:19:13] looking at https://github.com/wikimedia/puppet/blob/production/modules/profile/manifests/puppetmaster/frontend.pp#L52 , I'd guess the cergen class would be part of the puppetmaster config [16:20:20] but I don't see cergen on the host itself. I also can't find it with 'apt-cache search' [16:20:53] if you look at modules/cergen/manifests/init.pp you see it's pulled from a specific apt component, which is only included only if it's used [16:22:15] since most cloud vps projects don't need cergen I'd maybe configure role::puppetmaster::standalone include a profile that includes the cergen module if a some false-by-default hiera parameter is set to true [16:26:37] Hi all just a heads up im deploying a chnage to disable ldap verification for emails arriving at mx2001. with the new chnage any emails which dont hit a previous router will automatically be sent to gmail mx serveres without first validating the email is valid. this change has been deployed recently and workd without issues but was reverted due to unrelated kernal issues [16:26:44] cc moritzm jhathaway herron ^^^ [16:26:49] That seems reasonable, but isn't https://github.com/wikimedia/puppet/blob/production/modules/profile/manifests/puppetmaster/frontend.pp#L41 already doing that? [16:27:25] jbond: ack [16:29:20] So I tried pooling a new swift proxy [16:29:37] confctl select service=swift-fe,name=ms-fe2009.codfw.wmnet set/weight=40 ;confctl select service=swift-fe,name=ms-fe2009.codfw.wmnet set/pooled=yes [16:30:00] which seemed to work, but confctl select dc=codfw,cluster=swift get doesn't return the new host. What did I miss? [16:30:02] role::puppetmaster::standalone (which as currently implemented should really be a profile) isn't using that profile, which has been designed for production use and not for cloud vps [16:30:17] godog: I imagine you've done this before... [16:30:32] inflatador: some of the hiera config for deployment prep will be avalibe in hieradata/cloud/eqiad1/deployment-prep, however for the roles that get applied i you can either check in horizon via the puppet tabs, or checkout ssh://gerrit.wikimedia.org:29418/cloud/instance-puppet ` find deployment-prep/ -name \*.roles ` [16:31:26] Emperor: I did yeah, the hosts need to be in conftool-data/node/codfw.yaml too [16:31:28] however i would also like to push the use of cfsssl (https://wikitech.wikimedia.org/wiki/PKI) feel free to ping me if you have further question in relation to that [16:32:31] godog: argh, OK will push that (and update the docs to note the need to do so) [16:33:24] <_joe_> Emperor: wait, the docs should be clear about the fact you need to add the data in the yamls [16:33:30] Emperor: thank you! I can quickly +1 the change [16:33:50] <_joe_> btw, we could decide at some point we want to generate those yamls instead of writing them down [16:34:02] <_joe_> either from puppetdb or from netbox [16:34:23] _joe_: it probably is documented, but not in the Swift/HOWTO [16:34:32] (or I missed it, always a possibility :) ) [16:34:48] <_joe_> ah completely possible yes [16:35:05] <_joe_> that was a decision we made when we introduced conftool that made sense to preserve a trace of what the new tool did in git [16:35:19] <_joe_> but we could probably autogenerate the yamls for the lvs pools indeed [16:35:34] <_joe_> new servers get introduced as pooled=inactive anways [16:36:08] jbond thanks, cfssl is exactly what we are aiming for , taavi brought this up originally in https://phabricator.wikimedia.org/T298252 . If/when you have time I would love to go over the best way to do this on the deployment-prep puppetmaster [16:37:45] jhathaway: I am doing a puppet-merge and there's "Revert "profile::apt::mirror: change apt mirror to deb.debian.org" (562591a457)" [16:37:54] jhathaway: is that OK to merge? [16:37:58] Emperor: yup, thanks [16:38:14] done, thanks [16:39:39] inflatador: ack i can respond on task, if things are allready behind envoy it should be a very simple change but will take a better look and respond there [16:39:52] it's not based on envoy :( [16:40:16] looking better now. [16:40:27] ahh ok taavi from https://phabricator.wikimedia.org/T298252#7587752 im guyssing thats the plan though? [16:40:54] tha's my suggestion, of course it depends on who's implementing it (not me) [16:41:03] ack [16:41:07] thx [16:41:23] currently it's using elasticsearch::tlsproxy which in turn uses nginx via the tlsproxy::localssl define [16:41:58] We can't change that ATM [16:42:12] ahh cool ok that may not be too difficult to update, i have changed a few other things from tlsproxy::localssl to tlsproxy::envoy so may not be too hard [16:42:51] ahh ok, ill stop trying to solve here and will take a better look and respond on task :) [16:43:34] volans: you've been waxing lyrical for years about f-strings, and now you're saying i shouldn't use them for logging? grrr [16:44:05] I was just trying to provision certs with cergen on that host, and gehel (aka my boss) suggested we install cergen via puppet instead of just doing it manually . The goal for me is just to get those new certs provisioned for the new hosts, but I don't want mess things up/waste other's time doing it [16:44:41] kormat: :) (but I never did that ;) ) [16:50:00] Swift/Howto updated [20:41:14] ottomata _joe_ is that 'puppet-sign-cert' script y'all wrote on the prod puppet master in a repo somewhere? Wondering if we can get it on the deployment-prep04 PM [20:42:09] inflatador: https://github.com/wikimedia/cergen/blob/master/ext/puppet-sign-cert [20:42:50] should come with the cergen debian package i think [20:42:53] Thanks! Guess I need to up my web searching game [20:43:35] Good to know, I guess that's why cergen isn't working on deployment-prep04