Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

[IMP] Add Dockerfile and hooks #477

Closed
wants to merge 12 commits into from
Closed

[IMP] Add Dockerfile and hooks #477

wants to merge 12 commits into from

Conversation

yajo
Copy link
Member

@yajo yajo commented Aug 11, 2017

The objective of this PR is to make us able to run the full MQT power in a dockerized, reproducible environment, and make anyone able to plug it quickly and easily into any CI pipeline.

I'm trying to reinvent as less as possible while keeping the Docker logic in place.

I'd love to be able to add oca dependencies in a way that fits better with https://github.com/Tecnativa/docker-odoo-base, although I'd prefer to get some help from MQT pros around.

Of course, after merging, to see the effect, we need to create a https://hub.docker.com/r/oca organization and configure an automatic build there. I configured https://hub.docker.com/r/tecnativa/maintainer-quality-tools for testing purposes, that will rebuild on each push to the PR branch. I could do it myself if I were a PSC here, but I don't feel like I know much what I'm doing here yet.

... and guess what? phantomjs, sass, compass and bootstrap-sass is preinstalled. Deal with it, #471. 😎

@Tecnativa

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what about Pylint/Flake8 and other repositories non OCA?

@yajo
Copy link
Member Author

yajo commented Aug 21, 2017

It's already installed by the travis_install_nightly script.

@pedrobaeza
Copy link
Member

pedrobaeza commented Aug 21, 2017

I'm not talking about installing them. I'm talking about running them.

@yajo
Copy link
Member Author

yajo commented Aug 21, 2017

The usage is added to README. Just use it as usual, but instead of adding env variables as travis does, add them to the docker run command.

@pedrobaeza
Copy link
Member

I still no get it, but maybe it's because my unknown about Docker, but I'm seeing that you run here https://github.com/OCA/maintainer-quality-tools/pull/477/files#diff-3254677a7917c6c01f55212f86c57fbfR64 the command for testing the server, and that's why I'm saying that you should run also pylint/flake8 or none.

@yajo
Copy link
Member Author

yajo commented Aug 21, 2017

AFAICS, travis_run_tests accepts env variables to be configured:

lint_check_disabled = os.environ.get('LINT_CHECK') == '0'
lint_check_enabled = os.environ.get('LINT_CHECK') == '1'
tests_enabled = os.environ.get('TESTS') == '1'
tests_unspecified = os.environ.get('TESTS') is None
transifex_enabled = os.environ.get('TRANSIFEX') == '1'
weblate_enabled = os.environ.get('WEBLATE') == '1'
is_oca_project = os.environ.get('TRAVIS_REPO_SLUG', '').startswith('OCA/')
is_oca_transifex_user = os.environ.get('TRANSIFEX_USER') == \
'[email protected]'

Currently travis sets those in its matrix section and then those get exported to the same command always:

matrix:
- LINT_CHECK="1"
- TRANSIFEX="1"
- TESTS="1" ODOO_REPO="odoo/odoo"
- TESTS="1" ODOO_REPO="OCA/OCB"

script:
- travis_run_tests

So, what you should do now is:

global:
  - OWNER=OCA
  - REPO=server-tools # for example. Not sure if we get these automatically from Travis
  - VERSION=10.0
  - ODOO_REPO=odoo
matrix:
  - LINT_CHECK="1"
  - TRANSIFEX="1"
  - TESTS="1" ODOO_REPO="odoo"
  - TESTS="1" ODOO_REPO="OCB"
script:
  - docker container run -it --rm -e "LINT_CHECK=$LINT_CHECK" -e "TRANSIFEX=$TRANSIFEX" -e "TESTS=$TESTS" -v "$(pwd):/root/build/$OWNER/$REPO:ro,z" oca/maintainer-quality-tools:$ODOO_REPO-$VERSION

Now let me explain the script part by part for the Docker-unaware readers:

  • docker container run -it --rm: Usual method to boot a docker container.
  • -e "LINT_CHECK=$LINT_CHECK" -e ...: We explicitly forward required environment variables to the container. Normal exported variables do not get exported to the container in Docker as they would do in a normal script.
  • -v "$(pwd):/root/build/$OWNER/$REPO:ro,z": We mount the readonly. SELinux-labeled (just in case) repo in the container where it should be, to make the scripts work magically.
  • oca/maintainer-quality-tools:$ODOO_REPO-$VERSION: The image name (currently oca/maintainer-quality-tools does not exist, it could start to exist after we configure an automatic build in the docker hub after merging this PR) and tag (we use tags such as OCB-10.0, odoo-8.0, etc., to get images that already contain the odoo or OCB code, which will speed up the builds).

@pedrobaeza
Copy link
Member

Thanks for the explanations, @yajo.

What do you think, @lasley, @moylop260 ?

I'm thinking that this would be incompatible with travis2docker in the current form.

@moylop260
Copy link
Contributor

Do you have planned change runbot-travis2docker module in order to do compatible with this change?

Because this change will be required a privileged and so on

@lasley
Copy link
Contributor

lasley commented Aug 21, 2017

Because this change will be required a privileged and so on

I don't like the thought of my build images having privileged host access. This is a blocker IMO

@lasley
Copy link
Contributor

lasley commented Aug 21, 2017

I'm pretty sure Travis is spinning up VMs for their sudo builds, which allows isolation of the actual hosts. This is not the case with Runbot, and we definitely can't have those builds being aware of the build host in any way.

@yajo
Copy link
Member Author

yajo commented Aug 22, 2017

I'm thinking that this would be incompatible with travis2docker in the current form.

Well, all .travis.yml files should be upgraded to use the new form, but that doesn't make the old one stop working.

Do you have planned change runbot-travis2docker module in order to do compatible with this change?

I have not looked into that yet, this would serve the purpose to speed up & standarize travis builds, not runbot ones.

However I think we could build a new image that runs the tests (instead of mounting the volumes, use a Dockerfile, then run tests, then commit and push image). Then the runbot could run that same image.

I don't like the thought of my build images having privileged host access. This is a blocker IMO

Travis needs that, but as long as you have a proper docker image, all runbot has to do is boot it. Yes, runbot needs access to a docker socket (as it does now), but you can always use dind if you don't want it to use the main one.

Or we could dump travis, use http://drone.io/ and be happy. 😛

@pedrobaeza
Copy link
Member

I'm thinking that this would be incompatible with travis2docker in the current form.

Well, all .travis.yml files should be upgraded to use the new form, but that doesn't make the old one stop working.

You are not understanding me: if you switch .travis.yml to this new form, runbot won't work as it uses .travis.yml file and travis2docker for creating a dockerized runbot build, and I doubt that this new .travis.yml is compatible with it. And this is why also @lasley talks about privileged host access (not on Travis, but on runbot).

@yajo
Copy link
Member Author

yajo commented Aug 22, 2017

Ah OK, I thought you were talking about Travis only. Any ideas on how to make t2d work with this?

@lasley
Copy link
Contributor

lasley commented Aug 22, 2017

Ah OK, I thought you were talking about Travis only. Any ideas on how to make t2d work with this?

I've recently been experimenting with a Docker inception strategy for my Runbot so that the build workers don't need privileged host access.

In a standard setup where Docker containers build Docker containers, you:

  1. Install Docker on the host
  2. Install Docker on the build worker
  3. Expose the host docker socket in the build container as a volume (-v /var/run/docker.sock:/var/run/docker.sock)
  4. Launch build worker as privileged to host

You can modify this strategy slightly by removing the °3 and °4 steps, which will make the build worker spawn Docker containers inside of itself instead of the host.

I'm running into some major walls on cleanup though - the Docker build worker basically won't stop once you get a few containers running inside of it. Killing it will leave some massive memory, mounted volume and port leaks, so that's not advisable either.

There's also the issue of accessing the containers built by the worker from outside of the worker. I'm using two sets of Traefiks for that, which route around all the network stuff that is Docker-ception.

I dunno how close I am to solving this though.

@yajo
Copy link
Member Author

yajo commented Aug 23, 2017

Well, I recently had to do something similar in our CI. Just spin up a dind (Docker in Docker) container, put it in the same network as your runbot container, and add DOCKER_HOST=tcp://dind:2375 to it. Then the runbot will talk to the dind container instead. No privilege escalation possible, since there is no docker socket mounting.

You will probably have to use a couple of traefiks as you do now, nothing much special. Remember to set overlay2 volume driver in the dind container if you don't want it to eat your whole disk, and you should be done.

@yajo
Copy link
Member Author

yajo commented Aug 23, 2017

If you let me the thought, and I don't mean to offend anyone, IMHO I think the travis2docker approach is wrong by definition.

Travis can perfectly run docker containers, so this should be docker2travis instead: First, have a base image where to inject code and run tests. Then run that image in travis for unit tests and in runbot for human tests. Doing it the other way around is what complicates everything.

@lasley
Copy link
Contributor

lasley commented Aug 23, 2017

If you let me the thought, and I don't mean to offend anyone, IMHO I think the travis2docker approach is wrong by definition.

I dunno if it's necessarily wrong, it's just that it was built with the goal of using the existing OCA workflows from within another build agent.

I think this is instead an example of feature creep at its finest that's now causing us scaling issues, as with any sort of legacy code. There's a point where giant leaps have to be made, and it seems we may be there.

Travis can perfectly run docker containers, so this should be docker2travis instead

Yeah ok so I think we first need to take a step back and identify what exactly it is that MQT is bringing to the table here. We're basically talking about uprooting our entire build system for this, am I correct?

@lasley
Copy link
Contributor

lasley commented Aug 23, 2017

We're basically talking about uprooting our entire build system for this, am I correct?

Ok nope we're not. I traced this Dockerfile a few times, then launched it in my local & I understand more. Basically we're just supplying the Dockerfile that T2D would normally create.

So in this instance, we would simply need a new module runbot_docker that would just build and run Dockerfiles. Interestingly enough, I was already planning a module like this.

@moylop260
Copy link
Contributor

What about supports "sudo" and "docker service" options for travis2docker?

@lasley
Copy link
Contributor

lasley commented Aug 23, 2017

But why? In the case here, we already have a Dockerfile. With the goal of T2D being to create a DockerFile from a Travis file, I feel this would be circular logic creating a Dockerfile from a Travis file of which the sole purpose is to run a different Dockerfile.

@moylop260
Copy link
Contributor

Maybe I'm a little lost.
I will prefer wait to know the full configuration required to run travis and runbot with new changes.

@yajo
Copy link
Member Author

yajo commented Aug 24, 2017

What @lasley tries to say, if I'm not wrong:

What we do now:

  1. There's a .travis.yml file that:
    1. Downloads all code
    2. Installs all dependencies
    3. Installs a new database
    4. Runs tests
  2. There's a travis2docker tool that gets all of that and:
    1. Converts it in a Dockerfile.
    2. Builds a docker image.
  3. There's runbot_travis2docker that:
    1. Gets the image built by travis2docker.
    2. Boots it to get a runbot instance.

What we could then do after having this:

  1. A Dockerfile that:
    1. Downloads MQT
    2. Uses MQT to install all dependencies
    3. Makes different image tags for different odoo versions (ocb-8.0, odoo-8.0, ocb-9.0...)
    4. Pushes all those tags to Docker Hub as oca/maintainer-quality-tools.
  2. A .travis.yml that:
    1. Runs MQT through the prebuilt docker image, just mounting the repo code.
    2. Runs docker container commit $test_container oca/ci-builds:$repo-$pr_number.
    3. Runs docker image push oca/ci-builds:$repo-$pr_number.
  3. A runbot_docker addon that:
    1. Gets notified of new build from Docker Hub (using hooks for that)
    2. Boots an odoo instance from the image that travis built.

As you can see @moylop260, under such scenario, travis2docker should just disappear.

Pros:

  • Instead of having to tweak our tools, we would use them as they were designed for.
  • MQT could now be used in any CI tool, not only travis.
  • All would be waaaay faster.
  • If you want to test locally a runbot instance, you should just do docker container run -p 80:8069 oca/ci-builds:$repo-$pr_number and open http://localhost.
  • And, most important, no more sass install problems! (just joking 🤣) (but really, no more of that).

Cons:

  • We'd have to change all .travis.yml files possibly (or maybe not, we can change current travis script to leverage all of this).
  • New mindset (better, OTOH).
  • Maintain both systems until new one is stable and old one not used (this should not be too long).
  • Odoo/OCB code would be pre-baked into the image, so in case a new commit comes in, you'd have to wait until it gets rebuilt before having that available. Today it's not the case because the code is git-downloaded each time. We have nowadays a tool that syncs OCB from Odoo every night, I imagine we should add a trigger to it that tells the Docker Hub to rebuild MQT image, so in case you really need that last commit, you can wait for tomorrow and rebuild travis, or ask a PSC to rebuild the docker hub image and then rebuild travis when it's done. Every other build would still get very faster, so I think it's better to speed up 90% of the builds than get these last commits for the other 10% or less.

Please remember this PR is WIP, I just thought it's easier to speak above code than above nothing 😊

@yajo
Copy link
Member Author

yajo commented Aug 24, 2017

@moylop260 Now you have a script that would do all the work for you and a sample .travis.yml file on how to use it.

