Feature #12 debian

[JohnHalleyGotway]

Pull Request Testing

Note that coordinating changes between this repo and MET is a little difficult. This PR is pulling the compile script from MET’s develop branch even though the develop branch doesn’t contain the updates yet. They haven’t been merged in from the feature_2669_proj branch yet.

• Describe testing already performed for these changes:
  
  Created v2.1_beta4 tag and see that this GHA run successfully built the base image and pushed the result to DockerHub:
  https://github.com/dtcenter/METbaseimage/actions/runs/6005438417

Note that this GHA proves that MET can be compiled in this Docker environment:
https://github.com/dtcenter/MET/actions/runs/6005888920

• Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
  
  Review the changes.
  Confirm the full transition to Debian base image and elimination of centos.
  Note the addition of the proj library.

• Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]

• Do these changes include sufficient testing updates? [Yes]

• Will this PR result in changes to the test suite? [No]
  
  If yes, describe the new output and/or changes to the existing output:
  

• Please complete this pull request review by [8/29/23].
  

Pull Request Checklist

See the METplus Workflow for details.

• Review the source issue metadata (required labels, projects, and milestone).
• Complete the PR definition above.
• Ensure the PR title matches the feature or bugfix branch name.
• Define the PR metadata, as permissions allow.
  Select: Reviewer(s)
  Select: Organization level software support Project or Repository level development cycle Project
  Select: Milestone as the version that will include these changes
• After submitting the PR, select the ⚙️ icon in the Development section of the right hand sidebar. Search for the issue that this PR will close and select it, if it is not already selected.
• After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
• Close the linked issue and delete your feature or bugfix branch from GitHub.

[georgemccabe]

I was going to suggest that the unit tests be run to ensure that the changes to the base image work as expected. It looks like the PR run for dtcenter/MET#2672 had a couple seg faults, so that should be resolved before this is approved. I am going to click "Request changes" but if no changes are required to fix the issues in the MET unit tests, then I can approve, because the changes look good as far as I can tell.

Some additional comments:

• I see that the following tools are now built using apt install to add the proj library:
  • cmake
  • libtiff-dev
  • sqlite3
  • libsqlite3-dev

The GitHub diff is a bit hard to follow since the Dockerfiles were renamed, so I wanted to note the list of new tools here so it is clear if the changes in this PR are revisited later.

• I see that the deprecated set-output command was replaced with devops-actions/action-get-tag. The command could be replaced with the echo "{name}={value}" >> $GITHUB_OUTPUT syntax. This action was created by a verified creator, so it seems like a good solution that can be more easily updated if anything changes on GitHub's end (e.g. update this action to a later version instead of determining the new correct syntax to extract the tag name).

[JohnHalleyGotway]

I'd like to proceed with merging this PR and creating a new v2.1 tagged release. I'll re-request this review.

[georgemccabe]

We will need to remember to change this path to no longer reference the feature branch once dtcenter/MET#2672 is merged.

[JohnHalleyGotway]

Note, this GHA run for the dtcenter/MET#2672 PR runs successfully using the dtcenter/met-base:v2.1 Docker image. Differences are flagged in the output as described in this comment. We discussed these diffs during the METplus Engineering meeting on 9/5/23 and decided to proceed with these changes.

The next steps are:

1. Approve/merge the Feature #2669 proj MET#2672 PR to get the compile_MET_all.sh updates into the develop branch.
2. Update the Dockerfile to pull from develop rather than the feature branch.
3. Approve/merge this PR.
4. Delete the existing v2.1 images from DockerHub.
5. Create a new v2.1 release for METbaseimage, along with release notes.
6. Let GHA build/push the v2.1 image back up to DockerHub.
7. Create a METplus issue to update the truth data to move past the changes that will likely be introduced by upgrading to Debian 12 with a newer GCC version.

[georgemccabe]

I made a few in-line notes:

• Make sure to update the URL in the Dockerfile after the MET PR is merged because the branch listed will no longer exist
• Mention update from Debian 10 to 12 in the v2.1 release notes

I see that the MET PR tests are still running, but the diffs were explained in this comment. We discussed this in the METplus Engineering meeting on 9/5/2023 and decided that we are OK with these diffs.

I approve as long as there are no explainable diffs that pop up besides the ones that were described above and the minor changes are made at the appropriate time.

[georgemccabe]

A note about the order of merging of this and the corresponding MET PR:

To test the changes for dtcenter/MET#2672, we had to create the dtcenter/met-base:v2.1 DockerHub tag that uses the build script from the MET feature_2669_proj branch. We will update the Dockerfile in this PR to no longer reference the feature branch, as it will go away once the MET PR is merged.
We discussed that we should merge dtcenter/MET#2672 pull request, then update this branch to use develop instead of feature_2669_proj in the URL to the build script. We can retag v2.1 in METbaseimage after it is merged, which will kick off a rebuild of the Docker image. This last step is not mandatory, but would be good to do to ensure that we don't introduce a typo in the METbaseimage PR that we will have to track down later.