[07:57:29] We forgot to officially acknowledge that yesterday, but WCQS streaming updater is running on production! mutation topic is getting filled up, getting ready to be consumed by WCQS consumers, once data reload and few mandatory steps are completed [07:59:38] gehel: what's the state of those reloads, btw? [08:00:05] completed! [08:00:18] yay! [08:00:39] the best part is that we can safely deploy on Friday, because nothing yet uses those instances :) [08:02:03] we're still missing the updater [08:13:17] yep, and offsets on mutation topics for each consumer [08:26:00] About TypeAheadSearch migration to Vue 3, how much do we need to be involved? I don't see anything about instrumentation in the specs (https://docs.google.com/document/d/1cfVktyQHmmIbOqpUCznplGpoJa9yl6y4FYWSKBGA3LE/edit?disco=AAAATtLlB30) and that's suspicious to me. [08:44:52] I can't answer that question, but lack of metric instrumentation is suspicious, indeed [08:45:24] (and we lost them in the past due to migration) [08:53:30] we might have to redo that indeed, IIRC the new REST api is already blocking our custom A/B test params [09:02:00] Is anyone selecting a game for the gaming unmeeting today? [09:13:23] I wouldn't say no to board games today, but my recent selections were met with controversy, so I'll abstain ;) [09:15:57] not sure I'll be able to make it but played https://boardgamearena.com/gamepanel?game=loveletter recently and it was fine :) [09:16:14] (not online) [09:30:09] all of this reminds me that in two short weeks I'll finally have a table exclusively available for board games :) [09:30:44] now I'll be able to play Civilisation: Through The Ages, which takes waaay to long to finish in one sitting [09:31:00] (I've had that games for the last 3 years, still mostly unpacked) [09:36:52] a dedicated table sounds great, reminds me when I see my kid starting a puzzle with 1000+ pieces on the dining table [09:39:21] this is what I need: https://odditymall.com/dining-table-hidden-puzzle-game-compartment [09:40:00] looks fantastic :) [10:15:46] dcausse: I lost track on how the producer needs to get onto wcqs instances - do we already have some patch for it waiting? [10:17:25] zpapierski: if you you mean the consumer service running close to blazegraph we briefly talked that it was missing but not sure we did anything, looking at Erik's outgoing patches [10:18:02] doh, yeah - I meant consumers [10:18:04] yes he made a patch: https://gerrit.wikimedia.org/r/c/operations/puppet/+/752737 [10:19:10] checking with PCC [10:19:15] I didn't write down the start time of streaming updater, but kafka cli has an option --to-earliest [10:19:22] so I think we should use it [10:20:00] if the topics were empty I don't see a reason not to do that [10:20:28] they were, we only just created them before the updaters [10:20:52] now, where did I leave that conda session... [10:21:14] ok, got it [10:21:53] actually, kafka cli is what I need [10:21:58] is it available anywhere? [10:22:25] no I doubt [10:22:59] what version of kafka do we use? [10:23:17] no clue :/ [10:24:17] lemme check the consumer code, it might simply reset to earliest by default [10:24:38] thx [10:24:39] Erik's patch is noop, might be missing something [10:25:19] can you look at that? I'll take care of those offsets [10:25:40] ok [10:27:26] https://www.irccloud.com/pastebin/hi809oAM/ [10:27:35] it seems we're good with the offsets [10:28:21] I believe we had the requirement on offsets for producer, where it would make much more sense (we shouldn't ever start out from the earliest) [10:29:03] hmm, what happens when we restart a consumer? it starts again from the earliest? [10:30:26] or do we need to provide correct offsets for each restart? [10:30:45] it's using the "reset" thing IIRC [10:31:08] so it's only triggered if the offsets for the consumer group are not found [10:33:58] auto.offset.reset: "earliest" c.f. https://docs.confluent.io/platform/current/installation/configuration/consumer-configs.html#consumerconfigs_auto.offset.reset [10:44:19] ah, ok then [10:54:04] pcc looks good for the streaming updater: https://puppet-compiler.wmflabs.org/pcc-worker1002/33247/wcqs1001.eqiad.wmnet/index.html [10:54:13] lunch break, but I can merge it when back [10:56:46] thanks! [11:03:27] lunch [11:41:03] errand + lunch [11:41:56] lunch [13:20:11] dcausse, zpapierski: ping me when around and I'll merge https://gerrit.wikimedia.org/r/c/operations/puppet/+/752737 [13:20:46] gehel: ping [13:20:52] pong [13:21:26] do we need to reset the kafka offsets before merging? [13:21:54] gehel: no this should be ok [13:22:23] ok, merging [13:22:47] running puppet on wcqs1001 [13:23:00] dcausse: can i have like 10 mins with you [13:23:09] ejoseph: sure [13:23:31] There's this problem we've fixed before cant seem to remember how [13:24:05] ejoseph: on the elastic plugins? [13:24:16] Yh [13:24:49] To join the video meeting, click this link: https://meet.google.com/isk-oxzo-beu [13:24:50] Otherwise, to join by phone, dial +27 10 823 1083 and enter this PIN: 941 285 997# [13:24:50] To view more phone numbers, click this link: https://tel.meet/isk-oxzo-beu?hs=5 [13:25:41] minor issue with the path to the journal in the updater systemd unit, fix coming [13:48:06] dcausse: if you have a minute to review: https://gerrit.wikimedia.org/r/c/operations/puppet/+/753945 [13:48:50] gehel: sure [13:53:05] gehel: lgtm [13:54:13] running puppet again on wcqs1001 [13:55:41] updater is running on wcqs1001 [13:57:30] gehel: thanks, checking if the metrics are getting through [13:57:54] I don't see any logs ... [13:58:30] Jan 14 13:55:43 wcqs1001 wcqs-updater[17353]: 13:55:43.233 [main] WARN o.w.q.r.u.c.StreamingUpdaterConsumer - Applied batch with too many inconsistencies. RDFPatchResult(expectedMutations=15, actualMutations=1, [13:58:31] no logs no errors :) [13:58:54] metrics are not getting there [13:59:07] no logs on disk, but logs in journalctl [13:59:50] strange... [14:00:58] we don't pass `-Dlogback.configurationFile=` to the updater, so default logback configuration only sends to stdout [14:01:54] complete log WARN (last line of the logs): [14:01:57] Jan 14 13:55:43 wcqs1001 wcqs-updater[17353]: 13:55:43.233 [main] WARN o.w.q.r.u.c.StreamingUpdaterConsumer - Applied batch with too many inconsistencies. RDFPatchResult(expectedMutations=15, actualMutations=1, possibleSharedElementMutations=6, actualSharedElementsMutations=2, deleteMutations=0, reconciliationMutations=0) for StreamConsumer.Batch(patch=ConsumerPatch(added=15, linkedSharedElements=5, removed=0, unlinkedSharedElements=0, [14:01:57] entityIdsToDelete=3, reconciliations=0), averageEventTime=2022-01-09T20:51:42.750Z, batchStartMsgId=6f84aa6d-5c6d-4d4a-9193-12dbf19a5dea, batchStartDt=2022-01-13T15:44:11.601767Z, batchEndMsgId=15a3fe29-317b-42e6-8187-542ceeaf93c5, batchEndDt=2022-01-13T15:44:11.603213Z). [14:02:14] * gehel hopes that dcausse understands what this means [14:02:34] gehel: is that the sole line? [14:03:09] https://www.irccloud.com/pastebin/HZjZF1U7/ [14:03:21] full log above. I'm assuming that the other lines are irrelevant [14:03:30] ok not too bad [14:09:27] greetings! [14:09:51] o/ [14:09:59] o/ [14:10:29] gehel: seems to work, updater metrics are not there but looking [14:10:53] metrics are expected in prometheus? [14:11:11] yes [14:11:17] if that's the case, we need to have puppet run on the prometheus servers as well to refresh the config [14:11:29] should happen after 30', unless there is an issue [14:12:29] it's very possible there are issues the way we select these machines is messy IIRC [14:13:04] I'll open a task for the logback config [14:13:36] thanks! [14:15:00] there are prometheus things missing in the command line [14:15:05] things like -javaagent:/usr/share/java/prometheus/jmx_prometheus_javaagent.jar=9101:/etc/wdqs/wdqs-updater-prometheus-jmx.yaml [14:16:47] not super surprising that we're missing a few things on the first deployment! [14:23:20] ejoseph: errand took a bit longer than expected, but I only need now to finish a meal and I'm around [14:23:35] Ok [14:24:45] not sure I understand how these options can be missing [14:24:57] it's static in puppet: $default_jvm_options = ['-XX:+UseNUMA', "-javaagent:${prometheus_agent_path}=${prometheus_agent_port}:${prometheus_agent_config}"] [14:26:56] they are missing from profile::query_service::streaming_updater [14:28:17] we have too many levels of indirection in our startup scripts. Between puppet, systemd units, bash scripts, ... [14:29:22] :/ [14:33:56] if [ -r /etc/default/wdqs-updater ]; then [14:33:58] . /etc/default/wdqs-updater [14:34:00] fi [14:34:05] wdqs is hardcoded all over the place :/ [14:38:09] * gehel needs to focus on pre-annual planning :/ [14:38:17] dcausse: can you have a look at those startup scripts? [14:38:29] I'm here as well, if I could help with anything [14:38:32] gehel: yes [14:38:46] zpapierski: it's runStreamingUpdater.sh that hardcodes this [14:39:21] I can pick it up [14:40:26] should we completely remove the use of that bash script and generate the appropriate CLI directly in the systemd unit? [14:43:51] I wonder if we could verify on what instance the script resides [14:44:33] zpapierski: what do you mean? it is part of the deployment package, so it is deployed on all wdqs* and wcqs* nodes [14:46:13] hmm, existence of wcqs/wdqs-updater seems to be a good indication [14:47:10] something like this (it's a quick fix, I agree that maybe this script shouldn't be here at all): [14:47:12] https://www.irccloud.com/pastebin/58D64Hne/ [14:49:40] actually I don't know what -r does [14:50:16] ah, exists and is readable [14:50:17] -r = "readable" [14:50:22] that should be executable, though [14:50:38] -x, I think? [14:52:10] yep [14:52:33] nope, we're sourcing it, it does not need to be executable [14:52:44] a, true, missed the space [14:52:56] so yeah - that works? [14:58:21] gehel or anyone else, for the elasticsearch-oss pkg , are we rolling our own pkg or is it the vendor pkg mirrored on our wikimedia repo? [14:58:44] mirrored from elastic [15:00:00] https://github.com/wikimedia/puppet/blob/production/modules/aptrepo/files/updates#L62 [15:08:29] dcausse: https://gerrit.wikimedia.org/r/c/wikidata/query/rdf/+/753972 [15:09:26] zpapierski: QUERY_SERVICE [15:09:36] should there be a default value? [15:09:49] ah, forgot that [15:09:56] yeah, it should be set as well, or only [15:12:49] dcausse: I meant that https://gerrit.wikimedia.org/r/c/wikidata/query/rdf/+/753972 [15:13:12] ok I see [15:13:12] it takes care of the default value and consistency [15:14:26] gehel when you have time let's talk about that puppet PR, guessing https://gerrit.wikimedia.org/r/plugins/gitiles/operations/puppet/+/f2d1e58701c6e1b2147d10e4820bbf411d017f4c/modules/elasticsearch/manifests/packages.pp is the proper place to add openjdk pkg req, but let me know [15:14:46] ok this should work /var/log/query_service links /var/log/wdqs [15:15:25] inflatador: yes, at line 14, add `require => Package['openjdk8'],` (or something similar, the right package name [15:15:58] gehel thanks for confirming, will get a new PR out soon [15:16:18] inflatador: don't send a new PR, amend the current one [15:16:29] inflatador: did you have an introduction to gerrit already? [15:17:40] sorry, amend. And yes, I have done the gerrit thing a few times (git review -R). I think I can manage, but feel free to lash me with a wet noodle if I fail! [15:18:42] be careful what you ask for when it comes to punishing gerrit related failures. Gerrit doesn't forget, Gerrit doesn't forgive. [15:21:45] this is the corresponding puppet patch: https://gerrit.wikimedia.org/r/c/operations/puppet/+/753973 [15:23:33] ╱o╲ [15:27:25] I'm afraid you're too far ahead for me in the ascii art domain, what does it mean? [15:27:35] s/for/of [15:27:35] I was thinking the same thing [15:27:43] though for works too [15:27:53] are you in a tent, inflatador ? [15:28:02] hmm, maybe illuminati? [15:28:07] if not, I have no clue [15:28:50] ¯\_(ツ)_/¯ [15:29:21] weird, https://gerrit.wikimedia.org/r/c/wikidata/query/rdf/+/753972 is not merging [15:29:22] it's a helmet made of hands [15:29:28] I offended gerrit? [15:30:00] ah, ok too hasty, I am [15:34:08] Trey314159 it's supposed to be the "waving emoticon guy", except he is panicking [15:34:23] Although it does kinda look like illuminati or Pyramid Head mebbe [15:37:20] That's exactly the kind of denial a clever member of the Illuminati would make if they accidentally revealed their secret Illuminati ascii art. (≖_≖ ) [15:42:47] inflatador: are you perhaps, in fact, a lizard person? [15:43:57] Actually, I'm more of an axolotl person [15:44:58] ah, more of an amphibian than a reptlie then [15:45:12] (that's the extend of my knowledge of the animal kingdom) [15:45:42] Join us! You get super regeneration powers and access to exclusive IRC emoticons [15:46:27] I would, but that would ruin my application to the lizard community and I feel the are more connected [15:48:41] I guess it depends on whether you want to rule the world, or hang out at an aquarium and look derpy all day. Personally, I choose the latter [15:48:47] https://en.wikipedia.org/wiki/Axolotl#/media/File:Ambystoma_mexicanum_1.jpg [15:48:59] nope, I'm nothing if not ambitious [15:50:48] * inflatador blinks eyes, chews on seaweed [15:56:10] The external gills are aesthetically pleasing, but psychologically disturbing.. lungs on the outside? [16:00:04] and they always look surprised [16:00:16] \o [16:01:00] o/ [16:03:21] o/ [16:22:13] inflatador ejoseph - if you plan to attend today's unmeeting, might I ask you to create an account on boardgamearena.com ,if you don't have one already? [16:22:43] I set mine up last time, we should be good [16:22:58] ah, I missed the last one, so that's ok [16:23:01] your login? [16:23:02] interesting, it seems like the ceo incoming priorities include considering if a tech vs product split is the right way to continue [16:24:13] I strongly support revisiting that decision :) [16:24:21] indeed :) [17:09:58] workout, back in ~25 [17:40:29] and back [17:54:22] OK, PR for puppet stuff is ready if anyone wants to check it out: https://gerrit.wikimedia.org/r/c/operations/puppet/+/753988 [18:16:59] looking [18:23:44] (same) [18:32:27] lunch, back in ~30 [18:34:10] there are actually like 12 ways to declare ordering in puppet, not sure which is appropriate :P [18:39:08] inflatador: FYI it's the change-id in the commit message that ties a commit to a specific gerrit patch. you want to make sure not to nuke that when amending commits, otherwise you'll split off new patches [18:39:55] for ex gehel's comments in https://gerrit.wikimedia.org/r/c/operations/puppet/+/753857 are not visible in https://gerrit.wikimedia.org/r/c/operations/puppet/+/753988/ b/c of the different changeid btw the two commits [18:41:13] when making changes you just need to change whatever files are necessary, `git add` them followed by `git commit --amend` (changing the commit message if appropriate, but always leaving the change-id intact), then `git-review -R` [18:41:31] whereas if instead you were to do a `git reset --hard origin/production` and then make a new commit you're gonna end up with a new changeid and therefore a new patch [18:41:44] ok that concludes my wet-noodling :D [18:44:22] yea gerrit is always an odd one, the learning curve is why it's on it's way out, as much as some like it [18:51:07] As for the actual patch itself, I think I like ebernhardson's approach of just `require => Class['Java']` [18:51:49] it looks like gehel previously suggested adding a "dependency between the elaticsearch package and the openjdk package" so I think the only difference between those two proposals is just whether to require the specific openjdk package or the java class in general [18:52:12] i had considered, but thought it might be annoying when we switch jdk's to have that value all over the place [18:52:29] but given what moritzm said about `profile::java` automatically setting the correct packages, I think the more broad `Class['Java']` makes sense [18:52:31] ebernhardson: ^ [18:53:21] Oh yes, Class[Java] makes a lot more sense ! [19:02:54] Threw up a new patchset with that approach, kicking off a pcc run with `check experimental` [19:03:20] ah actually need to fix the failure first [19:03:45] > Could not find resource 'Class[Java]' in parameter 'require' (file: /srv/workspace/puppet/modules/elasticsearch/manifests/packages.pp, line: 14) on node 62f0235cf794.integration.eqiad.wmflabs [19:05:14] and back [19:06:39] I think we probably want `Class['::profile::java']` not `Class['Java']` [19:07:08] Giving that a try [19:09:26] > Could not find resource 'Class[Profile::Java]' in parameter 'require' (file: /srv/workspace/puppet/modules/elasticsearch/manifests/packages.pp, line: 14) on node 544ab7647bb2.integration.eqiad.wmflabs [19:12:14] according to the generated catalog, it has Class['Profile::Java'] and Class['Java']. Odd :S [19:12:32] from https://puppet-compiler.wmflabs.org/pcc-worker1003/33259/elastic2054.codfw.wmnet/prod.elastic2054.codfw.wmnet.pson, but other prod catalogs should be similar [19:13:40] Yeah, very bizarre [19:14:58] Is there a way to force puppet to execute in a certain order? The problem appears to be that it's installing a plugin package before ES and before java. It's also confusing that installing ES itself doesn't appear to pull in java [19:15:37] inflatador: yes the `Require` that we're trying to do will tell puppet that it must run the required resource before the current resource [19:15:43] inflatador: thats what the require statement does, but there are really two execution orders happening i suspect [19:15:55] there is the order puppet code is run to build up the catalog, and then the order the catalog i executed in [19:16:13] mostly we only worry about the second [19:16:36] but i wonder if we are hitting the first here, that it's not defined yet? [19:16:52] Hmm [19:16:55] it==Class['Java']. But that seems unlikely since we use a require. I dunno :S [19:18:47] Naively I would expect that a `require` would impact both the building up of the catalog and also the execution itself [19:18:58] The docs are only talking about execution though so it's not clear [19:19:20] (And your hypothesis seems very plausible, just seems weird that that would be the behavior, but right now it's the only explanation that really makes sense) [19:19:30] putting the "require" in packages.pp is considered a style violation, where do we need to put it? [19:20:18] my memory, which is always odd with puppet, was that include/require runs the code immediately, and that require also adds an ordering requirement of that thing before the thing doing the require [19:20:34] but that doesn't seem to be happening here, so i'm probably wrong :P [19:22:41] I wonder if there is a debug mode that shows its execution plan or something [19:23:30] inflatador: btw I might be wrong but I think the style violation we ran into yesterday is different [19:23:47] that was us requiring a class from another module with a require statement, whereas this is us adding a require "metaparameter" [19:24:20] pcc seems to think Class['Profile::Java'] is fine, if it's only the rspec tests failing that's because rspec only has what you explicitly give it, We probably have to define a mock [19:24:21] yeah, I wish I hadn't screwed up the git history by deleting the change ID on a few of my commits [19:24:31] https://puppet-compiler.wmflabs.org/pcc-worker1003/33260/elastic2050.codfw.wmnet/index.html [19:25:29] inflatador: have you been introduced to `git reflog` yet? :P [19:25:44] inflatador: for debug mode, usually i use the catalogs produced by the puppet-compiler, but those really only give insight to the second half of execution (resulting catalog and ordering wtihin) [19:25:52] nothing in git is ever lost once committed [19:26:04] https://gerrit.wikimedia.org/r/c/operations/puppet/+/753988/1/modules/elasticsearch/manifests/packages.pp#9 is what it considers a style violation , if using Class['Profile::Java'] works then that's fine [19:26:26] whatever will let us stand up servers again [19:26:31] :) [19:27:21] i do wonder if the plugins package needs the same require statement though [19:27:27] inflatador: yeah the new way is using a relationship metaparameter, per https://puppet.com/docs/puppet/7/lang_relationships.html#lang_rel_metaparameters [19:27:53] "new way" meaning what we have in the latest patchset [19:28:53] * ebernhardson tries to remember how rspec works [19:30:01] ryankemper OK, that seems to make sense [19:30:33] so that 'works' except that our pipeline needs some tests for it? [19:32:03] inflatador: i think so [19:34:30] ebernhardson got it, if I can help on that let me know. Thanks for the puppet-compiler link too [19:41:18] now i remember the rspec fun times, we provide a string to be eval'd prior to each test [20:12:05] so i have the tests passing, but i have to say i still don't understand the ordering problem [20:12:30] ┐('～`)┌ [20:12:40] profile::elasticsearch has a `require ::profile::java` along with creating the elasticsearch class [20:12:49] which should mean the profile::java has to come firs [20:12:55] * ebernhardson sighs :P [20:18:22] doh, i have an extra require in there thatwasn't intended [20:22:58] at a more theoretical level, i wonder if this is violating the rule of modules: "Any class, define or resource in a module must not use classes from other modules" (delegating that to profiles). I suppose it's saying that the right place for this ordering is in the profiles, but the profile already has ordering and it doesn't work :( [20:23:05] https://wikitech.wikimedia.org/wiki/Puppet_coding#Modules [20:26:43] yeah, I think that's where the CI rules for style come in, but I dunno the best solution for that. Should we reach out to a puppet SME? [20:29:00] it couldn't hurt, i suspect that while this might work it will just be more debt later if we don't understand the underlying problem [20:31:06] OK, let me silence elastic2051 for reals and reimage it again so we have a "cleanly broken" server [20:36:32] ebernhardson sorry, just saw your latest patch. I'm gonna go ahead and merge it unless ryankemper objects, that will unblock our work on ES migrations [20:36:43] https://gerrit.wikimedia.org/r/c/operations/puppet/+/753988 this one [20:36:53] inflatador: sure [20:54:45] inflatador: actually, i think i figured it out [20:56:27] ebernhardson cool, I haven't merged yet [20:57:07] inflatador: https://gerrit.wikimedia.org/r/c/operations/puppet/+/754034 [20:57:48] ooh I like that [20:58:04] inflatador: the problem was the order of dependencies, it went profile::elasticsearch::cirrus -> profile::elasticsearch -> [profile::java, elasticsearch, ...]. The call in profile::elasticsearch::cirrus to install the plugins package (our initial source of problems) isn't depending on the other things because they are just deeper down [20:58:21] at least, i hope :) [21:07:21] OK, submitting now [21:22:21] inflatador: https://gerrit.wikimedia.org/r/c/operations/puppet/+/753953 is the revert patch [21:22:34] Merging it as soon as the jenkins returns happy [21:23:26] cool, I +2'd [21:24:26] inflatador: +2 means "ready to merge", so always wait for the jenkins build to come back first :P [21:24:39] ah the failure was just the commit message [21:24:40] classic [21:24:44] commit message style* [21:24:51] oops, footgun city today [21:26:16] inflatador: oh and to be clear the jenkins build is a test build, so a change can theoretically be merged before the jenkins test build finishes and be totally fine, but we use the test build to make sure stuff doesn't blow up [21:26:38] in other words it's not that merging before the test build comes back would break anything by itself, just that without the test build we don't know if it will "compile" properly [21:26:46] * inflatador needs to remember that "slow is fast" [21:27:20] * ryankemper is always forgetting and relearning that same lesson [21:27:46] * ryankemper flashes back to dropping a million log events at his prior job by not waiting enough time between restarts of kafka brokers :P [21:28:36] i wonder why pcc liked that then, which hosts does it fail on? [21:28:45] puppet-merge complete [21:28:54] ebernhardson: I tested it on an arbitrary `elastic` host [21:29:10] FWIW I briefly looked at the production catalog and the oss-elasticsearch seems to be there, so I feel like we're back to the earlier confusion [21:29:18] :( yea [21:29:21] earlier confusion meaning the production catalog implies that the change should be fine yet somehow isn't [21:31:01] maybe we don't need the package dep? I suppose our problem was only with java [21:31:39] That was my first thought on the matter as well [21:31:39] makes the install odd though ... the debian package depends on elasticsearch-oss so it will pull it in. The problem we are solving is that java wasn't available [21:32:08] i guess what i mean is that we are indirectly installing elasticsearch-oss there, but in the end i'm sure thats fine [21:32:49] I'll get that patch up [21:33:03] I think ES has different pkg names? [elasticsearch, elasticsearch-oss] IIRC we are setting a var for that somewhere else based on facts [21:35:25] right but regardless of how the sausage gets made i'm pretty sure that we ultimately are using the `elasticsearch-oss` package on these hosts [21:36:11] so it should be there [21:36:13] (don't quote me on that though) [21:36:28] in other news it's one of these days, i am not sure why gerrit is telling me this: [21:36:35] https://www.irccloud.com/pastebin/LWwxwV29/ [21:36:57] Before making my patch I did a `git fetch && git reset --hard origin/production` so I don't know how it thinks there's like 6 commits being submitted [21:38:04] ryankemper: try `git fetch gerrit` [21:38:40] ryankemper: various repos have different configs, but if you have a separate gerrit repo then `git review` will compare against that. [21:38:48] s/gerrit repo/gerrit remote/ [21:39:08] https://www.irccloud.com/pastebin/Z3gjt3qX/ [21:39:35] I suppose the `.git` is different...but it looks like the commits should be the same [21:39:35] ryankemper: yup, so `git review` will always compare your current state against gerrit/production and not origin [21:40:04] oh I see [21:40:06] most likely `git fetch` is only pulling origin [21:40:09] yup that was it [21:40:19] (i don't know why it's configured such, it's annoying, better is origin with the ssh remote) [21:40:26] never ran into that on my old laptop, weird [21:50:27] that would suggest there is some way to change it back :) I'm not sure, i have a bunch of repos that do that [21:52:57] ryankemper lmk if/when you wanna merge https://gerrit.wikimedia.org/r/c/operations/puppet/+/754042 [21:54:33] inflatador: let's do it, want to hop on a call? i can walk thru the process of disabling puppet on the fleet, should be a good exercise [21:58:54] sure! meet.google.com/ics-srds-esv [22:01:43] ryankemper ^^ [22:51:04] sigh...the test searching for 'África' fails because something between the test files and the browser is eating the accented character. [22:51:14] these things are never fun :P [23:04:44] Puppet run on `elastic2051` seems to have succeeded following the re-image (there's still some final re-image stuff going on but it looks like it will actually succeed) [23:05:09] so looks like https://gerrit.wikimedia.org/r/c/operations/puppet/+/754042/2/modules/profile/manifests/elasticsearch/cirrus.pp is functioning correctly [23:07:50] sweet [23:09:48] and on that happy note, I'm out. Enjoy Payday Friday! [23:13:11] \o [23:13:44] BTW it sounds like from the convo in #wikimedia-sre that the patch would have worked with `Package['elasticsearch']` instead of `Package['elasticsearch-oss']` [23:13:56] And it's probably theoretically cleaner if we do have the plugins resource explicitly depend on elasticsearch [23:14:27] But in practice the current patch is working so I probably won't spend any time seeing if it works with the other way too :P