CI on PR's should build the Docker image

[smoya]

Reason/Context

We only build the Docker image right after creating a new release, where we are building and pushing the image to Docker hub.

In the case there is a bug on the image (or anything that runs on it), we won't notice until then, meaning we could have been merging code that breaks.

This is an example of a bug we could catch most probably way earlier (even though it could not be related with a bug introduced by us): https://github.com/asyncapi/server-api/runs/5127206219?check_suite_focus=true

Description

Consider running CI tests within Docker.

[github-actions]

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

[HarshCasper]

Hi @smoya 👋

Can I work on this?

[magicmatatjahu]

@HarshCasper Hi! Yes, you can :) However, testing is done by this workflow https://github.com/asyncapi/server-api/blob/master/.github/workflows/if-nodejs-pr-testing.yml but we cannot update it, because the source for this workflow is located here https://github.com/asyncapi/.github/blob/master/.github/workflows/if-nodejs-pr-testing.yml Even if you change something on this repo (in the source of that workflow) in future it will be overwritten by asyncapi-bot.

I suggest you to make something like we do with release process -> https://github.com/asyncapi/server-api/blob/master/package.json#L39

So we need to run build of docker image when workflow will run npm run test script :)

I hope you understand.

[HarshCasper]

Hi @magicmatatjahu

Thanks for the general overview!

I wish to propose a slightly different solution: Running Docker build with npm run test might not be conducive since Docker is not pre-installed on macOS and Windows images (reference: actions/runner-images#1143)

We can make a new action on asyncapi/.github which can be tuned to run only on the Server-API repository. We can use Docker's Build & Push action where can easily build our Docker container on PRs and capitalize on caching capabilities and more.

Let me know what you think. It would be just one more YAML (🙃) but something that would simplify the workflow for developers who might not want to build an entire image while testing their code.

[magicmatatjahu]

@HarshCasper You're right! I forgot that it will be also running on all OS. Hmm 🤔 We can create new workflow only for this repository, however in our organization we publish also another tools as Docker Image, so we should enable that workflow for all repositories. As you wrote it should be located in the asyncapi/.github repo.

I think that we can run npm run test:docker script for all NodeJS project and of course If given script doesn't exist we should skip given workflow. However we have also GoLang repositories so I don;t know if we should make that workflow generic to handle also Docker images for Golang projects. We can always have 2 workflows, one for Golang and one for NodeJS.

cc @smoya @derberg You should be interested with that topic.

[HarshCasper]

Hi @magicmatatjahu 👋

This would be an interesting problem with a fairly easy solution. We can create a docker.yml that will house all the Docker build jobs for the organization. Every repository can have its own job, since the image tag, context and the Dockerfile name might be different.

We can add an if: condition (like if: "github.repository == 'asyncapi/server-api') to make sure specific jobs are run only for specific repositories. Creating a single workflow might be a bad idea for all projects because then we would have to deal with the tagging complexity.

[magicmatatjahu]

This would be an interesting problem with a fairly easy solution. We can create a docker.yml that will house all the Docker build jobs for the organization. Every repository can have its own job, since the image tag, context and the Dockerfile name might be different.

I don't like that solution. In this repo and also in studio repo we publish Docker Image running release script, because Docker publish is integrated with semantic-releases. You can check how workflow for releasing NodeJS project looks like in the .github repo. https://github.com/asyncapi/.github/blob/master/.github/workflows/if-nodejs-release.yml#L73 For NodeJS it works well and there are no problems. A better option as I wrote would be to have for each language a different way of publishing docker image and each repo should adapt to it.

We can add an if: condition (like if: "github.repository == 'asyncapi/server-api') to make sure specific jobs are run only for specific repositories. Creating a single workflow might be a bad idea for all projects because then we would have to deal with the tagging complexity.

So as I wrote, we need to contribute to the asyncapi/.github repo and run given script only if it exists and include the ability to tag etc. :) We should wait for @smoya @derberg feedback.

[HarshCasper]

Ah, got it. It's understandable now. Thanks, @magicmatatjahu for penning it down!

[derberg]

I think the best would be to have independent workflow, nodejs/go agnostic that just runs docker build. I do not see other solution. The only problem I see is that we will push workflow that is used by about 5 repos into 60 repos again and there is no easy "skip" mechanism, so these will occupy job runtime a bit.

I'm working now on global action to support "topic" filter per workflow file. So we will be able to say that docker_pr_build_test.yml goes only to repos with docker topic. so maybe what I wrote above is not a concern anymore.

[magicmatatjahu]

@derberg Yeah, but have in mind that for example in this repo we publish Docker Image using semantic-releases (as I remember also event-gateway uses semantic-release to release Image) so that workflows for docker build should run only for PR to test build. I wonder if we should also run tests inside Docker to check if Image is properly builded (like we did in previous job) 🤔

[derberg]

so that workflows for docker build should run only for PR to test build

sorry I didn't get this part

[derberg]

@magicmatatjahu ping 😉

[magicmatatjahu]

so that workflows for docker build should run only for PR to test build -> new workflow should be run only on PR to test if image building is successful or not (of course with deps/code changes) :)

[HarshCasper]

Hi @derberg @magicmatatjahu The project has a generate:docker script to build the Docker image. Shall we just use that for our purpose? Would be a small PR!

[derberg]

best would be to come up already with the "agnostic" approach

[magicmatatjahu]

Handled by asyncapi/.github#147