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(workflow): enable community contributions #1848

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

csatib02
Copy link
Member

@csatib02 csatib02 commented Jun 25, 2024

Description

  • This PR enables community contributions, by dropping ghcr uploads on pull requests, and instead uses artifacts.
  • If there is a release ghcr.io will be used, no change was made there.
  • In case of pull requests, artifacts are created and used for e2e testing.
  • No comments will be posted under PR's with the images, instead the build artifacts can be used for further manual testing.
  • Minor design changes.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

@csatib02 csatib02 added github actions Pull requests that update GitHub Actions code ci Continious Integration related PRs labels Jun 25, 2024
@csatib02 csatib02 self-assigned this Jun 25, 2024
@csatib02 csatib02 requested a review from a team as a code owner June 25, 2024 10:16
@adamtagscherer
Copy link
Contributor

  1. What will happen if we want to wire in the cloud e2e tests? Will we have the latest tagged images on the main branch? If I see it correctly then we build and push to ghcr images only when creating a release.

No comments will be posted under PR's with the images, instead the build artifacts can be used for further manual testing.

  1. How can I use the build artifacts for manual testing? For example if I want to deploy the whole system on AWS and test my changes there, how do I overwrite the images in the cloudformation file?
    I guess we have to build the images locally, upload them to DockerHub and use those in the cloudformation file. I have no objections against it, if the others agree that it's a good trade off for enabling the members to contribute.

@csatib02
Copy link
Member Author

csatib02 commented Jun 25, 2024

@adamtagscherer

  1. What will happen if we want to wire in the cloud e2e tests? Will we have the latest tagged images on the main branch? If I see it correctly then we build and push to ghcr images only when creating a release.

We are also building and pushing images whenenver a PR has been merged into main, check main-merge.yml, those images will be tagged latest, and can be used for the e2e tests for the different environments.

No comments will be posted under PR's with the images, instead the build artifacts can be used for further manual testing.

  1. How can I use the build artifacts for manual testing? For example if I want to deploy the whole system on AWS and test my changes there, how do I overwrite the images in the cloudformation file?
    I guess we have to build the images locally, upload them to DockerHub and use those in the cloudformation file. I have no objections against it, if the others agree that it's a good trade off for enabling the members to contribute.

We can create a workflow that can be run manually by org members, to upload images to ghcr tagged with dev, so it can be used for such use-cases you mentioned.
The artifacts themselves can be found on the actions page if you scroll down. The workflow creates docker images, that can be loaded to your own local docker registry or any. After downloading use docker load {image}.

@ramizpolic
Copy link
Member

ramizpolic commented Jun 25, 2024

  1. What will happen if we want to wire in the cloud e2e tests? Will we have the latest tagged images on the main branch? If I see it correctly then we build and push to ghcr images only when creating a release.

No comments will be posted under PR's with the images, instead the build artifacts can be used for further manual testing.

  1. How can I use the build artifacts for manual testing? For example if I want to deploy the whole system on AWS and test my changes there, how do I overwrite the images in the cloudformation file?
    I guess we have to build the images locally, upload them to DockerHub and use those in the cloudformation file. I have no objections against it, if the others agree that it's a good trade off for enabling the members to contribute.

@adamtagscherer raises important concerns and i would also like to add to this. Although we are not pushing PR images to GHCR, we could solve this by creating another (manually triggered) workflow that takes specific PR commit reference and pushes built artifacts for that commit to GHCR. This would allow us to test things, would also unblock community contributions, and it would also address the security concerns. My suggestion is that we add this action in a separate PR after this one is merged (it can be reused similarly to pushes to main).

I wrote a small PoC bash script that can import images from a specific GA pipeline run into local docker registry. I have tested it against GA run https://github.com/openclarity/vmclarity/actions/runs/9660640440. You need GitHub CLI to run this. Note that pulling artifacts can take some time. It can be used in case we use docker-only approach for E2E testing on cloud providers. It does not solve directly the second concern regarding CF file (unless it is extended with this script). But the suggestion above

#!/usr/bin/env bash

## Defines which image artifacts to download
GH_ACTION_RUN_ID="9660640440"
PLATFORM="arm64"

## Defines how to tag loaded VMClarity images
IMPORTED_IMAGE_REGISTRY="ghcr.io/openclarity"
IMPORTED_IMAGE_TAG="latest"

## Set path to temporary dir where artifacts will be pulled into
mkdir -p /tmp/vmclarity-images
cd  /tmp/vmclarity-images

## Download all artifacts using GitHub CLI
gh run download --repo openclarity/vmclarity ${GH_ACTIONS_RUN_ID}

## Load all VMClarity platform-specific images
for image_archive in vmclarity-*-${PLATFORM}/*.tar; do
  # load image
  image_name=$(basename $image_archive .tar)
  image_load_output=$(docker load --input "$image_archive" --quiet)
  image_id=${image_load_output#"Loaded image ID: sha256:"} # strip prefix, very hacky due to https://github.com/moby/moby/issues/40188 limitations

  # tag image
  tagged_image="$IMPORTED_IMAGE_REGISTRY/$image_name:$IMPORTED_IMAGE_TAG"
  echo "Tagging image ID=$image_id as $tagged_image"
  docker tag $image_id $tagged_image
done

## Cleanup artifacts temporary dir
cd ~
rm -rf /tmp/vmclarity-images

@csatib02
Copy link
Member Author

@adamtagscherer raises important concerns and i would also like to add to this. Although we are not pushing PR images to GHCR, we could solve this by creating another (manually triggered) workflow that takes specific PR commit reference and pushes built artifacts for that commit to GHCR. This would allow us to test things, would also unblock community contributions, and it would also address the security concerns. My suggestion is that we add this action in a separate PR after this one is merged (it can be reused similarly to pushes to main).

I wrote a small PoC bash script that can import images from a specific GA pipeline run into local docker registry. I have tested it against GA run https://github.com/openclarity/vmclarity/actions/runs/9660640440. You need GitHub CLI to run this. Note that pulling artifacts can take some time. It can be used in case we use docker-only approach for E2E testing on cloud providers. It does not solve directly the second concern regarding CF file (unless it is extended with this script). But the suggestion above

@ramizpolic I also agree that there should be a way to manually upload to ghcr to further test changes, let's sit together and decide on a solution that works for everyone.

We had something like this created by @zsoltkacsandi recently in #1768, I removed it in this PR since essential parts of the workflow has been reworked, and I believe we need to adhere with the changes made on it.

ramizpolic
ramizpolic previously approved these changes Jun 26, 2024
Copy link
Member

@ramizpolic ramizpolic left a comment

Choose a reason for hiding this comment

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

LGTM, great work on this @csatib02!

@ramizpolic
Copy link
Member

lets just wait for #1828 #1794 to be merged first

adamtagscherer
adamtagscherer previously approved these changes Jun 27, 2024
Copy link
Contributor

@adamtagscherer adamtagscherer left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

adamtagscherer
adamtagscherer previously approved these changes Jun 27, 2024
akijakya
akijakya previously approved these changes Jun 28, 2024
Copy link
Contributor

@akijakya akijakya left a comment

Choose a reason for hiding this comment

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

LGTM!

ramizpolic
ramizpolic previously approved these changes Jun 28, 2024
@csatib02 csatib02 force-pushed the ci/enable-community-contributions branch from a4d37be to 673fb30 Compare July 8, 2024 12:02
@csatib02 csatib02 requested a review from ramizpolic July 8, 2024 12:03
ramizpolic
ramizpolic previously approved these changes Jul 8, 2024
Signed-off-by: Bence Csati <bcsati@cisco.com>
Signed-off-by: Bence Csati <bcsati@cisco.com>
Signed-off-by: Bence Csati <bcsati@cisco.com>
Signed-off-by: Bence Csati <bcsati@cisco.com>
@csatib02 csatib02 added this pull request to the merge queue Jul 8, 2024
Merged via the queue into main with commit 3adddf7 Jul 8, 2024
17 checks passed
@csatib02 csatib02 deleted the ci/enable-community-contributions branch July 8, 2024 13:24
@csatib02 csatib02 removed the blocked label Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continious Integration related PRs github actions Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants