alastairp: if you remove images before removing containers, docker should not remove the images (and intermediate layers) that are still used by the containers, maybe that's good enough?
2021-01-28 02815, 2021
alastairp
Mineo: oh, that's a neat idea
2021-01-28 02840, 2021
alastairp
though, I'm not sure if it'll work.
2021-01-28 02846, 2021
alastairp
imagine if I have 4 layers, the last of which churns a lot because of code files changing. we have lots of different services running tests, so ideally I want to remove images based on its name
2021-01-28 02846, 2021
Mineo
it definitely won't work if you have images for more than one project on the docker host, yeah
2021-01-28 02825, 2021
alastairp
so, what actually happens is we use the jenkins project name _and_ build number as part of the container/image name
2021-01-28 02805, 2021
alastairp
so I think I can say "all images whose name includes the project name, and are older than the image that I just built now"
2021-01-28 02822, 2021
alastairp
because we'll actually keep the labels for previous images
2021-01-28 02857, 2021
yvanzo
maybe that would be more clear with examples (?)
2021-01-28 02813, 2021
alastairp
yes, I'm just running some experiments now have clear what happens
see that we have 2 labeled images here from build1 and build2. when build2 runs, we can use it to remove all images that match the reference and are older than build2. When build3 runs, it will remove build2. This will untag the image, we will still need to run a periodic image prune to remove dangling intermediate images
2021-01-28 02858, 2021
alastairp
the filter=reference= could also be done with a label, we don't have labels in most of the python apps, but it seems like a good idea to copy what yvanzo does in some dockerfiles
2021-01-28 02825, 2021
yvanzo
alastairp: IMHO, docker images should have labels that allow filtering them, see counter-example: docker inspect listenbrainzunittestjenkinsbuildjenkinslistenbrainzunit259_listenbrainz
2021-01-28 02856, 2021
alastairp
yes, I agree that adding labels here will make it a lot easier to filter
I mean, sure, but was it not needed before and now it is? I'm just confused about why :D
2021-01-28 02819, 2021
alastairp
maybe he wanted to re-trigger testing?
2021-01-28 02834, 2021
yvanzo
sorry, yes, I just triggered a build.
2021-01-28 02801, 2021
alastairp
yvanzo: "@brainzbot retest this please" will do the same for jenkins, but I don't know about circleci
2021-01-28 02826, 2021
yvanzo
that is right, but I keep forgetting the exact syntax of this command
2021-01-28 02845, 2021
alastairp
me too. there are many commands that I only remember by looking in jenkins config. maybe we should document them somewhere
2021-01-28 02830, 2021
Darkloke has quit
2021-01-28 02814, 2021
alastairp
yvanzo: do you have specific labels when building MBS for development?
2021-01-28 02827, 2021
yvanzo
alastairp: does listenbrainz really need that many images for each build?
2021-01-28 02845, 2021
alastairp
are you asking about jenkins?
2021-01-28 02853, 2021
yvanzo
alastairp: we have only one base image to run all tests for musicbrainz-server.
2021-01-28 02838, 2021
yvanzo
currently musicbrainz-tests:v-2020-11
2021-01-28 02842, 2021
alastairp
there are a few things. one is that we currently run 3 separate jenkins jobs for unit tests, integration tests, and js test. I'm working on combining those
2021-01-28 02859, 2021
zas
yvanzo: test-keydb-A and test-keydb-B are test containers I made to evaluate keydb (replacement for redis), please keep for now
how does your musicbrainz-tests:v-2020-11 image work? Is that a single image built in november to run all subsequent tests?
2021-01-28 02822, 2021
alastairp
what is/isn't in that image?
2021-01-28 02852, 2021
yvanzo
alastairp: for mbs tests, we have one job in jenkins and one in circleci, services are not ran using separate images/containers.
2021-01-28 02846, 2021
yvanzo
alastairp: this image contains dependencies and services
2021-01-28 02844, 2021
alastairp
so what happens if you change dependencies? Do you have to add a new test image?
2021-01-28 02857, 2021
alastairp
how do you include updated sourcecode into the image? Do you build a new one?
2021-01-28 02829, 2021
yvanzo
Dependencies are just pre-installed, but they are updated by each build.
2021-01-28 02807, 2021
alastairp
are the updated during a docker build stage, or in the container before tests are started?
2021-01-28 02814, 2021
yvanzo
We need to build a new image either when dependencies cannot be updated: breaking changes in dependencies installation method, or breaking change in services dependencies.
2021-01-28 02828, 2021
yvanzo
alastairp: in the container
2021-01-28 02805, 2021
yvanzo
alastairp: sourcecode is copied to the container too
2021-01-28 02808, 2021
alastairp
ah, I see now. so in https://github.com/metabrainz/musicbrainz-server/… you specify the base image to run, and circleci runs that image and then copies the code into the container, and then runs the `steps` ?
2021-01-28 02828, 2021
yvanzo
yes, it's the same for jenkins
2021-01-28 02846, 2021
alastairp
this is very different to the docker-compose-based system that we currently have. In my view it adds significant complexity for little value
2021-01-28 02833, 2021
alastairp
I think that it's still best to continue using our current system but ensure that we delete images after each build
2021-01-28 02838, 2021
yvanzo
The musicbrainz-tests approach is more lightweight but not necessarily as flexible as using docker-compose.
2021-01-28 02823, 2021
yvanzo
alastairp: at least that explains why we don't face the same issues regarding jenkins :)
2021-01-28 02833, 2021
alastairp
yes, absolutely
2021-01-28 02852, 2021
alastairp
OK, I am going to add labels to ListenBrainz for testing
2021-01-28 02817, 2021
yvanzo
+1
2021-01-28 02833, 2021
alastairp
however, during development we shouldn't update these labels each time a user builds, because if the build date variable changes it will invalidate the entire cache
2021-01-28 02841, 2021
alastairp
does that make sense to you?
2021-01-28 02812, 2021
yvanzo
do you mean creating a LABEL named BUILD_DATE?
2021-01-28 02820, 2021
alastairp
yes
2021-01-28 02833, 2021
yvanzo
It's not needed as this is already part of image's metadata.
reosarevok: yvanzo: orderingattribute ultimately wasn't intended to be searchable, but I think was left in on accident or in case it was used in the future
2021-01-28 02829, 2021
alastairp
yvanzo: is the only way to view labels to use `docker inspect` from the host?
2021-01-28 02836, 2021
alastairp
can a process in a container see the label?
2021-01-28 02837, 2021
bitmap
originally series were designed to support different ordering attributes but we decided to just have one after the schema change iirc
2021-01-28 02814, 2021
bitmap
(or maybe before but it was too late to change the sql)
well it was intended to have its meaning determined by the series type, I guess, so it needed a general name
2021-01-28 02845, 2021
bitmap
and usually it's a volume number, catalog number, or part number...
2021-01-28 02824, 2021
yvanzo
alastairp: it would require to break out of the containement
2021-01-28 02858, 2021
bitmap
I do think adding a searchable 'number' field makes sense, just not sure about reusing orderingattribute for that. afaict it wasn't intended to be added or at least not publicly documented
2021-01-28 02820, 2021
alastairp
yvanzo: yes, I thought so. no problem then
2021-01-28 02842, 2021
alastairp
I have a working solution now, based on your and Mineo's suggestions. just preparing PR
2021-01-28 02844, 2021
yvanzo
bitmap: do you mean a list of "numbers" for items of this series?
2021-01-28 02826, 2021
yvanzo
"orderingnumbers" maybe?
2021-01-28 02829, 2021
bitmap
yes
2021-01-28 02832, 2021
yvanzo
Alright, would it be fine to drop the useless (and unused) orderingattribute for now?
2021-01-28 02815, 2021
bitmap
that sounds fine to me
2021-01-28 02811, 2021
yvanzo
Ok, thanks, I understand the situation better now. :)
2021-01-28 02834, 2021
yvanzo
alastairp: but you could create an ENV variable similar to these labels.
2021-01-28 02851, 2021
yvanzo
it would be available from containers.
2021-01-28 02807, 2021
alastairp
yes, either an ENV or the file that we add. you're right
oh yeah, cool. you can choose based on total number, or new ones
2021-01-28 02852, 2021
alastairp
OK, I tried to configure that.
2021-01-28 02855, 2021
alastairp
re-running tests
2021-01-28 02835, 2021
alastairp
[ESlint] Creating SCM blamer to obtain author and commit information for affected files
2021-01-28 02835, 2021
alastairp
[ESlint] -> No blamer installed yet. You need to install the 'git-forensics' plugin to enable blaming for Git.
2021-01-28 02838, 2021
alastairp
that looks scary
2021-01-28 02808, 2021
Mr_Monkey
Huh
2021-01-28 02812, 2021
alastairp
Mr_Monkey: cool! the test has failed, because there are errors
2021-01-28 02821, 2021
alastairp
so I guess it can say "error introduced by alastairp"
2021-01-28 02834, 2021
Mr_Monkey
Great
2021-01-28 02842, 2021
alastairp
can you fix those 10 errors?
2021-01-28 02845, 2021
Mr_Monkey
uh, "introduced by alastairp"?
2021-01-28 02857, 2021
Mr_Monkey
Yeah, that's the next step. I'll do it in the same PR I suppose
2021-01-28 02812, 2021
alastairp
yeah, because then we should see the state of that check change from failed to passed
2021-01-28 02832, 2021
alastairp
-> class for the afternoon. have a good day
2021-01-28 02809, 2021
Mr_Monkey
Thanks for setting that up alastairp ! g'luck !
2021-01-28 02811, 2021
zas
bitmap, yvanzo: can you have a look at trille, something is creating load spikes, badly impacting mb containers and rabbitmq performance, unsure what to do. I see critiquebrainz runs there too.
2021-01-28 02833, 2021
zas
this is why we often get response times alerts for trille
2021-01-28 02855, 2021
zas
basically users served by trille got very slow responses if they are unlucky enough to hit a load spike
alastairp: critiquebrainz-prod logs are probably too large: 100MB in just 24h
2021-01-28 02856, 2021
alastairp
ok thank. possibly related to transient errors during yesterday's outage. I can put this on my list to check tomorrow
2021-01-28 02802, 2021
alastairp
sorry, no time this afternoon
2021-01-28 02803, 2021
Mr_Monkey
alastairp: Now without errors but with warnings, the build correctly end in an unstable state. However on the PR on GH that seems to be interpreted as a failed check.Not sure if there's a config change needed on Jenkins to send a non-failure status for unstable builds;
2021-01-28 02803, 2021
Mr_Monkey
Alternatively we could just want to pass the builds in case of only ESLint warnings.
2021-01-28 02812, 2021
Mr_Monkey
(No rush of course)
2021-01-28 02815, 2021
sumedh_p has quit
2021-01-28 02832, 2021
alastairp
Mr_Monkey: interesting. I don't know if github has the concept of 3 states
2021-01-28 02856, 2021
Mr_Monkey
I don't recall ever seeing that (other than "pending" as a transient state)
2021-01-28 02809, 2021
alastairp
it looks like there is an option in the github runner to tell it to treat unstable as success:
