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

Optimize the METviewer Docker image creation process #527

Closed
20 of 25 tasks
JohnHalleyGotway opened this issue Apr 19, 2024 · 2 comments · Fixed by #538
Closed
20 of 25 tasks

Optimize the METviewer Docker image creation process #527

JohnHalleyGotway opened this issue Apr 19, 2024 · 2 comments · Fixed by #538
Assignees
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: code optimization Code optimization issue priority: high High Priority requestor: METplus Team METplus Development Team required: FOR OFFICIAL RELEASE Required to be completed in the official release for the assigned milestone type: enhancement Improve something that it is currently doing

Comments

@JohnHalleyGotway
Copy link
Contributor

JohnHalleyGotway commented Apr 19, 2024

Describe the Enhancement

These ideas arose during development for issue #521. The sonarqube.xml workflow uses Docker to build METviewer prior to scanning the code. Note that that image is built locally and NOT pushed up to DockerHub. However, we could add a metviewer-dev DockerHub repo if/when needed to facilitate additional testing.

METviewer currently contains 3 Dockerfile variations: Dockerfile, Dockerfile.copy, and Dockerfile.apptainer. I recommend making the following enhancements to this setup.

  • Switch from using the deprecated centos7 as the base image to using an updated version of the dtcenter/met-base image (see Create a dtcenter/met-base-metviewer image to make it suitable for METviewer development and testing METbaseimage#23).
  • A LOT of functionality is duplicated in these 3 files. Factor out the commonality into an internal/scripts/docker/build_metviewer_docker.sh script and update the Dockerfiles to call it.
  • Enhance the logic in internal/docker/hooks/build to properly handle the version dependencies between METviewer and the METplus-Analysis tool components.
  • No other automated testing of METviewer exists. Recommend defining a testing.yml workflow similar to the other METplus repositories to build METviewer in Docker and run a series of regression tests on the result. However, this work could also be done in a separate issue -- not done for this issue. Moved to new METviewer#539 issue.

Time Estimate

2 days?

Sub-Issues

Consider breaking the enhancement down into sub-issues.
None needed.

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

Define the source of funding and account keys here or state NONE.

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Select component(s)
  • Select priority
  • Select requestor(s)

Projects and Milestone

  • Select Repository and/or Organization level Project(s) or add alert: NEED CYCLE ASSIGNMENT label
  • Select Milestone as the next official version or Future Versions

Define Related Issue(s)

Consider the impact to the other METplus components.

Enhancement Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding Source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issues
    Select: Repository level development cycle Project for the next official release
    Select: Milestone as the next official version
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added type: enhancement Improve something that it is currently doing priority: high High Priority alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: code optimization Code optimization issue requestor: METplus Team METplus Development Team labels Apr 19, 2024
@JohnHalleyGotway JohnHalleyGotway added this to the METviewer-6.0.0 milestone Apr 19, 2024
@JohnHalleyGotway JohnHalleyGotway self-assigned this Apr 19, 2024
JohnHalleyGotway added a commit that referenced this issue Apr 23, 2024
…cy versions as main_v2.1 rather than develop. Note that DockerHub already uses the hooks/build file to define the dependencies... but the SonarQube workflow actually uses the versions listed in the Dockerifle. Note that issue #527 will clean up and refine this version depedency logic.
bikegeek pushed a commit that referenced this issue Apr 23, 2024
* Per #521, migrate the changes over to a branch for main_v5.1 so that the workflow_dispatch option can become available via GitHub

* Per #521, more work is needed in the DockerHub build hook. For now, switch to building against the develop version of the dependencies.

* Per #521, for the main_v5.1 branch, set the METplus-Analysis dependency versions as main_v2.1 rather than develop. Note that DockerHub already uses the hooks/build file to define the dependencies... but the SonarQube workflow actually uses the versions listed in the Dockerifle. Note that issue #527 will clean up and refine this version depedency logic.
@hankenstein2
Copy link
Contributor

Updating the METviewer Dockerfile
need to remove dependency on CentOS (no more support after next month)
Debian 12 should be the OS
Get the current docker compose file working with debian 12 along with the latest release of Tomcat/Apache/Java that works with METviewer
John H.G. to look for what Java version we are using
Tomcat 9.0.x, 10.1.x work with Java 8.x
Current Tomcat is 9.0.80 we should test with Tomcat 9.0.89
need to update the Tomcat version for Jonathan Vigh and NRIT on ‘urma’ to address security vulnerabilities
See related METviewer SonarQube Dockerfile issue.
Can see DockerHub ‘docker scout’ vulnerabilities here.
Step 1 - Update METviewer Dockerfile in place
Switch CentOS7 to Debian12.
Use Tomcat 9.0.89 and stick with Java 8.x
Step 2 - Switch to using new dtcenter/metviewer-base image
Upgrade to Java 11.x
Decide later if a Tomcat upgrade is also needed.

@hankenstein2 hankenstein2 added the required: FOR OFFICIAL RELEASE Required to be completed in the official release for the assigned milestone label Jun 12, 2024
@JohnHalleyGotway
Copy link
Contributor Author

On the feature_527_met_base_image feature branch, I modified Dockerfile and Dockerfile.copy to use a new dtcenter/met-base-metviewer:v3.2 image. This GHA run before that change took 15m 19s to run the SonarQube scan, while this GHA run after that change only took 4m 16s to run.

So the SonarQube scan time is great improved, as hoped.

JohnHalleyGotway added a commit that referenced this issue Jul 3, 2024
… since it differs for Dockerfile.apptainer. Also modify Dockerfile.apptainer to use the dtcenter/met-base-metviewer image.
@JohnHalleyGotway JohnHalleyGotway linked a pull request Jul 3, 2024 that will close this issue
14 tasks
bikegeek pushed a commit that referenced this issue Jul 8, 2024
* Per #527, try switching to met-base image

* Per 527, remove yum reference since Python 3.10 is already provided in the dtcenter/met-base image

* Per #527, upgrade tomcat to 9.0.89 and java to 17.0.11

* Per #527, add build_metviewer_docker.sh script and update Dockerfile.copy to call it.

* Per #529, update call to build script

* Per #529, migrate changes to Dockerfile

* Per #572, remove geos-3.7.2 unsupported --enable-php configuration option.

* Per #527, switch to using dtcenter/met-base-metviewer:v3.2

* Per #527, move configuring build.properties back into the Dockerfiles since it differs for Dockerfile.apptainer. Also modify Dockerfile.apptainer to use the dtcenter/met-base-metviewer image.

* Per #526, fix syntax error in build_metviewer_docker.sh

* Per #527, get docker build working for apptainer
@bikegeek bikegeek closed this as completed Jul 8, 2024
@github-project-automation github-project-automation bot moved this from 🔖 Ready to ✅ Done in METplus-Analysis-6.0.0 Development Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: code optimization Code optimization issue priority: high High Priority requestor: METplus Team METplus Development Team required: FOR OFFICIAL RELEASE Required to be completed in the official release for the assigned milestone type: enhancement Improve something that it is currently doing
Projects
No open projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

3 participants