#metabrainz

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?

Mineo: oh, that's a neat idea

though, I'm not sure if it'll work.

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

it definitely won't work if you have images for more than one project on the docker host, yeah

so, what actually happens is we use the jenkins project name _and_ build number as part of the container/image name

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"

because we'll actually keep the labels for previous images

maybe that would be more clear with examples (?)

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

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

alastairp: IMHO, docker images should have labels that allow filtering them, see counter-example: docker inspect listenbrainzunittestjenkinsbuildjenkinslistenbrainzunit259_listenbrainz

yes, I agree that adding labels here will make it a lot easier to filter

yvanzo: what's the point of https://github.com/metabrainz/musicbrainz-serve... ? :)

reosarevok: it is self-explanatory :)

I mean, sure, but was it not needed before and now it is? I'm just confused about why :D

maybe he wanted to re-trigger testing?

sorry, yes, I just triggered a build.

yvanzo: "@brainzbot retest this please" will do the same for jenkins, but I don't know about circleci

that is right, but I keep forgetting the exact syntax of this command

me too. there are many commands that I only remember by looking in jenkins config. maybe we should document them somewhere

yvanzo: do you have specific labels when building MBS for development?

alastairp: does listenbrainz really need that many images for each build?

are you asking about jenkins?

alastairp: we have only one base image to run all tests for musicbrainz-server.

currently musicbrainz-tests:v-2020-11

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

yvanzo: test-keydb-A and test-keydb-B are test containers I made to evaluate keydb (replacement for redis), please keep for now

within a test we may have multiple services (although I see in https://github.com/metabrainz/listenbrainz-serv... we appear to reuse the same image in both services, so that's fine)

how does your musicbrainz-tests:v-2020-11 image work? Is that a single image built in november to run all subsequent tests?

what is/isn't in that image?

alastairp: for mbs tests, we have one job in jenkins and one in circleci, services are not ran using separate images/containers.

alastairp: this image contains dependencies and services

so what happens if you change dependencies? Do you have to add a new test image?

how do you include updated sourcecode into the image? Do you build a new one?

Dependencies are just pre-installed, but they are updated by each build.

are the updated during a docker build stage, or in the container before tests are started?

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.

alastairp: in the container

alastairp: sourcecode is copied to the container too

ah, I see now. so in https://github.com/metabrainz/musicbrainz-serve... 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` ?

yes, it's the same for jenkins

this is very different to the docker-compose-based system that we currently have. In my view it adds significant complexity for little value

I think that it's still best to continue using our current system but ensure that we delete images after each build

The musicbrainz-tests approach is more lightweight but not necessarily as flexible as using docker-compose.

alastairp: at least that explains why we don't face the same issues regarding jenkins :)

yes, absolutely

OK, I am going to add labels to ListenBrainz for testing

+1

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

does that make sense to you?

do you mean creating a LABEL named BUILD_DATE?

yes

It's not needed as this is already part of image's metadata.

ah. so perhaps https://github.com/metabrainz/docker-irccat/blo... should be updated?

right

ok, thanks. how about VCS_REF?

hmm, not really

the main point here is that local developers use the same Dockerfile that we use for building the production image

BUILD_DATE is unneeded for irccat but it can be useful when tagging

I think that was a lazy solution.

Build date can be extracted from image as in https://github.com/metabrainz/docker-python/blo...

VCS_REF seems to be more useful at least :)

yvanzo: is the only way to view labels to use `docker inspect` from the host?

can a process in a container see the label?

originally series were designed to support different ordering attributes but we decided to just have one after the schema change iirc

(or maybe before but it was too late to change the sql)

alastairp: not sure, I think host is the only way

yvanzo: for example we do this in LB push.sh: https://github.com/metabrainz/listenbrainz-serv...

this allows us to report the git hash from the app itself

bitmap: it always looked weird to me to have a field name 'number' that actually is free text.

see https://chatlogs.metabrainz.org/brainzbot/music... too

well it was intended to have its meaning determined by the series type, I guess, so it needed a general name

and usually it's a volume number, catalog number, or part number...

alastairp: it would require to break out of the containement

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

yvanzo: yes, I thought so. no problem then

I have a working solution now, based on your and Mineo's suggestions. just preparing PR

bitmap: do you mean a list of "numbers" for items of this series?

"orderingnumbers" maybe?

yes

Alright, would it be fine to drop the useless (and unused) orderingattribute for now?

that sounds fine to me

Ok, thanks, I understand the situation better now. :)

alastairp: but you could create an ENV variable similar to these labels.

it would be available from containers.

yes, either an ENV or the file that we add. you're right

[listenbrainz-server] MonkeyDo opened pull request #1255 (master…jenkins-js-reporter): Use checkstyle eslint reporter format for Jenkins https://github.com/metabrainz/listenbrainz-serv...

alastairp: drumroll ^

Yeah, the report looks really nice !

great!

(on top of reporting errors/warnings separately)

I'll slowly update other jobs to use warnings-ngs instead of regular old warnings too!

so now we need to decide what difference there is between stable, unstable, and failed:

I think we'll be happy with 1 error = fail

so we can differentiate between failed (tests fail) and unstable (tests pass, but there are 2 warnings)

right, so errors -> fail, warnings -> unstable ?

or warnings -> pass ?

And unstable if there are +5 new warnings?

How about under 5 new warnings, pass, over that mark as unstable. Any error = fail.

oh yeah, cool. you can choose based on total number, or new ones

OK, I tried to configure that.

re-running tests

[ESlint] Creating SCM blamer to obtain author and commit information for affected files

[ESlint] -> No blamer installed yet. You need to install the 'git-forensics' plugin to enable blaming for Git.

that looks scary

Huh

Mr_Monkey: cool! the test has failed, because there are errors

so I guess it can say "error introduced by alastairp"

Great

can you fix those 10 errors?

uh, "introduced by alastairp"?

Yeah, that's the next step. I'll do it in the same PR I suppose

yeah, because then we should see the state of that check change from failed to passed

-> class for the afternoon. have a good day

Thanks for setting that up alastairp ! g'luck !

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.

this is why we often get response times alerts for trille

basically users served by trille got very slow responses if they are unlucky enough to hit a load spike

there's also critiquebrainz there

and mbspotify

[listenbrainz-server] alastair opened pull request #1256 (master…jenkins-image-cleanup): Jenkins image cleanup https://github.com/metabrainz/listenbrainz-serv...

alastairp: critiquebrainz-prod logs are probably too large: 100MB in just 24h

ok thank. possibly related to transient errors during yesterday's outage. I can put this on my list to check tomorrow

sorry, no time this afternoon

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;

Alternatively we could just want to pass the builds in case of only ESLint warnings.

(No rush of course)

Mr_Monkey: interesting. I don't know if github has the concept of 3 states

I don't recall ever seeing that (other than "pending" as a transient state)

it looks like there is an option in the github runner to tell it to treat unstable as success:

Oooh, juicy :)

Let's do that then.

please-and-thank-you