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

CI to publish docker images #9

Closed
wants to merge 11 commits into from
Closed

CI to publish docker images #9

wants to merge 11 commits into from

Conversation

nshyrei
Copy link
Contributor

@nshyrei nshyrei commented Apr 8, 2024

Creates a new Github action that builds and publishes enclave-base and parent-base image into https://hub.docker.com/u/fortanix docker repository. The action is triggered only when someone makes a change to any file in the corresponding folders.
While this solution lacks versioning support like the previous iteration, where we had a tag file specifying image version, I am not fully sure that it is needed right now as those images are updated rarely and there was never a need for backward compatibility.

@nshyrei nshyrei added the enhancement New feature or request label Apr 8, 2024
@aditijannu
Copy link
Collaborator

A few questions:

  1. When is this workflow triggered i.e. in a PR job or is there a job that runs after a PR is merged to master?

  2. If versioning/tagging of the images is not done, what tag are we publishing the images with? Will this not overwrite previously published images? It makes sense to preserve previous images if there are any issues with the latest published images.

  3. Will the newly published image be referenced in the salmiac CI pipeline? If yes, can jenkins also pull these images for the release build? If no, then we might want to update references throughout the code base of the enclave-base and parent-base ECR repository references to docker hub references.

@nshyrei
Copy link
Contributor Author

nshyrei commented Apr 8, 2024

  1. @aditijannu This job is triggered when PR targets folders specified in paths variables in github actions file.
  2. By default docker will use latest tag. This is also the default tag for a docker pull command. If we don't have an explicit versioning the job will indeed overwrite previous image. But as I have said, maybe that's not an immediate issue.
  3. Yes, references should be updated, good catch.

@aditijannu
Copy link
Collaborator

@nshyrei

  1. If the job is triggered during PR jobs then we should ensure that the images are not pushed to docker repository until the PR is merged. If someone makes a PR where there are changes in the files to enclave-base or parent-base directories and updates the PR again with additional changes, it would be pushing unnecessary images to the docker hub. Building these images multiple times is okay but pushing multiple times is unnecessary and could result in faulty images being published to our docker hub repo.
  2. Versioning need not be automated at the moment, I agree. But some level of manual versioning can be followed so that previously built versions of the images do not get overwritten. Even if we hard code the version for now, it is still better than losing older images (which are probably part of a salmiac release artifact used in CCM).

@nshyrei
Copy link
Contributor Author

nshyrei commented Apr 9, 2024

@aditijannu How do you see automatic publishing of the images? Should it be triggered manually maybe?

@aditijannu
Copy link
Collaborator

@aditijannu How do you see automatic publishing of the images? Should it be triggered manually maybe?

There is github actions event github.event.pull_request.merged where you can trigger publishing of the images on merge, see here for reference - https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-your-pull_request_target-workflow-when-a-pull-request-merges

@nshyrei nshyrei force-pushed the ns/SALM-564 branch 3 times, most recently from 8417fdf to 7c3bb04 Compare April 9, 2024 18:27
@nshyrei
Copy link
Contributor Author

nshyrei commented Apr 9, 2024

@aditijannu Updated the PR with an additional job that is triggered only on PR merge. The built images are exchanged between jobs using upload-artifact action as jobs have isolated executors.

path: /tmp/enclave-base
- name: Load enclave-base image
run: |
docker load --input /tmp/enclave-base/enclave-base.tar
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we include a hard coded version tag here and check that the image doesn't already exist?
docker tag $IMGNAME $IMGNAME:$IMGTAG
where IMGNAME= enclave-base or parent-base and IMGTAG is maybe 2.0.0

and then docker manifest inspect $IMGNAME:$IMGTAG >> If this command fails then the image doesn't exist and the workflow is a success. If this command succeeds, it means the image already exists and we should update the version tag. This will enforce users to update the tag whenever there is an update to the docker images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added tag files in the latest commit, please take a look.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please update references of 513076507034.dkr.ecr.us-west-1.amazonaws.com/nitro-parent-base and 513076507034.dkr.ecr.us-west-1.amazonaws.com/nitro-enclave-base throughout the repository. For the purpose of the first image you can simply make a copy of whats in the s3 in our docker hub i.e.

docker tag 513076......amazonaws.com/nitro-parent-base:1.1.4 fortanix/nitro-parent-base:2.0.0
docker push fortanix/nitro-parent-base:2.0.0

similarly for enclave-base

Copy link
Contributor Author

@nshyrei nshyrei Apr 11, 2024

Choose a reason for hiding this comment

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

Pushed to fortanix:

Proof that those are the latest versions:

aws ecr describe-images --repository-name  nitro-parent-base --region us-west-1 --output json | jq '.imageDetails[].imageTags'
[
  "1.0.2"
]
[
  "1.0.0"
]
[
  "1.1.1"
]
[
  "1.0.4"
]
[
  "1.0.3"
]
[
  "1.1.2"
]
[
  "1.1.3"
]
[
  "1.1.0"
]
[
  "1.0.1"
]
aws ecr describe-images --repository-name  nitro-enclave-base --region us-west-1 --output json | jq '.imageDetails[].imageTags'
[
  "1.0.1"
]
[
  "1.0.2"
]
[
  "1.0.0"
]

I have kept the original version tags to not add confusion with a sudden bump in version without making any updates to the images.

@nshyrei nshyrei requested a review from aditijannu April 11, 2024 16:12
@nshyrei nshyrei closed this Apr 12, 2024
@nshyrei nshyrei deleted the ns/SALM-564 branch April 12, 2024 16:35
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.

2 participants