Still unhandled the point on how to put the docker login password as protected to avoid PRs on leaking it, and also the combination between travis and runbot, since those steps would still require some extra things, such as setting up a oca organization in the docker hub and a runbot_docker addon.

I hope that you get the point now at least (although I'd be surprised if this worked as it is right now). *

I'm currently in the task of plugging in MQT with our gitlab-ci, so I'll use it to check it works. If it does, it should work on travis too.


* Edit: Surprise, it built! https://hub.docker.com/r/tecnativa/maintainer-quality-tools/tags/

@moylop260
Copy link
Contributor

no more sass install problems!

Well, we will have sass install problems if we install 2 times the packages (wrong way first). That was the last issue. We had the .travis.yml with

  - ENV: WEB_REPO=1
  - BEFORE_INSTALL: install sass wrong

If we had this same error in any architecture we will have sass install issues again. 😄

I understand the architecture but I'm talking about the configuration.
I mean, we will have a .travis.yml from root repo with Docker image to use, script to run, modules to install, versions to use... fine!
Runbot will have a _______ from ________ to know what image to use, script to run, modules to install... configured.

@yajo
Copy link
Member Author

yajo commented Aug 25, 2017

Runbot will have a _______ from ________ to know what image to use, script to run, modules to install... configured

Could you point me to resources of current runbot implementation please? I didn't dig enough on that part.

@lasley
Copy link
Contributor

lasley commented Aug 25, 2017

If we had this same error in any architecture we will have sass install issues again. 😄

Our Compass/SaaS problems seem to have come back actually. I've been meaning to look into it.

Basically what this boils down to for me is that we're maintaining build compatibility with two different images - none of which are OCA controlled. This adds unpredictability into our builds and regularly causes failures in one system or the other.

We could easily adapt this strategy to support multiple images too, such as maybe a CentOS build. In doing that though, we'd still have a unified file, application, and user hierarchy. This is where our issues are stemming from I think.

@lasley
Copy link
Contributor

lasley commented Aug 25, 2017

Could you point me to resources of current runbot implementation please? I didn't dig enough on that part.

Also I tried on this and honestly I can't find where the modules to test are appended into the run string. I thought it would be here or here, but seem to be horribly wrong.

@moylop260
Copy link
Contributor

Could you point me to resources of current runbot implementation please? I didn't dig enough on that part.

Source code?

@moylop260
Copy link
Contributor

moylop260 commented Aug 25, 2017

In the past, t2d tool have created a bash file to run directly the .travis.yml directly in the host.
This is useful if you don't wish use docker directly.

We can create this piece of code again in order to avoid create 2 configuration files and change all our current CI arch.
I mean, runbot will be execute docker run IMAGE_FROM_TRAVIS_FILE based on a mako template extracting important parameters from .travis.yml

@moylop260
Copy link
Contributor

moylop260 commented Aug 25, 2017

Our Compass/SaaS problems seem to have come back actually. I've been meaning to look into it.

Yeah, because that branch have the error mentioned from #477 (comment) in the .travis.yml

  • screen shot 2017-08-25 at 13 47 43

@yajo
Copy link
Member Author

yajo commented Aug 28, 2017

I'm having some problems on setting this up, I think that it mostly boils down to current MQT being almost absolutely travis-specific. This makes wrapping all in a Dockerfile quite hard, and making that image work in another CI even harder. Not sure where to go from here; definitely having a plug&play docker image for testing (both automated and manual) seems to be the way to go, but making current code work in such a way is quite hard.

Just as an example, pylint is not finding any modules to lint, even when everything is properly set in place...

@moylop260
Copy link
Contributor

We can start with TESTS=1 builds without pylint for now (and without transifex)

@nhomar
Copy link
Member

nhomar commented Oct 26, 2017

  • I do not get the point of not use ENV variables, can you elaborate a little bit on that design decision?
  • This proposal is wiring all to docker, and IMHO this would be agnostic to the OS not pure docker.

It means:

  • python stuff install and configure the OS.
  • Docker is the environment itself.

Regards.

@yajo
Copy link
Member Author

yajo commented Oct 26, 2017

Yikes, I forgot this is still open. I'm gonna close it since I'm not gonna develop it further.

IMO the easiest way to make these tests would be to have a little collection of little Docker images, each one testing one purpose.

This PR is definitely wrong and highly related to the final conclusion (if it ever comes) from OCA/runbot-addons#144, so closing.

@yajo yajo closed this Oct 26, 2017
@yajo yajo deleted the dockerfile branch October 26, 2017 11:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants