Skip to content

Conversation

@BenjaminRodenberg
Copy link
Member

This PR simplifies the spack CI.

  • Uses approach proposed in haampie-spack/ci-example-docker@b98a5e8.
  • Uses Cron job for creating base image
  • Automatically gets dependencies from package.py using --only=dependencies
  • Using jinja and template is not required anymore, since we now use dev-build to build the package directly from the branch

Thanks again @haampie for your talk at SIAM CSE 21! Feel free to add comments or a review to this PR, if you want.

@BenjaminRodenberg BenjaminRodenberg added the enhancement New feature or request label Mar 3, 2021
@BenjaminRodenberg
Copy link
Member Author

74a5dbf might break the tests, but this should be resolved as soon as the upstream PR spack/spack#22059 arrives here.

@fsimonis
Copy link
Member

fsimonis commented Mar 4, 2021

Why do we need the local py-pyprecice spack package? Isn't this enough for this pipeline?

@haampie
Copy link

haampie commented Mar 4, 2021

I usually add add a copy of package.py to the sources so that I can change the build script, add dependencies, etc in a pull request and test it immediately. When merged I create a PR to upstream spack with the new package.py. The downside is that you manually have to sync it with upstream changes to your package.

@ajaust
Copy link
Collaborator

ajaust commented Mar 4, 2021

@fsimonis I think the idea was to have a CI pipeline that one can run on any branch and not just develop..

I personally like(d) the approach using a package.py such that one can set up a proper environment with root spec. I can see that the (old) approach using the template to generate a package.py that references the branch you actually want to test had its advantages. However, our workflow has not been optimal as we also build a new package.py more or less by hand for merging it into the Spack repository. Maybe it would have been worth extending the template approach to generate new package.py for ci and for updating the actual Spack package?

I am no expert on Spack's dev-build feature. If it makes life much easier it could be worth it after all. However, I also read some discussion about either the dev-build or build-dev command of Spack where it was not clear whether the command will be removed at some point or not. I am not sure what the status on that is at the moment. I could not find the discussion either.

Copy link
Collaborator

@ajaust ajaust left a comment

Choose a reason for hiding this comment

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

i added my reviews. I think its basically the question whether dev-build is better than using the template-generated package.py.

Comment on lines 7 to 10
RUN spack --color=always env create --without-view ci /sources/spack/spack.yaml && \
spack --color=always -e ci repo add /sources/spack/repo && \
spack --color=always -e ci install --fail-fast --only=dependencies && \
spack --color=always clean -a
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we have to evaluate here what is better. The approach have specs in the environment or the new one. Why do you create the environment ci using --without-view ci?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied this approach from https://github.com/haampie-spack/ci-example/blob/b98a5e8b1fb7e8aec67707a7eced78efa92b0619/tools/docker/build-env.Dockerfile#L7-L12. So I cannot really give a good reason. I'm not sure, if this is really a satisfactory answer, but I'll try to get through the review with it ;)

spack/spack.yaml Outdated
@@ -0,0 +1,6 @@
spack:
specs:
- py-pyprecice@develop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a risk that it will be misinterpreted because @develop might not point to the actual develop branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think for our testing this "misinterpretation" is exactly what we want: dev-build uses the current version of an arbitrary branch as a patch for py-pyprecice@develop. Then we install/use (the patched) py-pyprecice@develop in all our tests. Does this make sense? If yes: @ajaust feel free to mark this as resolved. If no: Let's try to get to a conclusion here. I should probably also document this somewhere...

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the spack.yaml in e3371fd. I think this new approach is clearer (I hope it works as expected).

@BenjaminRodenberg
Copy link
Member Author

@fsimonis I think the idea was to have a CI pipeline that one can run on any branch and not just develop..

Exactly. I think this is a nice feature, since it allows us to test compatibility of a feature with spack before we merge it into develop.

If we take this feature as a "must", then I really prefer the dev-build over the jinja template solution for the following reason:

  • I think it is more simple (of course subjective)
  • it allows to use the same package.py for submitting to the upstream spack repository and testing
  • I was in the end to remove quite some documentation here
  • I could also remove a lot of code required by jinja in the gitlab action workflows

@BenjaminRodenberg
Copy link
Member Author

Currently the test fails due to changes in bad16b0. As soon as this job is done, I will rerun the test and it is expected to succeed.

@BenjaminRodenberg
Copy link
Member Author

BenjaminRodenberg commented Mar 4, 2021

Currently the test fails, because openssh is missing on the baseimage. This job is currently rebuilding the baseimage including openssh, but this will take a bit longer.

@fsimonis
Copy link
Member

fsimonis commented Mar 8, 2021

I think the idea was to have a CI pipeline that one can run on any branch and not just develop.

With dev-build you can run the pipeline on every branch as develop: "Dear spack, please install this package at the given spec, but use the sources in this directory instead of downloading them."

@BenjaminRodenberg BenjaminRodenberg requested a review from ajaust March 9, 2021 11:26
@BenjaminRodenberg BenjaminRodenberg linked an issue Mar 9, 2021 that may be closed by this pull request
Copy link
Collaborator

@ajaust ajaust left a comment

Choose a reason for hiding this comment

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

Looks good to me know. I assume the dev-build discussion is (somewhat) resolved for the moment.

@haampie
Copy link

haampie commented Mar 12, 2021

Fwiw, you might also be interested in spack/spack#22115

@BenjaminRodenberg BenjaminRodenberg merged commit 5fed8bd into develop Mar 23, 2021
@BenjaminRodenberg BenjaminRodenberg deleted the spack-ci-improvements branch March 23, 2021 17:44
@IshaanDesai IshaanDesai mentioned this pull request Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build_spack sporadically fails due to timeout

5 participants