[01:47:11] 10Traffic, 10Operations, 10Performance-Team (Radar): Determine cause of upload.wikimedia.org requests routed to text-lb (404 Not Found) - https://phabricator.wikimedia.org/T207340 (10BBlack) Yeah I think @Bawolff's explanation seems plausible. If they's a DNS hijacking "transparent" proxy which returns the... [05:50:05] 10netops, 10Operations, 10Patch-For-Review: Renumber office-DC interconnect link - https://phabricator.wikimedia.org/T205985 (10ayounsi) 05Open>03Resolved the re-numbering went as expected, BGP sessions are back up. The failover tests were not done, as the exact links needs to be properly identified on t... [07:47:11] 10Traffic, 10Operations: Document eqsin power connections in Netbox - https://phabricator.wikimedia.org/T207138 (10ayounsi) All the power connections have been imported, note that some data can't be imported such as cable length and cable ID. See https://netbox.wikimedia.org/dcim/power-connections/?site=eqsin [08:10:54] 10Traffic, 10Operations: Deprecate pybal SSH health checks - https://phabricator.wikimedia.org/T111899 (10MoritzMuehlenhoff) Are very ready to deprecate this now? We have disk health checks in place via Icinga for a while now which would warn us about faulty disks. [08:19:05] morning [08:19:17] morning vgutierrez [08:19:19] Krenair: I've just spotted a warning on the package building process that's going to bite us in the ass [08:19:22] I: dh_python3 pydist:220: Cannot find package that provides josepy. Please add package that provides it to Build-Depends or add "josepy python3-josepy" line to debian/py3dist-overrides or add proper dependency to Depends by hand and ignore this info. [08:19:48] hm [08:20:42] it's pretty weird though [08:20:54] josepy it's a dependency of python3-acme [08:21:03] yeah... [08:21:11] and it's provided as a debian package so.. [08:21:58] 10Traffic, 10Operations: Document eqsin power connections in Netbox - https://phabricator.wikimedia.org/T207138 (10ayounsi) About the other links, the only less straightforward interfaces are the server's uplinks as their name can't be derived from the device/port table. I'll keep the task open until at least... [08:22:17] vgutierrez, interesting thing [08:22:27] I just looked at the package I built for testing in deployment-prep [08:22:28] and it's actually installed on certcentral1001 [08:22:31] ii python3-josepy 1.0.1-1~bpo9+1 all JOSE implementation for Python 3.x [08:22:33] it has a dependency on python3-josepy [08:22:42] like directly [08:23:21] really? [08:23:23] Depends: init-system-helpers (>= 1.18~), python3-acme, python3-cryptography, python3-dnspython, python3-flask, python3-openssl, python3-requests, python3-yaml, python3:any (>= 3.3.2-2~), adduser [08:23:42] yep [08:23:49] Depends: python3-acme, python3-cryptography (>= 1.7.1), python3-dnspython, python3-flask (<< 1.0.0), python3-flask (>= 0.12.1), python3-josepy, python3-openssl, python3-requests, python3-yaml, python3:any (>= 3.3.2-2~), adduser [08:24:04] hmm boron package it's slightly different [08:24:19] due to that warning I guess [08:24:55] didn't we change one of the versions of something used to build it all at one point? [08:25:23] debhelper? [08:31:24] also [08:31:42] I built it on ubuntu bionic if that changes anything [08:47:18] 10Traffic, 10DNS, 10GitHub-Mirrors, 10Operations, 10Release-Engineering-Team (Kanban): Github: add verified domain - https://phabricator.wikimedia.org/T207364 (10hashar) [08:47:55] 10Traffic, 10DNS, 10GitHub-Mirrors, 10Operations, 10Release-Engineering-Team (Kanban): Github: add verified domain - https://phabricator.wikimedia.org/T207364 (10hashar) [10:07:47] 10netops, 10Cloud-Services, 10Operations: Consider renumbering Labs to separate address spaces - https://phabricator.wikimedia.org/T122406 (10aborrero) Agreed. [10:19:32] vgutierrez, so [10:19:43] tasks for the open commits [10:19:55] investigate weird packaging warning [10:20:18] get brandon to +1 the authdns commit maybe? [10:20:27] yup [10:26:00] 10Certcentral: Investigate weird packaging warning - https://phabricator.wikimedia.org/T207371 (10Krenair) [10:27:01] 10Certcentral: Add simple script for account creation - https://phabricator.wikimedia.org/T207372 (10Krenair) [10:27:25] 10Certcentral, 10Patch-For-Review: Add simple script for account creation - https://phabricator.wikimedia.org/T207372 (10Krenair) a:03Krenair [10:28:44] 10Certcentral: Remove maximum version dependencies - https://phabricator.wikimedia.org/T207373 (10Krenair) [10:28:50] 10Certcentral: Remove maximum version dependencies - https://phabricator.wikimedia.org/T207373 (10Krenair) a:03Krenair [10:31:12] 10Certcentral: Check for expired/outdated certs in the main loop - https://phabricator.wikimedia.org/T207374 (10Krenair) [10:32:18] vgutierrez, uh https://gerrit.wikimedia.org/r/#/c/operations/software/certcentral/+/457933/8/certcentral/cli.py is a certcentral commit [10:32:35] it's really geared more towards the idea that we would make certcentral available for others to use [10:33:21] it just uses the existing acme_requests.ACMEAccount.save function which is incompatible with our puppet setup [10:33:41] maybe we can provide both appraoches? [10:33:45] *approaches [10:34:33] dump the account details in stdout (that would be suitable for our puppetization), or persist in in disk with the .save() method [10:35:17] so provide a param to call the save function? [10:35:20] that wikimedia doesn't use [10:35:23] indeed [10:35:35] or the other way around [10:35:41] I don't care about what's the default [10:36:01] but I think it could be useful for both words (WMF and outside) [10:36:06] *worlds [12:25:51] vgutierrez, so I fixed that, probably [12:26:11] when we get the authdns commit in we should decide which domain to test on [12:26:31] I don't know if there's any point using a test domain for it, should be safe to try to get a cert for a domain we'll actually use right? [12:56:06] Krenair: I'd do the test with pinkunicorn.wm.o [12:56:50] eh [12:56:51] ok [12:57:42] it won't hurt/break anything and it's in use [12:57:58] On an entirely unrelated note, I hate git submodules. [12:58:12] Mainly when other people are changing ones I don't care about [13:01:58] vgutierrez, tempted to also throw in *.pinkunicorn [13:05:25] vgutierrez, yay or nay? [13:06:04] agreed re: submodules, I've been hating them slightly less now that git grep can be configured to recurse in submodules [13:07:24] godog, oooh how? [13:08:11] git config submodule.recurse true # git >= 2.14 [13:08:23] ok, the authdns+CC commit! [13:09:26] godog, I have 2.17 [13:09:35] godog, also <3 [13:11:30] heheh you're welcome Krenair [13:11:54] 1) Can we get rid of the hardcoded IPs in hieradata/common.yaml and use the same pattern for ferm+ssh that the authdns server role uses for itself for authdns-update? [13:12:21] that would be ideal wouldn't it [13:13:08] [13:13:42] basically, replace "profile::authdns::certcentral_target::certcentral_hosts" with "certcentral_servers" that's just an array of the two hostnames. [13:14:17] does it need to be global? [13:14:21] i.e. non-namespaced? [13:14:33] good question! [13:14:45] it'll only get used in that class [13:14:47] but if it's not, it doesn't need to be common.yaml either [13:14:53] so pick a path :) [13:14:56] true [13:15:40] so yeah currently it's even used like a proper profile hieradata param: [13:15:43] $certcentral_hosts=hiera('profile::authdns::certcentral_target::certcentral_hosts') [13:16:46] so we should be able to place it there I think? in hieradata/common/profile/authdns/certcentral_target.yaml ? [13:17:32] so yeah I guess try moving the data there, and making it an array of the two hostnames [13:18:18] and then the srange line, in the other similar case that's using hostnames instead of IPs, looks like: [13:18:23] srange => "(@resolve((${authdns_ns_ferm})) @resolve((${authdns_ns_ferm}), AAAA))", [13:18:32] where authdns_ns_ferm is the result of a join of the array of hostnames [13:18:58] hieradata/role/common/authdns/server.yaml ? [13:19:34] yeah, that was my example, which uses a different hostname list to authorize ssh between authdns servers themselves [13:22:13] just trying to figure out this security::access::config thing [13:22:31] on to point 2, which is the whole thing I can never remember all the arguments or context on, about the 3 different lists of nameservers! [13:22:44] oh, rewind! [13:23:02] what is security::access::config again? [13:23:12] keyholder? [13:23:15] no [13:23:23] the sudo rules? [13:23:26] the thing that allows us to let certcentral into a labs instance without it being in the relevant ldap group [13:23:45] actually [13:23:51] I was just using gdnsd inside labs to test this [13:23:55] make it labs only? :) [13:24:05] doesn't really solve the problem [13:24:06] but [13:24:15] going forward I expect to have deployment-prep talk to designate instead of my testing gdnsd instance [13:24:18] so I can just remove that resource [13:24:35] ok great [13:24:43] also I just found that we seem to be able to use domains in there (or at least toollabs does so for clushuser) instead of IPs [13:25:01] anyways, so on to point 2 about the lists of nameservers [13:25:27] so there's a canonical list of the "live nameservers" already in global hieradata, which authdns itself uses to find its own set of servers to update data on, etc [13:26:00] which is hiera('authdns_servers') [13:26:08] we are aware [13:26:19] and then there's the 2x lists in the current patch [13:26:27] one of which matches that and one of which is the ns[012] hostnames [13:26:47] vgutierrez, be nice :) [13:27:34] vgutierrez: sure go ahead, explain it to me :) [13:28:03] we did look into referencing the existing list [13:28:10] unfortunately it's a list rather than a string [13:28:24] if it were a string we could have hiera do something like %{hiera('authdns_servers')} [13:28:27] but there's this annoying line in the docs [13:28:41] that valentin found - https://puppet.com/docs/hiera/3.2/variables.html#the-hiera-lookup-function [13:28:55] 'The result of the lookup must be a string; any other result will cause an error.' [13:29:00] thx Krenair [13:29:21] so [13:29:36] how is that possibly true? [13:30:05] that limitation only applies to interpolation in hiera data files [13:30:08] not to puppet code [13:30:10] we have structures/arrays all over our hieradata that are fetched with hiera() [13:30:22] I had that reaction at first [13:30:32] so I guess we have to pivot through puppet code to get that working [13:30:41] without actually duplicating the list [13:30:42] it's when you go some_key: "%{hiera('authdns_servers')}" [13:30:43] oh, you mean you wanted to put the hiera call inside your new hieradata? [13:30:50] indeed [13:31:04] it seemed the simplest way to me [13:31:14] but again I was wrong [13:31:25] so since 'dns-01' and its keys are explicit anyways, you can just kill both of those nameserver lists in the patch's hieradata [13:31:31] and move th ehiera call right into the config template too [13:31:52] _joe_ discouraged that in one CR [13:31:54] so then you have to have a dns-01 config and you can't use our puppet module without it [13:32:05] that's not true [13:32:21] it still relies on the top-level bit being defined in hieradata or it doesn't configure dns-01 [13:32:51] oh it looks like we do have checks in there for challenges being empty etc. [13:32:56] and missing dns-01 [13:33:00] ok [13:33:21] the other layer of this onion, aside from "at least one of these lists shouldn't duplicate the authdns_servers data" [13:33:33] is whether the two lists should be identical for push/verify [13:34:13] I honestly can never remember what I said the time before or what was said the time before every time this topic comes up (I think we're on like round 4 of this now, at least) [13:34:29] but it seems to me at present like they should be the same list from the same source [13:34:43] so we'd do [13:34:53] yup, no problem with that [13:35:04] because ns[012] are virtual ideas that can be re-routed at any time, and may not in any case route the same for CC-verify that they do for the ACME provider. [13:35:08] - sync_dns_servers: <% @challenges['dns-01']['sync_dns_servers'].each do |dns_server| %> [13:35:08] + sync_dns_servers: <% scope.function_hiera(['authdns_servers']).each do |dns_server| %> [13:35:18] or was that the old way of doing function calls in templates, I forget [13:35:32] well I don't know about the syntax detail there without looking myself [13:36:02] but yes, that's the idea, for both lists, and then kill just those sub-stanzas in the patch's hieradata that we don't need anymore (but leave top-level dns-01 definition and its configured script path) [13:36:29] from the previous CR... https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/441991/64/modules/profile/manifests/certcentral.pp@18 [13:36:30] well, the only other thing is, I think the linter will still ding you for directly referencing a global hieradata like that, right? [13:36:36] "Please don't suggest hiera calls from templates. [13:36:39] we just ignore that? [13:36:52] I think this is why the pattern that we ran into in the earlier commit [13:36:59] to do a real.pp matching hiera [13:37:18] realm.pp also has: [13:37:22] $certcentral_host = hiera('certcentral_host') [13:37:23] vgutierrez, I think that was because I was using it to evade one of the puppet style checks [13:37:31] which pulls global hieradata up to a global variable [13:37:46] it seems like authdns_servers fits this same pattern [13:38:05] so I'd promote it to a global in realm.pp, and then just use it as a regular global var in the templates [13:38:13] perfect [13:38:52] I don't know why that's better, but I think it meets the rules and has been qausi-ok'd as a solution anyways [13:38:53] 10Traffic, 10Operations: Document eqsin power connections in Netbox - https://phabricator.wikimedia.org/T207138 (10faidon) Awesome, thanks! No field for Cable IDs or labels is a bit disappointing :( It doesn't look like we can do it with a custom field either, but I'm not 100% sure. We should file an upstream... [13:39:06] s/qausi/quasi/ heh [13:39:39] bblack, so we drop sync_dns_servers from hiera and pull that into the template by accessing the global and not calling hiera? [13:40:56] yes, and same thing for the other list for verify [13:41:29] I think it makes sense for CC-the-software to have two lists, because they may be needed elsewhere, but I think for us we just want identical syncd lists here [13:42:52] the ns[012] routing is fairly static, but they can route differently depending on the source network, and later on at some point ns[012] would be replaced by anycast which makes the situation even vague-er [13:43:33] basically you can't actually know you're verifying against the same gdnsd instances that LE hits. But you can know that you've verified all the ones you claimed to push to, which if correctly configured should affect all public queries [13:44:57] I have a meeting soon, but let me drop in the Bomb while I'm here: [13:45:26] https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/459809/19..20 like this? [13:45:56] almost [13:46:03] authdns_servers twice [13:46:11] we don't need the external_fdqns right now [13:46:16] certcentral ends up being a poor name choice, because it's already the name of a commercial trademarked software that overlaps in functionality confusingly, which means user confusion and/or legal woes [13:46:20] https://www.digicert.com/mpki/ [13:46:24] vgutierrez, what? [13:46:43] so we're going to have to rename things, but I think we can stall doing it a little to get through the current stuff and make it work, and then deal with that next week or whatever [13:46:45] Krenair: set both sync_dns_servers and validation_dns_servers to ::authdns_servers [13:46:54] 10netops, 10Cloud-Services, 10Operations: Consider renumbering Labs to separate address spaces - https://phabricator.wikimedia.org/T122406 (10faidon) 05Open>03Resolved a:03faidon Perfect! As far as I can see, there a few pending tasks, but are or should probably be covered in other tasks. Specificall... [13:46:55] bblack, FFS [13:47:03] yeah, I know :P [13:47:18] bblack: give us a name and it will be done :) [13:47:34] but let's ignore that a few more days and get the puppet shit working [13:47:38] vgutierrez, :| [13:47:40] just putting it out there [13:47:50] I might have not given it the best name but jeez [13:47:53] Krenair: I'm not gonna argue about naming honestly [13:48:06] I like the name you chose [13:48:10] but I see bblack's point [13:48:16] sure and I'm willing to change [13:48:49] it's a PITA, but it's not the end of the world and we know it (REM) [13:49:41] yeah [13:49:51] I'm just not thrilled about you saying bblack: give us a name and it will be done :) [13:49:54] esp w/ digicert being one of our cert vendors, it's hard for the foundation to argue innocent ignorance later heh [13:49:57] did you wanna talk about that first? [13:50:14] talk about it later, worry about the puppet stuff now. I just wanted you to be aware :) [13:50:28] Krenair: oh sorry, I didn't want to bypass you by saying that [13:50:36] it's just I don't care about the name [13:50:45] it's fine by me if you get to pick the new one too [13:51:34] I'm not saying I won't accept one from bblack, it's more the principal [13:51:51] anyway [13:52:03] Krenair: set both sync_dns_servers and validation_dns_servers to ::authdns_servers [13:52:10] we can consensus something anyways. I'm pretty bad at naming as well, and tend to name things functionally, which is exactly how you end up with such a name. [13:52:37] f.aidon was suggesting in another chat that this is a good reason to pick abstract names for projects with no semantic meaning :) [13:52:41] so [13:52:51] was I? [13:52:54] I don't remember that! [13:52:57] but sure, that sounds good :) [13:53:07] yesterday, the discussion of LE's original naming, which was right after bringing this up [13:53:17] we'd be changing the list of verification servers to use the same IPs as are used for SSH [13:53:17] project chocolate etc [13:53:21] ah [13:53:25] does gdnsd listen on those? [13:53:28] it does [13:53:49] sorry, I've been feverish for a couple of days now, can't say that I'm very focused on what I'm saying :P [13:53:53] it does because that's how we icinga-check them independently of ns[012] routing, too [13:54:08] alright [13:56:30] bblack: another CR for you: https://gerrit.wikimedia.org/r/c/operations/puppet/+/468320 [13:56:47] about allowing the same cert to be used by multiple proxies. [13:56:55] bblack, vgutierrez: updated the authdns-CC commit [13:56:58] running pcc against PS21 [13:56:59] I'm not sure how much sense it makes for LE certs [13:57:12] bblack, is it okay if I open a task about the naming? [13:57:47] I guess, scoped to doing the work of the actual rename? [13:58:35] what about deciding the name? [13:59:03] that too, I'm just saying debating naming isn't very tasklike, we probably just hash that out in IRC and then the legwork is more tasky [13:59:10] ok [13:59:14] but whatever, it can be a bullet point there too that it has to happen :) [13:59:35] looking good: https://puppet-compiler.wmflabs.org/compiler1002/13052/certcentral1001.eqiad.wmnet/ [14:00:02] btw [14:00:11] will we have to rename the VMs? [14:00:37] I think it makes sense to respawn them with proper names [14:00:52] and it's a pretty fast thing [14:01:17] BTW, I'm not pretty fond of /etc/certcentral to drop an executable file [14:01:40] /usr/local/bin feels way better [14:02:34] 10Certcentral: Rename this project - https://phabricator.wikimedia.org/T207389 (10Krenair) [14:02:46] true [14:03:56] good point [14:04:08] PS22 [14:05:33] 10Certcentral: Rename this project - https://phabricator.wikimedia.org/T207389 (10Vgutierrez) p:05Triage>03Normal [14:07:11] right [14:07:29] I'm off to campus for a bit, will be back later [14:07:36] ok [14:08:09] we will be in meetings since 16 till 18 WEST [14:08:24] bblack even more :) [14:34:44] 10Certcentral: Rename the Certcentral project - https://phabricator.wikimedia.org/T207389 (10Aklapper) [14:39:18] vgutierrez: so we should be good to deploy the patch? looking again [14:41:52] vgutierrez: I have one final thought: we don't need gdnsdctl execution as root technically. We could limit the scope of possible future sec problems here a bit by having sudo use the "gdnsd" user the dns daemon runs as to execute gdnsdctl. [14:42:16] vgutierrez: assuming that can be made to work with sudo, given 'gdnsd' is likely a nologin user [14:43:07] nologin meaning "has a shell of /bin/false" [14:43:25] I can try it manually from my user and see how sudo works out [14:49:01] yes, it works [14:49:08] I'll amend right quick [14:51:31] https://gerrit.wikimedia.org/r/c/operations/puppet/+/459809/22..23/modules/profile/manifests/authdns/certcentral_target.pp [14:51:32] I'm kind of around for a it [14:51:34] bit [14:52:09] bblack, uh you'll need to change the command it runs too then [14:52:17] so it does sudo -u gdnsd instead of just sudo [14:52:33] oh right [14:54:06] anyways, I did manually test. I temporarily pulled ops out of the nopasswd root stuff (so I don't override with blanket access), and stuff that rule in for user bblack, and was able to do: [14:54:10] bblack@authdns1001:~$ sudo -u gdnsd /usr/bin/gdnsdctl -- acme-dns-01 example.com 0123456789012345678901234567890123456789012 [14:54:13] info: ACME DNS-01 challenges accepted [14:54:49] yeah [14:55:23] at least this way when someone finds a buffer overflow in gdnsdctl it will only affect the dns daemon and not take root with it :) [14:55:48] although [14:55:57] https://gerrit.wikimedia.org/r/c/operations/puppet/+/459809/22..24/ [14:56:09] `dig _acme-challenge.example.com TXT @authdns1001.wikimedia.org` doesn't return anything [14:56:16] oh wait refused [14:56:18] it's not supposed to, it's ok [14:56:37] if, before the default 10 minute timer on that expired, we defined an /etc/gdnsd/zones/example.com zone and hit reload-zones though [14:56:38] I assume it refuses for anything for a domain it doesn't have a zone file on disk for? [14:56:41] the challenge would suddenly appear too [14:56:45] yes [14:57:10] 'only' the dns daemon :D [14:57:23] well there's little way to effectively avoid that :) [14:57:27] yeah [14:58:02] so I think we're done [14:59:40] I think so too [14:59:44] meeting time, but +1'd :) [15:00:07] nice :D [15:01:25] I should edit the response message on the CLI there to just say [15:01:28] Challenge accepted! [15:01:55] :D [15:02:12] late as usual, making one more coffee before I join [16:01:23] back soon [16:28:48] back [16:28:50] vgutierrez, we doing it tomorrow? [16:29:46] good Q [16:29:58] Krenair: friday morning? [16:30:09] I could push it now too, but I'd really like him to be here to look at all related things as they happen, so it depends whether he's done for the day now basically [16:30:13] timezones are awful! [16:30:31] yeah they're awful :) [16:30:45] I'm still here for another 30minutes though [16:31:07] want to give it a stab and see if it seems to basically apply without huge problems? [16:31:19] we can always save testing a cert for later, and I can handle rollback cleanup if necc [16:31:45] sure, let's merge this today, and check tomorrow morning cert issuance [16:31:50] ok [16:32:00] playing with authdns servers on Fridays is not the best idea [16:32:01] :) [16:32:01] so [16:32:11] since the puppetization hits the authdns, I'm going to puppet-disable those ahead [16:32:29] let's see if it rolls on cc1001 and such successfully first, and if it does I'll release those one at a time and see if they work out ok too [16:33:27] ok [16:33:29] they're all disabled now [16:33:43] I'll do the +2 then [16:33:49] go for it! [16:36:45] and it's merged [16:36:47] running puppet in cc1001 [16:36:52] between the CC patch and the authdns patch we have 92 total PS count [16:36:56] what could possibly go wrong? :) [16:40:35] puppet seems happy [16:40:43] but I've spotted a small bug in the config.yaml [16:40:43] accounts: [16:40:43] - id: 6e01c693ed6e9d9a6b5930923ecef104 [16:40:43] directory: https://acme-staging-v02.api.letsencrypt.org/directory [16:40:46] certificates: {} [16:40:52] that naughty tab in certificates [16:42:28] I don't see a tab? [16:42:35] yeah I've got spaces here [16:42:41] but it shouldn't be indented [16:42:44] sorry, the spaces [16:42:45] yeah [16:42:57] one sec [16:42:58] easy fix anyways [16:43:01] I'm seeing the .erb and it doesn't makes sense... [16:43:06] trying one of the authdns to see how it flies there [16:43:28] probably the <%- / -%> related issues and how they can mess with whitespace/newlines? [16:43:42] that's what I'm thinking [16:44:11] oh I see [16:44:26] after the directory line [16:44:34] there's spaces before the next if block [16:44:36] Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Function Call, secret(): invalid secret ssh/authdns-certcentral.pub at /etc/puppet/modules/profile/manifests/authdns/certcentral_target.pp:12:20 on node authdns1001.wikimedia.org [16:44:44] so missing secret, and also why is the public key in secret()? [16:44:46] then it does nothing more before adding certificates: [16:45:03] I didn't catch that before [16:45:05] bblack, originally it was hieradata but apparently all the public keys are in secret? [16:45:11] WTF [16:45:11] so we moved it for consistency [16:45:19] authdns-certcentral.pub [16:45:26] I guess ok if it seems to be standard [16:45:27] in pcc that got changed to authdns_certcentral.pub [16:46:02] is in the private repo as authdns_certcentral and authdns_certcentral.pub [16:46:04] for manual-deploy TLS we tend to only put privkeys in secret and pubkeys in the main puppet repo, but if ssh keys seem to go all-secret I'm ok with following whatever the pattern is [16:46:40] so does the repo need fixing or the puppet code? [16:46:49] I mean I guess puppet code is better at this point [16:47:45] puppet code [16:47:52] damn [16:47:54] secret('ssh/authdns-certcentral.pub') [16:48:00] we missed the whole change to keyholder there [16:48:12] I'm making the CR now [16:48:15] ok [16:48:40] I think it would've been easier to build the hash in the manifest and then ordered_yaml it into a file, rather than this template [16:49:07] but I've uploaded https://gerrit.wikimedia.org/r/468358 which might hopefully fix our config indentation problem [16:49:29] by changing <% end -%> to <% end %> when it is indented [16:49:52] https://gerrit.wikimedia.org/r/#/c/468359/ [16:50:02] alternatively we could unindent all blocks but then we'd have a mess [16:50:47] vgutierrez: yeah shove the patch through and we'll see if it unbreaks the things [16:50:58] assuming jerkins is happy [16:50:58] ack [16:51:10] it is [16:51:23] [ 2018-10-18T16:51:08 ] INFO: Compilation failed for hostname authdns1001.wikimedia.org in environment prod. [16:51:28] pcc nope [16:53:07] ? [16:53:09] I guess that means the labs/private secret paths don't match the prod secret paths? [16:53:23] that's prod.. sigh [16:53:26] https://puppet-compiler.wmflabs.org/compiler1002/13066/authdns1001.wikimedia.org/ [16:53:26] https://puppet-compiler.wmflabs.org/compiler1002/13066/ <- I see no-op? [16:53:39] yep [16:54:17] I'm confused, new the patch is good or the patch fails? [16:54:45] prod fails.. so I guess pcc fails to see the changes? [16:54:51] s/see/detect/ [16:55:04] how did you detect the fail on the unmerged patch? [16:55:19] I see jenkins +1 and pcc no-op [16:55:24] yeah [16:55:30] pcc output says prod is failing [16:55:31] not the change :) [16:55:35] I missread the output [16:55:39] ok, expected! [16:55:40] let's try [16:55:41] indeed [16:55:43] merging it [16:56:42] done [16:57:09] I get a non-failing no-op run, trying a real run [16:58:11] 10netops, 10Analytics, 10Analytics-Kanban, 10Operations: Figure out networking details for new cloud-analytics-eqiad Hadoop/Presto cluster - https://phabricator.wikimedia.org/T207321 (10fdans) [16:58:33]