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

DAS-2070 - Implement GitHub workflows CI/CD. #1

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

owenlittlejohns
Copy link
Member

@owenlittlejohns owenlittlejohns commented Jan 23, 2024

Description

This PR implements CI/CD to run the unittest suite for all PRs. It also adds a Docker image publication workflow for when changes are merged into the main branch containing an update to docker/service_version.txt.

The workflows are essentially copies of those used in the HOSS and Swath Projector repositories.

Jira Issue ID

DAS-2070

Local Test Steps

It's probably good to check the bin/build-image, bin/build-test and bin/run-test scripts all work on your local machine (given the change in image name).

You can also test this all works with Harmony locally:

  • Clone this repository.
  • Run bin/build-image.
  • Update your local Harmony instance to use the new HyBIG image name:
    • In packages/util/env-defaults update HYBIG_SERVICE_QUEUE_URLS to use an image name of ghcr.io/nasa/harmony-browse-image-generator:latest. (It should just be a case of replacing sds with ghcr.io/nasa)
    • In services/harmony/env-defaults update the HyBIG image: HYBIG_IMAGE=ghcr.io/nasa/harmony-browse-image-generator:latest.
  • Rebuild the image Harmony uses locally with npm run build-all.
  • Start a new Harmony instance with bin/bootstrap-harmony.
  • Run through the HyBIG regression test suite using a Jupyter notebook server in the browser, and setting HARMONY_HOST_URL='http:localhost:3000'. The tests should run and complete.

Things to look for on this PR itself:

  • The unit tests should run and complete successfully.
  • I think a Snyk check should also have been kicked off and pass. (Checking both security and licenses)

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes. (Already prepared for v1.0.0)
  • docker/service_version.txt updated if publishing a release. (Already at 1.0.0)
  • Tests added/updated and passing. (No new tests needed)
  • Documentation updated (if needed).

@lyonthefrog
Copy link
Collaborator

lyonthefrog commented Jan 24, 2024

Take out the elusive sds's:

image

@owenlittlejohns
Copy link
Member Author

@lyonthefrog - good catches. I made some changes in the latest commit to fix most of those:

  • Updated all the references in the exceptions.py module. I made that just "harmony-browse-image-generator" and made it a variable, so it only ever needs to be tweaked once.
  • Updated the references in build-test and build-image.
  • Updated the references in the service.Dockerfile and tests.Dockerfile.
  • Left things as they were in the CHANGELOG.md, as that is trying to say that v1.0.0 is the same as v0.0.11 of the old sds-related Docker image.

@lyonthefrog
Copy link
Collaborator

The HyBIG regression tests were successful, as were the unit tests (report below). And it appears that the Snyk checks below have passed.

image

Copy link
Collaborator

@lyonthefrog lyonthefrog left a comment

Choose a reason for hiding this comment

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

I have already commented on my successful tests and one issue that has already been resolved (sds's), so I approve this.

Copy link
Member

@flamingbear flamingbear left a comment

Choose a reason for hiding this comment

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

I found some typos that need to be fixed, a suggestion for extracting the changelog differently. But overall, this is looking great.

.github/pull_request_template.md Show resolved Hide resolved
README.md Outdated
Initially service Docker images will be hosted in AWS Elastic Container
Registry (ECR). When this repository is migrated to the NASA GitHub
organisation, service images will be published to ghcr.io, instead.
The CI/CD for HyBIG is contained in GitHub workslofws in the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The CI/CD for HyBIG is contained in GitHub workslofws in the
The CI/CD for HyBIG is contained in GitHub worksflows in the

README.md Outdated
You can reach out to the maintainers of this repository via email:

* [email protected]
* [email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* matthew.savioe@colorado.edu
* matthew.savoie@colorado.edu

I don't love email, but I should be able to ignore it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gah! Sorry!

Comment on lines 13 to 24

CHANGELOG_FILE="CHANGELOG.md"
VERSION_PATTERN="^## v"
# Count number of versions in version file:
number_of_versions=$(grep -c "${VERSION_PATTERN}" ${CHANGELOG_FILE})

if [ ${number_of_versions} -gt 1 ]
then
grep -B 9999 -m 2 "${VERSION_PATTERN}" ${CHANGELOG_FILE} | sed '$d' | sed '$d'
else
cat ${CHANGELOG_FILE}
fi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
CHANGELOG_FILE="CHANGELOG.md"
VERSION_PATTERN="^## v"
# Count number of versions in version file:
number_of_versions=$(grep -c "${VERSION_PATTERN}" ${CHANGELOG_FILE})
if [ ${number_of_versions} -gt 1 ]
then
grep -B 9999 -m 2 "${VERSION_PATTERN}" ${CHANGELOG_FILE} | sed '$d' | sed '$d'
else
cat ${CHANGELOG_FILE}
fi
# File name
CHANGELOG_FILE="CHANGELOG.md"
# Regular expression
VERSION_PATTERN="^## v"
# Read the file and extract text between the first two occurrences of the VERSION_PATTERN
result=$(awk "/$VERSION_PATTERN/{c++; if(c==2) exit;} c==1" "$CHANGELOG_FILE")
# Print the result
echo "$result" | grep -v "^#"

What do you think? I suppose you'd have to update the comment too.

# 2023-06-16: Created.
# 2023-10-10: Copied from earthdata-varinfo repository to HOSS.
# 2024-01-03: Copied from HOSS repository to the Swath Projector.
# 2024-01-23: Copied from Swath Projector repository to HyBIG.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# 2024-01-23: Copied from Swath Projector repository to HyBIG.
# 2024-01-23: Copied and modified from Swath Projector repository to HyBIG.

Copy link
Member Author

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

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

@flamingbear - I think I fixed up all the typos and adopted all the changes you suggested. Thanks for catching all that.

.github/pull_request_template.md Show resolved Hide resolved
Copy link
Member

@flamingbear flamingbear left a comment

Choose a reason for hiding this comment

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

:shipit: LGTM. Thanks for the updates

@owenlittlejohns owenlittlejohns merged commit 04c6ea0 into main Jan 24, 2024
3 checks passed
@owenlittlejohns owenlittlejohns deleted the DAS-2070-HyBIG-GitHub-CICD branch January 24, 2024 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants