[09:40:22] the rule about puppet modules not being allowed to load hiera, only profiles, seems a bit.. artificial. the natural consequence seems to be that modules get promoted to profiles, at which point.. what was the gain? [09:43:28] <_joe_> uhm, no? [09:43:41] <_joe_> or well, if people do that, then we have other issues I guess [09:44:13] <_joe_> the idea is that the module implements a base piece of functionality, and classes in modules get only *explicitly* declared in profiles [09:44:32] <_joe_> where all hiera calls are concentrated, so that it's more explicit [09:44:51] <_joe_> kormat: care to give me an example of "modules promoted to profiles"? [09:44:58] well, i guess if we're ok with having the hiera calls duplicated a lot v0v [09:45:28] <_joe_> or well, of the problem you're trying to tackle [09:45:41] https://gerrit.wikimedia.org/r/c/operations/puppet/+/620722 is the case in point [09:45:51] right now there's a module that needs a mapping loaded from hiera [09:46:13] with the current approach that mapping will need to be loaded from about 8 different profiles in order to be passed in [09:46:17] which seems pointless to me [09:46:30] i'd much rather have the module load the data itself. [09:46:54] <_joe_> so, your problem is that you have 8 profiles when you probably should have one :) [09:47:12] uff. having one profile would be Very complex [09:47:39] <_joe_> what are the profiles? [09:48:43] modules/profile/manifests/mariadb/core/multiinstance.pp modules/profile/manifests/mariadb/dbstore_multiinstance.pp modules/profile/manifests/mariadb/misc/analytics/multiinstance.pp modules/profile/manifests/mariadb/misc/analytics/multiinstance.pp [09:48:46] and so on [09:48:54] <_joe_> ok [09:49:05] er, there's a dupe there. there's another 5 or so [09:49:15] <_joe_> and those need to include what class? [09:49:40] <_joe_> sorry, I'm trying to understand, the patchset you showed me doesn't make it clear [09:49:44] in the above CR, they will all end up including mariadb::wmfmariadbpy::common [09:49:53] (the CR has not been updated yet to show this) [09:50:01] <_joe_> oh I see [09:51:00] <_joe_> is that class already in puppet? [09:51:17] <_joe_> no it's not :) [09:51:18] it's added in the CR [09:51:32] <_joe_> not with that name [09:51:52] ah, it's just been renamed. it's now `mariadb::wmfmariadbpy` [09:52:24] <_joe_> ok, my question is: do those machines just need mariadb::wmfmariadbpy or the full profile::mariadb::wmf_root_client ? [09:52:28] I don't see what is the issue- it follow the same pattern that others do role > profile > module class [09:52:45] _joe_: wmf_root_client is for cumin hosts [09:52:51] <_joe_> ok [09:53:06] config from hiera is taken at the top level profile [09:53:15] and module classes are called with those parameters [09:53:55] jynus: my point is that the config will be loaded in ~9 places [09:54:01] which seems unnecessary [09:54:10] but that is a different issue unrelated to this patch [09:54:17] that is duplication on classes [09:54:27] on profile [09:54:31] that is not on my scope to fix [09:54:50] eventually there will be (almost) only 1 profile caller [09:55:19] <_joe_> kormat: let me rewind for one sec [09:55:45] <_joe_> what you want to obtain is to have wmfmariadbpy installed and properly configured on any mariadb server, correct? [09:56:03] on all machines that require it. (so mariadb servers + cumin, currently) [09:56:17] <_joe_> yes, but cumin is "solved" in your patch [09:56:37] <_joe_> so, if I had to guess, this is not the only thing that's common to all those profiles. [09:56:58] <_joe_> I would create a profile::mariadb::common that includes such stuff [09:57:01] sure, there is a lot of work pending [09:57:07] <_joe_> and is then required by the other profiles [09:57:09] but that is not in my scope [09:57:12] _joe_: are 'sub'-profiles allowed to load from hiera? [09:57:17] that is refactoring unrelated to this [09:57:23] <_joe_> it's not a sub-profile [09:57:26] (i assumed yes, but it appears there is uncertainty on this point) [09:57:42] _joe_: oh. what's the difference between that and a sub-profile? [09:57:56] kormat: what he says is to include a profile with the common stuff [09:57:58] <_joe_> I don't know what a sub-profile is [09:58:02] (i define a sub-profile as a profile included by another profile) [09:58:19] which you can do on a later patch [09:58:25] <_joe_> ahah ok. no it's not a sub-profile, if you want it's a pre-profile [09:58:39] <_joe_> it's something that needs to be installed for your profile to make sense [09:59:10] <_joe_> and yes, it's just a profile as all others, it can even be installed independently, and it can must make its hiera calls by itself [09:59:13] jynus: i'm interested in figuring out the best way to aim for. once i have a clearer picture of that, then we can discuss what bits belong in this CR vs later CRs [09:59:16] basically, I am not touching the profile [09:59:42] you can do whatever you want with that, I am just adding the new functionalytiy (on module) and calling it [09:59:49] you can refactor the profile as you want afterwards [10:00:27] <_joe_> jynus: ok, I'll help kormat with that in a followup patch [10:00:48] kormat: the thing is, if we don't merge this early, the problem will become worse beacus cloud is about to duplicate this code [10:00:56] in the patch I sent you [10:01:25] so we need the module and hiera ready [10:01:27] jynus: merging this particular CR doesn't gain us anything as-is. it's functrionality that won't be needed until the next wmfmariadbpy release, and when that release happens, this change will be needed for all relevat profiles [10:01:51] <_joe_> kormat: we can work on that part, it's not hard, I'm happy to help you :) [10:01:56] did you see https://gerrit.wikimedia.org/r/c/operations/puppet/+/622444 ? [10:02:01] they are duplicating it! [10:02:44] mm, ok, i see your point [10:03:06] so we need the bare minimum to promise we have a better, common option [10:03:10] so it doesn't get worse [10:03:25] I am not saying to not refactor, just to not make the problem worse :-D [10:03:33] the bare minimum would be `hieradata/common/profile/mariadb.yaml` [10:03:37] exactly [10:03:48] the rest is wmfmariadbpy-specific [10:03:52] the rest is more or less a proof of concetp on wmf-roots [10:03:54] so it works [10:04:09] we don't need to expand it elsewere now [10:04:22] but we should "proove" the POC works, I think [10:04:26] prove* [10:04:46] then refactor as you wish before deploying everywhere :-D [10:05:19] but that will take quite some time [10:06:11] what we don't want is 20 section_ports.yaml :-D, one per profile, that is what worries me [10:20:29] <_joe_> there is no reason to [10:20:58] <_joe_> for instance, all mediawiki machines run mcrouter, and that means 5 different profiles, and more roles. We have just one hiera definition [10:21:02] <_joe_> per datacenter [10:23:18] indeed, which is why I want to merge soon this (large refactoring can be done later) before cloud does exactly that :-D [10:23:37] my patch has nothing to do with whatever refactoring can be done later [10:23:47] I just added the module and called from a single place [10:24:13] <_joe_> that's ok, we can add a simple profile that will be the start of the refactorings [10:24:29] <_joe_> I'm puzzled by that duplication of code for wmcs though [10:24:39] <_joe_> that seems to be what hiera is designed to do [10:24:41] _joe_: please check https://gerrit.wikimedia.org/r/c/operations/puppet/+/620722 and see if there is something wrong with the patch by itself [10:25:22] <_joe_> jynus: the values in https://gerrit.wikimedia.org/r/c/operations/puppet/+/620722/22/hieradata/common/profile/mariadb.yaml are the same across all of production? [10:25:26] high level, there could be bugs [10:25:33] _joe_: unclear [10:25:38] not sure if only for profile [10:25:42] or for all [10:25:55] we can move it later I guess is your suggestion? [10:26:32] <_joe_> jynus: my point is - if it needs to vary per-role, maybe it should not be in common/ [10:26:43] <_joe_> but it's also ok to have a default [10:26:44] no, definitely not per role [10:26:57] but I mean that I don't know yet how global it is [10:27:01] <_joe_> ok [10:27:33] mw + misc, mw + misc + produiction cloud, or "everything, including vps" [10:27:57] open to move it depending on that [10:28:17] <_joe_> jynus: I think the code is ok, maybe we might want to use a deeper data structure later, but for now I think it's ok [10:28:27] deeper? [10:28:33] what do you mean? [10:29:00] <_joe_> depending on how data changes between different installations [10:29:07] <_joe_> but don't worry, I think this code is ok [10:29:31] <_joe_> then kormat can just create a profile::mariadb::common that looks up the same hiera key [10:29:35] so the main issue is, this file will probably be always correct [10:29:38] <_joe_> and that gets required by all other profiles [10:29:53] but maybe not all values should be correct on all envs [10:30:03] e.g. maybe cloud should not allow x1 [10:30:10] <_joe_> so you might only want to make some values available depending on env [10:30:15] but maybe the should not be on hiera, but on the profile [10:30:25] the file is always "true" [10:30:33] but some values should not allowed [10:30:39] <_joe_> ok, you can have this as a general hiera object, then filter with an ad-hoc allowlist [10:30:54] yeah, but more like a profile logic, right? [10:31:00] e.g. an allow list loaded by the profile [10:31:05] or something else [10:31:14] ? [10:31:43] so my discussion with kormat was that of, I am sure refactoring will be neded, but that is out of scope for me :-D [10:32:08] as long as the module is "usable" [10:36:28] or you know, vote -1 on https://gerrit.wikimedia.org/r/c/operations/puppet/+/620899/13 if he wants to do refactoring first [10:38:28] kormat _joe_: what do you think about having a regex on parameter type? [10:38:52] I don't like it, but if you think it is "standard puppet" I can move the check up there [10:38:54] <_joe_> leave it for later, it's not needed right nor [10:39:08] <_joe_> *w [10:39:16] you mean the second patch, right? [10:39:40] <_joe_> uh I am definitely not gonna look at that [10:39:42] <_joe_> :D [10:39:58] hey, all faults for jbond42 who was who suggested it :-D [10:40:06] I just did a normal regex check on code [10:40:10] 0:-DDDD [10:43:43] jynus: i think using Pattern for paramater type enforcment is fine. your comment on the CR is that you want to provide a better error message which i think is also fine, however currently you the call to fail dosn;t give an indication on what the correct format is [10:44:06] Yeah, I forgot to add the link [10:44:09] I can do that [10:44:14] but I am more interested on [10:44:23] is that something we do as standard? [10:44:38] because if it is, I don't mind you suggestion, it is just new to me [10:44:39] <_joe_> use Pattern[] in the type declaration? [10:44:42] <_joe_> yes. [10:44:43] yeah [10:45:03] I don't know, a regex there on a traditional language seems ... strange [10:45:15] <_joe_> joe@wotan:~/Code/WMF/operations/puppet (production=)$ git grep Pattern\\[ | wc -l [10:45:17] <_joe_> 53 [10:45:22] yes i think thats fine in gernel. many of the Std typs use Pattern under the hood [10:45:22] ok, fair enough [10:45:45] *Stdlib types [10:45:51] <_joe_> jynus: think of types in puppet as input validation vectors more than actual types [10:46:07] yeah, I didn't have them in that regard [10:46:09] ^^^ +1 to this [10:46:15] then I will move it to that [10:48:48] <_joe_> sorry I gotta get back to writing documentation, or I'll never finish my own tasks [10:58:50] jbond42++ for adding fulldiff to pcc output. wouldn't have noticed that a generated file looked wrong otherwise: https://puppet-compiler.wmflabs.org/compiler1002/24669/cumin1001.eqiad.wmnet/fulldiff.html [11:01:40] np :) [11:08:19] <_joe_> yeah jbond42 is doing all the nice stuff I planned to do someday, and the bonus is I didn't even had to tell him :)) [11:08:47] <_joe_> now, the game changer would be to make the puppet compiler work on your machine, there are some hurdles to that [11:14:01] lol and yes i know would be a great hack session if we ever get some in person time again :). if not before ill definetly look at it for this ticket https://phabricator.wikimedia.org/T236373 [13:44:02] rzl: I've updated the DC switchover gdoc and wikitech page, for all intents and purposes the swift switchover is now included in traffic switchover \o/ [13:44:18] ack, thanks! [13:44:53] np [13:45:49] godog: looks good -- should we just skip the 15:00 line in the Monday timeline then, and go straight from active-active service depool to cache depool? [13:46:52] rzl: +1, sounds good to me [13:48:00] ema: ^ okay by you if we move the traffic step from 16:00 to 15:00 on Monday? [13:48:34] gives us a little extra time if we turn out to be behind schedule, too :) [13:48:40] rzl: not only ok, better [13:49:01] it potentially lowers my TTFBeer [13:49:02] 👍 [13:49:09] haha [13:49:33] your MTTBeers also, it's good for metrics all around [13:49:39] 🍻 [13:50:02] MTBBeers? my coffee ingress rate is a little low [13:51:30] Between, yeah :) [15:30:03] anyone looking at the icinga configuration errors? [15:30:13] ahh go.dog just posted, nvm :) [15:33:32] yeah will revert now [16:30:12] hey bstorm andrewbogott o/ could you have a look at https://phabricator.wikimedia.org/T260688 please when you have a moment? [16:35:47] yep, looking [16:36:49] herron: you don't know by chance which db it's trying to access? [16:36:56] maybe it's in the error message and I'm missing it [16:37:11] let's see [16:41:30] looks like cloudcontrol100[345] and cloudcontrolNNNN-dev, the alerts just cleared I'm guessing you did something? [16:41:46] sorry, I don't mean which host I mean which db [16:41:54] and I didn't do anything :/ [16:43:02] ah, nevermind, the grant is on *.* [16:44:40] yeah I'm not seeing a db specified in the check [16:47:38] should be fixed with https://phabricator.wikimedia.org/T260688#6413426 [16:47:44] (if it wasn't somehow magically fixed already) [16:48:29] cloudcontrolNNNN look good now, forcing checks again on cloudcontrolNNNN-dev [16:48:48] andrewbogott: we should change ` critical => true,` [16:49:01] I can put up a patch for that [16:49:03] hm, yes we should [16:49:12] I will do that [16:49:17] Ok :) [16:49:51] looks like cloudcontrol200[1,3,4]-dev are not working yet [16:51:16] bstorm: I don't think it's set, should default to false [16:51:20] herron: getting paged for this? [16:51:44] It's in modules/profile/manifests/openstack/codfw1dev/galera/monitoring.pp unless I need to git fetch again :) [16:51:46] andrewbogott: getting a buster icinga server ready for cutover [16:52:34] * bstorm runs git fetch again [16:53:23] oh, huh, it's critical for codfw1dev but not for eqiad1, wonder why I did that? [16:53:56] I think it's because I fixed the one in eqiad1, but I forgot to check on codfw [16:54:14] that seems likely [16:54:29] I only saw it because of the alert info being codfw [16:54:44] I should have been more clever with `git grep` when I was trying to find them all [16:54:44] https://gerrit.wikimedia.org/r/c/operations/puppet/+/622613 [16:56:17] As for the check failing, the targeting of hiera maybe? [16:56:38] herron: all clear now? [16:57:29] * bstorm logs into an alert* server out of curiosity [16:57:47] andrewbogott: yup! all the galera checks green now [16:57:52] thanks much andrewbogott bstorm ! [16:57:52] cool [16:57:55] huh cool :) [16:58:26] I wonder why it went red in the first place, honestly, if this is just a central reporting host and not a new icinga [16:59:46] Well, as long as it's all good :) [17:01:40] ah, it's a new icinga host as well [18:47:16] hi! I wanted to discuss about keeping track of updates of a backported Debian package (from testing to buster) and if there is a standard practise that I am missing [18:47:38] so I "maintain" the backport of dnsdist from testing to buster and the package in testing was updated [18:48:14] that got me thinking: I actually don't track updates of the package on the testing branch and the only reason I saw it was that because well, I explicitly set out to do that [18:48:39] is there a common way of tracking updates other than manually checking to see if there is one? [18:49:47] note that the package was manually backported on deneb and the reason I care about this is that in case there was a security update that was applied to testing but not the backport (because I didn't know about it) [18:52:05] it's unfortunate that you're asking this on a day Moritz is out because he probably has a good answer :) [18:52:11] haha [18:55:43] I am sure there is a better way than requests + BeautifulSoup to compare the version at https://packages.debian.org/bullseye/dnsdist :) [19:17:47] <_joe_> sukhe: you can download the apt archive for testing, and query it [19:18:26] <_joe_> I mean by setting it up as a separate source in sources.list + some pinning to avoid ever seeing the buster packages :) [19:18:38] <_joe_> but yes, ask the true debian devs here :D [19:20:01] _joe_: ah, I see what you mean [20:54:00] sukhe: you can sign up for alerts/notifications at tracker.debian.org [20:56:37] https://release.debian.org/testing-watch/ [20:58:12] I think that might also notify you about changes to unstable, but you can probably filter those out pretty easily [21:13:05] legoktm: wow, thanks! this is great [21:14:16] :D