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

Feature/gitlab ci #2252

Open
wants to merge 10 commits into
base: next-hypre-outerloop-cuda
Choose a base branch
from
Open

Conversation

adrienbernede
Copy link
Collaborator

@adrienbernede adrienbernede commented Mar 8, 2021

This PR adds a basic CI implementation to test BOUT-dev on Gitlab LC GPU platform.

This is a basic implementation, some things would need to be improved. In particular:

  • I duplicated the config-bout-gpu.sh and modified it so that it is more generic, and extracted the CMake configurtion in a .cmake file that is invoked on command line.

The reason why I duplicated this script is because I didn't want to break the behavior for those using the original version. But it could in fact replace the original scripts.

  • More tests, build targets, or running on other machines are possibles steps forward.

For now this is only running on Lassen. But it runs on GPUs, which was the main goal here.

@adrienbernede adrienbernede changed the base branch from master to next-hypre-outerloop-cuda March 8, 2021 18:05
@adrienbernede adrienbernede self-assigned this Mar 8, 2021
Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

Most of the code seems to be able to run only on a very specific host.
I would thus prefer if you could hide it, I think /.gitlab/lassen/ would be a possible place ...

Also, it would be nice if the job would bail out without an error if it not run on a system that has the required features. I tried to run it on IPP's gitlab and the job is stuck, so it seems I have to cancel it, which implies it will be reported as a failed build.

.gitlab-ci.yml Outdated Show resolved Hide resolved
.gitlab-ci.yml Outdated Show resolved Hide resolved
@ZedThree
Copy link
Member

@dschwoerer A bit of background for this: this is essentially for testing the GPU branch on LLNL's machines. It won't be running automatically, but will required a member of the LLNL Github organisation to post LGTM to trigger the build (an unfortunate choice of keyword, as it's easily confused with a review!)

It is a good point about this being quite specific to that setup. @adrienbernede is the file required to be called .gitlab-ci.yml? Currently we're using Github Actions, but we've already had to move away from Travis CI, so it's entirely plausible we'll end up moving to Gitlab CI at some point in the future and it would be unfortunate if there was a clash in config files.

@dschwoerer
Copy link
Contributor

Ah thanks, I assumed it would be just pulled to a gitlab instance, where the tests would be automatically triggered.

If the setup is more complicated then a different name might be nicer, but the current setup is not nice, as .gitlab-ci.yml is the default file for gitlab to look for tests, so if I fork that branch to a gitlab instance, it will always trigger a run, without any changes on my side (assuming CI/CD is enabled in the gitlab instance). The run will not be able to succeed, for me it was a timeout, as no runner matched the requirements, which is regarded as a failure, which is IMHO bad user experience.

I do understand that such a setup needs to be quite specific - and I am perfectly happy with the files being in the repo, I just don't like them being in the tests folder, as I think it is confusing to mix CI instructions with the general tests.

So I think there are 2 issues:

  • location of CI instructions (easy to fix)
  • CI failing unless on a very specific gitlab instance. This is harder to fix, but hopefully should be possible. Unfortunately I don't know gitlab CI well enough to suggest a solution.

@adrienbernede
Copy link
Collaborator Author

@dschwoerer @ZedThree , thank you for reviewing this. I knew little about the context, and your comments help a lot.

Since there was no CI setup for Gitlab, I was acting as if it would only be used on LLNL Gitlab instance. Now, it is possible to make it safe to run on any other instance, it only requires some specific changes.

There are 2 ways to handle a "Gitlab agnostic configuration":

  1. .gitlab-ci.yml is the default location where Gitlab looks for CI setup. It can be changed, but then one needs to specify the custom location in the UI of the Gitlab instance used:
  • Pro: we can have one file per machine, and hide everything in a hidden .gitlab directory (like .github)
  • Con: we, as developers, need to configure Gitlab for each clone of the project on Gitlab. The CI is not automatically found, but that's a one time config per clone.
  1. We can have jobs that depends on the instance name.
  • Pro: we keep using the default configuration location : .gitlab-ci.yml.
  • Con: less straight-forward syntax.

After writing the this, I tend to go for 1. although 2. was may initial preference.

@adrienbernede
Copy link
Collaborator Author

Question for lassen users:

What do you think of the change from
scripts-config/lassen-examples/scripts/config-bout-gpu.sh
to
tests/gitlab/config-bout-gpu.sh and tests/gitlab/ppc64le-gcc.cmake

I duplicated the logic to be free to make it more generic and modular.
Would you guys be willing to move to the new syntax?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants