Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable support for gitlab #120

Merged
merged 5 commits into from
Mar 23, 2017

Conversation

mathias-luedtke
Copy link
Member

@mathias-luedtke mathias-luedtke commented Feb 12, 2017

This PR includes #119

Example: https://gitlab.com/ipa-mdl/industrial_ci/pipelines/6437124

gitlab.com runs the CI tests on shared Docker-based runners for free.
This means that Docker needs to be run in Docker..
The tricky part was the prerelease test: Docker is run by Docker, which is run in Docker.

The _DO_NOT_FOLD option should be migrated into the STYLE feature, it is just a preliminary hack.

@VictorLamoine, @ipa-bnm: FYI

@shaun-edwards
Copy link
Member

Really excited to see this feature added. Looks like a good number of changes were required. Thanks @ipa-mdl for the effort.

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Feb 13, 2017

Looks like a good number of changes were required

most changes were introduced in 3a9a910, which was not necessary for gitlab support. It was just another clean-up step before copying things..
Only the last 2 commits address gitlab support directly.

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Feb 14, 2017

v0.3.1 executed BEFORE_SCRIPT in the src space, the master executes it in $TARGET_REPO_PATH (which ir read-only) and this patch will run it in / (default docker workdir),

Should we enforce a default working directory in the docker container? And which one?
For travis and gitlab the default working directory is TARGET_REPO_PATH.

@130s
Copy link
Member

130s commented Feb 14, 2017

Sorry I don't have much time these days to look into a huge change like this. And I don't have working Gitlab setting right now to test.

Travis seems happy with this change, which is a sign that we can merge as long as someone can approve :).
It'd be ideal if we can integrate a script to setup another OS where Gitlab is installed and running, so that we can continuously integrate industrial_ci from Gitlab (I'm not pushing this idea for this PR though).

Should we enforce a default working directory in the docker container?

As long as the existing users' script used in BEFORE_SCRIPT work, I'm fine I think.

@mathias-luedtke
Copy link
Member Author

I have set-up a mirror of my fork on gitlab.com, which will get tested.

@130s
Copy link
Member

130s commented Feb 14, 2017

During today's dev mtg, @shaun-edwards offered to test this on his GitLab environment.

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Feb 14, 2017

@shaun-edwards: if you don't use Docker-based runners, you have to prepare docker within the before_script. The rest should work out-of-the-box.

@benmaidel
Copy link
Contributor

This feature comes in very handy. I will also test it on our gitlab environment. Thanks @ipa-mdl

@mathias-luedtke mathias-luedtke force-pushed the feature/gitlab branch 2 times, most recently from db6d673 to 75c7153 Compare March 6, 2017 12:28
@mathias-luedtke mathias-luedtke force-pushed the feature/gitlab branch 2 times, most recently from 9fdfb9f to ac4e8ea Compare March 8, 2017 16:08
@mathias-luedtke mathias-luedtke mentioned this pull request Mar 9, 2017
26 tasks
@mathias-luedtke mathias-luedtke force-pushed the feature/gitlab branch 3 times, most recently from a4d738e to 0321599 Compare March 11, 2017 12:53
@mathias-luedtke
Copy link
Member Author

I have rebase it to the latest master.
Gitlab still works: https://gitlab.com/ipa-mdl/industrial_ci/pipelines/6957440

@mathias-luedtke
Copy link
Member Author

I have sucessfully tested it wih gitlab.com and a local gitlab instance.
@shaun-edwards, @VictorLamoine: do you have time to test this within your set-up?

@VictorLamoine
Copy link
Contributor

VictorLamoine commented Mar 16, 2017

No Mathias sorry 😵 I will, but I don't know when

@@ -0,0 +1,27 @@
#!/bin/bash

# Copyright (c) 2016, Isaac I. Y. Saito
Copy link
Member

Choose a reason for hiding this comment

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

Copyright must be @ipa-mdl ;)

@shaun-edwards
Copy link
Member

@ipa-mdl, looking at this right now. I'll let you know if I have any questions/issues.

@shaun-edwards
Copy link
Member

I wasn't able to get it to work, but I'm sure it's just user error. I mirrored industrial_core for testing purposes. The test fails here. It appear my issue is trying to clone the industrial_ci repo. The script fails with this message:

$ git clone -b feature/gitlab https://github.com/ipa-mdl/industrial_ci
/bin/sh: eval: line 58: git: not found

My confusion is caused by a lack of an install tag in the gilab yaml format. Normally I would just add these lines to the Travis yaml file.

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Mar 20, 2017

The gitlab docker-in-docker feature uses the alpine-based docker image.
Please try to add git as an additional package: https://github.com/ipa-mdl/industrial_ci/blob/feature/gitlab/.gitlab-ci.yml#L8

Or use the docker:git image (recommended)

@shaun-edwards
Copy link
Member

I think I did that correctly here.

@mathias-luedtke
Copy link
Member Author

just change https://gitlab.com/shaun-edwards/industrial_core_mirror/blob/kinetic-devel/.gitlab-ci.yml#L2 to image: docker:git.
It contains docker and git :)

@mathias-luedtke
Copy link
Member Author

git clone -b feature/gitlab https://github.com/ipa-mdl/industrial_ci .industrial_ci

@shaun-edwards
Copy link
Member

This change is working for me. I suggest we add some instruction to the README or advanced instructions for setting up Gitlab. It's basically a file similar to the travis.yml but with a little bit different syntax. Here is a good example - Note the git clone command will be updated once this PR is accepted.

Thanks for all the hard work @ipa-mdl!

@mathias-luedtke mathias-luedtke force-pushed the feature/gitlab branch 2 times, most recently from ecd2714 to d1a6c8d Compare March 21, 2017 09:45
@mathias-luedtke
Copy link
Member Author

@shaun-edwards: Thanks for your test!
I have updated the example config, it now tests industrial_ci with the upstream industrial_ci.

The current environment variable passing is based on the matrix feature of Travis CI.
For Gitlab CI each job gets a script, so an alternate syntax with arguments passing is possible (same as for as for run_ci. For the sake of completeness, travis.sh supports it as well.

@mathias-luedtke
Copy link
Member Author

@shaun-edwards: I cannot use my Gitlabs jobs right now, because of the git clone from master. Can you restart your pipeline to test my latest fixes?

@shaun-edwards
Copy link
Member

Looks like everything is still working: https://gitlab.com/shaun-edwards/industrial_core_mirror/pipelines/7159777

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Mar 21, 2017

Thanks again!
The major difference is that the colored >>> and <<< are included again.
I will merge it tomorrow if @130s is okay with it.

The verbose rosinstall output git limited in #152

@mathias-luedtke mathias-luedtke merged commit 176aba9 into ros-industrial:master Mar 23, 2017
@mathias-luedtke mathias-luedtke deleted the feature/gitlab branch March 23, 2017 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants