Wednesday, 2021-07-07

opendevreviewMerged opendev/system-config master: Update gerrit image to v3.2.11
opendevreviewMerged opendev/system-config master: Good riddance to track-upstream and its cronjob
opendevreviewMerged opendev/system-config master: Stop updating Gerrit RDBMS for repo renames
opendevreviewIan Wienand proposed openstack/diskimage-builder master: fedora-container: install dnf-plugins-core
opendevreviewIan Wienand proposed opendev/system-config master: certcheck: also check linaro API
ianwlinaro seems happy, but we've go osuosl images stuck again04:11
opendevreviewMerged openstack/diskimage-builder master: Do not uninstall non-installed packages
opendevreviewIan Wienand proposed opendev/ master: Add paste01
opendevreviewIan Wienand proposed opendev/system-config master: Add paste service
opendevreviewIan Wienand proposed opendev/system-config master: lodgeit: use mariadb connector
opendevreviewIan Wienand proposed opendev/system-config master: Add opendev paste server
*** iury|holiday is now known as iurygregory05:53
opendevreviewMerged openstack/diskimage-builder master: fedora-container: install dnf-plugins-core
*** amoralej|off is now known as amoralej07:08
*** jpena|off is now known as jpena07:46
*** ysandeep is now known as ysandeep|lunch07:46
*** ykarel is now known as ykarel|lunch08:32
*** bhagyashris_ is now known as bhagyashris|ruck08:37
*** whoami-rajat is now known as Guest8909:05
opendevreviewMerged opendev/base-jobs master: Fix docstrings to match job updates
*** ysandeep|lunch is now known as ysandeep09:27
*** rpittau|afk is now known as rpittau09:42
*** ykarel|lunch is now known as ykarel10:14
*** jpena is now known as jpena|lunch11:39
*** ysandeep is now known as ysandeep|afk12:31
opendevreviewMerged opendev/system-config master: certcheck: also check linaro API
*** amoralej is now known as amoralej|lunch12:38
opendevreviewDmitriy Rabotyagov proposed openstack/project-config master: Deprecate os-panko role
*** jpena|lunch is now known as jpena12:43
opendevreviewDmitriy Rabotyagov proposed openstack/project-config master: Remove noop jobs for deprecated os-panko
*** ysandeep|afk is now known as ysandeep13:17
opendevreviewDmitriy Rabotyagov proposed openstack/project-config master: Create repo for Hashicorp Vault deployment
opendevreviewDmitriy Rabotyagov proposed openstack/project-config master: Add Vault role to zuul config
opendevreviewDmitriy Rabotyagov proposed openstack/project-config master: Add Vault role to Zuul jobs
*** amoralej|lunch is now known as amoralej13:36
*** bhagyashris is now known as bhagyashris|ruck13:59
*** bhagyashris_ is now known as bhagyashris|ruck14:24
*** ykarel is now known as ykarel|away15:04
*** ysandeep is now known as ysandeep|dinner15:12
opendevreviewwes hayutin proposed openstack/project-config master: update channel config with new registered nic
opendevreviewJeremy Stanley proposed opendev/git-review master: Ignore unstaged/uncommitted submodule changes
fungithat's wip to demonstrate job failures before i push a revision with the associated fix16:01
*** marios is now known as marios|out16:06
*** rpittau is now known as rpittau|afk16:09
clarkbAnyone know who is runnign stackalytics these days? Seems there may even be two of them with different teams involved? One of the accounts that has external id conflicts appears related to stackalytics and I worry that the bot email addrs won't end up in front of humans16:19 is updated, is/was not updated for a long time16:20
JayFI don't know who runs either one, whcih probably makes that not a helpful answer16:20
*** ysandeep|dinner is now known as ysandeep16:22
clarkbya in this case I'm fairly certain the accounts are for .com based on their age16:24
opendevreviewJeremy Stanley proposed opendev/git-review master: Ignore unstaged/uncommitted submodule changes
fungiand now that revision should succeed all its jobs16:27
*** amoralej is now known as amoralej|off16:30
fungiaccording to a recent rackspace support ticket, nl04 got rebooted yesterday due to a hypervisor host issue16:49
fungilooks like it's currently running though16:50
*** jpena is now known as jpena|off16:52
*** ysandeep is now known as ysandeep|away16:53
*** mgoddard- is now known as mgoddard16:54
clarkbfungi: so it is safe to rebase in the parent repo when the submodules are dirty?16:58
clarkbautostash won't discard changes in the submodule?16:58
fungiseems to be16:58
fungirebasing in the superrepo shouldn't change the state of the submodules anyway (i did not test whether autostash stashes the submodule worktrees but it seems highly unlikely since the rebase wouldn't operate on them to begin with)16:59
clarkbfungi: ok, I left a note on the change to assert the submodule hasn't changed after running git review17:00
clarkbwe don't seem to check that in the tests and that seems important given the regression17:01
fungiwell, the test rebase wouldn't alter anything, it does a reset after, the problem arises when rebase.autostash is set to stash things before rebasing, since the reset causes the stash to be dropped17:02
clarkbfungi: will the reset change the usbmodule though? I think you have to do that separately17:04
clarkbbut that may not be necessary at all since the submodule will hopefully be ignored when rebasing the parent.17:05
clarkbBut I think we should actualyl check the state of the submodule if that is what we are worried about here17:05
fungicould just as easily ask whether the rebase will change the submodule in that case17:05
clarkb(it isn't enough to know git review can push successfully in this case)17:05
fungiautostash should only stash what's being rebased, and a rebase of the superrepo shouldn't rebase submodules17:06
fungiany more than the reset of the superrepo would reset the submodules17:07
clarkbright I understand that, but to properly test the change being made we need to check that I think. Otherwise we could be reintroducing the previous regression to submodule users17:07
fungibut would that be a git-review bug or a git bug at that point?17:07
clarkbit would be a git issue, but considering how painful submodules are to use I doubt we'd get much sympathy and just be told to deal with it.17:08
clarkbsubmodules are known for these types of gotchas17:08
fungiwell, my point is, how do we detect whether git rebased the submodule and then reset it? the end result will be the same as if it was not rebased and not reset17:09
clarkbI think we check if the end result is not the same as that would be what users notice17:09
fungiusers only noticed it for normal repos if they were using rebase.autostash, which we don't (currently) try to turn on in our tests17:10
fungiseems like we would need tests explicitly enabling rebase.autostash to detect that case17:11
clarkboh we didn't add tests for autostash when we fixed the main issue?17:11
fungino, we simply made git-review break when there are unstaged or uncommitted changes17:12
clarkbI guess we just short circuited before running at all but now we want to run in some cases which makes it more complicated to test17:12
fungiso the newly proposed fix rescopes the prior fix to only apply on unstaged or uncommitted changes in the superrepo and ignore submodules, as the rebase should never touch them to begin with17:13
fungibefore the original rebase.autostash fix, the git-review behavior was to catch unstaged or uncommitted changes at push and let it give a more opaque error, at which point it had already discarded the unwound stash from rebase.autostash users17:14
fungiwe fixed that by detecting unstaged and uncommitted changes earlier and giving a clearer error message17:15
fungibut we missed that the solution used to detect those did not ignore submodules (like the rebase/reset did, as evidenced by this being a regression for users with dirty submodules)17:16
clarkbya I think my concern is that git interacts with submodules in unexpected ways. For example why does git diff in the parent show you the delta for submodules? But then apparently reabse ignores submodules unless you pass it extra flags. I just worry that auto stash or rebase will do the unexpected here and then we regress those users again17:17
clarkbrather than telling them to clean up their submoduels first which is the current state right?17:17
clarkbI'm not entirely convinced being told to clean up your submoduels first is the worst thing17:17
clarkbfungi: hrm I'm not able to reproduce the dirty submodule issue either fwiw17:19
fungithe regression tests demonstrate it on patchset 1 of 79986117:20
fungiyou can see i the logs that git diff doesn't report a clean result when there are unstaged or uncommitted changes in the added submodule17:21
clarkbok what I get is +Subproject commit 73f8acdc5c97e068143c86765995c4fb6923ee91-dirty17:21
clarkbI guess it doesn't really diff the submodule but does show you the submodule is dirty17:21
fungiand we're basically just relying on git diff to report there's no change17:21
clarkbRight if we land your change then git-review users will be able to have dirty submodules and push their clean parent repos up. I'm trying to think over if that is actually correct in my head17:22
clarkbif the parent changes rely on the dirty submodule updates to function then the most correct thing would be for the user to correct that.17:23
clarkbHowever in this case git-review is only interested in whether or not the parent changes are mergeable17:23
clarkb(it doesn't care about correctness or CI results beyond is this able to merge when it hits the gerrit server)17:23
fungiprior to the regression, git-review users were able to push changes when dirty submodules were present (and not when the superrepo worktree was dirty), so this regains the original behavior in that regard17:24
clarkbyes, and having a dirty submodule isn't a problem when it comes to mergeability. It would be what ever hash for the submodule you have committed against whatever is upstream. If upstream has changed and downstream has changed then you will need to rebase downstream to address that and I think that would be detected in your chagne still17:25
fungithe rebase.autostash fix didn't really change whether users could push changes with a dirty worktree (that didn't work before it either), just where in the process we detected the case. the fix there was to bail sooner rather than letting it break at push17:25
fungibut in so doing, we made it impossible to push when there was a dirty submodule present17:26
clarkbright and my first impression is that that is a good thing :) but I think it doesn't really matter much to git-review after thinking that through a bit more17:26
fungiwhich used to work17:26
clarkbpersonally for my own use of git I would like git-review to not let me push code if I have dirty submodules because that would likely indicate I have uncommitted changes necessary to make my change work17:27
clarkbBut git-review as a tool doesn't really care about that, just that it can check if the resulting push is mergeable17:27
clarkband we regressed on making that check accurate17:27
fungiwe could certainly add an option to break on dirty submodules, but that would be a new feature since the current behavior there is an unintended change from the prior release17:28
clarkbits also complicated because what you likely want to do si push/git-review the submodule first. Then once you know its commit hash update the child17:30
clarkbgerrit sort of works around this with the auto updating submoduesl though17:30
*** gthiemon1e is now known as gthiemonge20:57
opendevreviewwes hayutin proposed opendev/elastic-recheck master: Run elastic-recheck container
clarkbweshay|ruck: I don't have time to really help with ^ or look into it in depth, but one of the things that e-r has always struggled with is coupling too much of the operational information and the query list with the software itself. You might want to have a more generic dockerfile in the repo that can be reconsumed or FROM'd downstream to avoid making that coupling worse21:35
weshay|ruckclarkb, you are very responsive... :) thanks...  to be clear.. I'm really only expecting some folks in tripleo to look at this with me. I'm just dipping my toes in now21:37
weshay|ruckI'll take this input and run it by the folks tomorrow21:38
clarkbsure, I just always saw the queries being vendored as a bug in e-r and that change looks very site specific too. I know the existing coupling of various concerns in that repo probalby makes this messier too21:38
weshay|ruckclarkb, I totally understand infra not having time for this thing..21:38
clarkbweshay|ruck: also it might make sense to break the chagnes down into things like python3 compat, relative url paths, dockerfile, etc21:38
clarkbjust to make it easier to understand the various bits going on21:39
weshay|ruckhrm k.. ya.. I need to get my own hands dirty here a little bit more to have anything very intelligent to say at this point21:39
weshay|ruckclarkb, appreciate the advice!21:40

Generated by 2.17.2 by Marius Gedminas - find it at!