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

Update dockerfile and facilitate nrel/openfast Docker Hub registry #2124

Merged
merged 62 commits into from
Mar 28, 2024

Conversation

cortadocodes
Copy link
Contributor

@cortadocodes cortadocodes commented Mar 27, 2024

Summary

This pull request updates the existing Dockerfile to facilitate the building of production-ready docker images of OpenFAST which can be pushed to a Docker Hub image repository and used by the community.

Changes

  • Add build args for the base image, CMake options, Make build cores, and timezone
  • Minimise the final image size (down from multiple GB to ~200MB)
  • Add a manually-triggered GitHub Actions workflow to build specific versions of OpenFAST for the linux/amd64 and linux/aarch64 architectures and push them to a Docker Hub repository
  • Add a readme explaining how the new Dockerfile can be used

Related issue

#1851

Impacted areas of the software

Only the Dockerfile in share/docker/openfast_ubuntu/Dockerfile and the GitHub Actions workflows

Test results

  • I've locally built OpenFAST images for versions:
    • 3.1.0 - I then ran it as a container, successfully ran the openfast CLI command with no parameters, and ran a successful analysis with some test input files
    • 3.5.2 - I then ran it as a container and successfully ran the openfast CLI command with no parameters
  • I've used the new GitHub workflow to build and push 3.1.0 and 3.5.2 images to the octue/openfast Docker Hub repository

Pre-release to-dos

If the maintainers are happy with this pull request, we need to do the following things before merging:

  • NREL team:

    • Create the nrel/openfast docker repo
    • Add docker secrets for nrel/openfast access to this repository's GitHub actions secrets
    • Check the updated copyright notice in the Dockerfile is correct
  • @cortadocodes

    • Update octue to nrel in docker workflow (docker repo and location of dockerfile)
    • Move Dockerfile up one level (this will remove the diff that's currently visible for the file, which is why I haven't moved it yet)
    • Document the Dockerfile and docker images outside of the readme I've added

@cortadocodes
Copy link
Contributor Author

@deslaughter we can do either or both

We'll know if the build fails because the workflow will show as a failure. We can even add a badge to the readme (or anywhere else on the internet) that shows if the latest build was successful.

If you wanted to, we could automate GitHub releases of this repository and only release if the docker image builds on merge into main (and any other tests you want to run pass)

@cortadocodes
Copy link
Contributor Author

@deslaughter @andrew-platt

It sounds like this might be the set of workflows we want then:

  • Pull request merge into main: build and push docker image for both architectures
  • Every push to a release candidate branch: test building (but don't push) docker image for linux/amd64

For context, building from scratch for linux/amd64 took a few minutes while linux/aarch64 took 3 hours on GitHub actions. To avoid long waits and extra compute time/emissions for the docker build testing, I've suggested not testing building on non-release-candidate branches or multiple architectures.

@andrew-platt
Copy link
Collaborator

that's a really long build time for linux/aarch64. Testing just the linux/amd64 would be sufficient quick verification that we probably didn't break anything.

@cortadocodes
Copy link
Contributor Author

I've just added the new workflows - let me know what you think

@cortadocodes
Copy link
Contributor Author

I'm fixing a small issue with the test and automatic docker workflows; I'll confirm when it's done

@cortadocodes
Copy link
Contributor Author

cortadocodes commented Mar 28, 2024

I'm fixing a small issue with the test and automatic docker workflows; I'll confirm when it's done

This is fixed now so I think that's everything ready apart from the pre-release to-dos in the PR description

@andrew-platt andrew-platt merged commit 58155a4 into OpenFAST:rc-3.5.3 Mar 28, 2024
57 checks passed
@andrew-platt
Copy link
Collaborator

Whoops! I didn't mean to merge this just yet (and didn't intend to do it as a rebase merge). We can make some PR's to resolve the last questions:

  • Include FAST.Farm
  • openblas library (comment above from @deslaughter)

@andrew-platt andrew-platt mentioned this pull request Mar 28, 2024
27 tasks
@cortadocodes
Copy link
Contributor Author

@andrew-platt are you able to re-open this pull request so the new commits show up?

@andrew-platt
Copy link
Collaborator

@cortadocodes, unfortunately there isn't a good way to re-open the PR. We'll need to do a second PR with the updates (thanks for figuring those out!).

Again, my apologies for accidentally merging this too early.

@cortadocodes
Copy link
Contributor Author

No problem!

@cortadocodes
Copy link
Contributor Author

@andrew-platt here's the new PR: #2139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System: Docker Dockerfiles and recipes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants