Convert private ecr to public ecr #840
Conversation
|
What's the difference between #730 and this one? |
Not difference. I go along with the multiple regions but forgot that Seth has done it. Will close this |
| @@ -529,8 +529,8 @@ jobs: | |||
| if: steps.e2etest-release.outputs.cache-hit != 'true' | |||
| run: | | |||
| docker load < build/packages/$IMAGE_NAME.tar | |||
There was a problem hiding this comment.
BTW, I'd like to remove docker load from the workflow - we should just push to the ECR repo instead of docker saving. Then we could use the official docker build github action, using buildx, and making arm64 build trivial
There was a problem hiding this comment.
@khanhntd imo the repo should be named adot-collector-tests ; we could potentially have other test images there too wdyt?
There was a problem hiding this comment.
True thought. Mostly I thought the docker saving there is for separating the e2e preparations step with the build step. Other than that, I don't see any reasons to use docker save so I am down with using docker buildx for arm64.
There was a problem hiding this comment.
@alolita the only two tests that we have are: integration test from the CI and validation test from the CD. But the CD release is using public.ecr.aws/aws-observability/aws-otel-collector; so the only one we are using for this repo is the CI. So IMO, if we are planning to add additional testing such as unit testing or config testing such as FB in the future, then we should go with yours; if we don't, then specific enough such as adot-collector-integration-test is fine since its only for the CI.
There was a problem hiding this comment.
However, such tests are not feasible since we are a downstream repo so I cannot imagine we have addition tests beside integration test such as unit test or config test.
I'm down with that too. Having a hard time to decide the naming convention though. |
| @@ -4,4 +4,4 @@ s3ReleaseCandidateBucketName: "aws-otel-collector-release-candidate" | |||
| s3BucketName: "aws-otel-collector-test" | |||
| sshKeyS3BucketName: "aoc-ssh-key" | |||
| traceDataS3BucketName: "trace-expected-data" | |||
| testingImageRepoName: "611364707713.dkr.ecr.us-west-2.amazonaws.com/aws/aws-otel-collector" | |||
| testingImageRepoName: "public.ecr.aws/aws-otel-test/adot-collector-integration-test" | |||
There was a problem hiding this comment.
I assume its purpose is for notifying what resources are used with the release and testing. Currently ask Ying to confirm about it.
There was a problem hiding this comment.
Got a confirmation from @wyTrivail. So it's purpose is notify what resources are being used for the validation test and release validation. An example would be https://github.com/aws-observability/aws-otel-collector/blob/main/.github/workflows/CD.yml#L187. Somehow we decide to define it explicitly instead of using a yaml file anymore. IMO, it seems weird for me to delete it since most of the release validation or integration test, its used the same bucket so either define a yaml or define an environment variable is better for me instead of defining it explicitly though.
.github/workflows/CI.yml
Outdated
| @@ -520,6 +488,10 @@ jobs: | |||
| if: steps.e2etest-release.outputs.cache-hit != 'true' | |||
| run: s3_bucket_name=aws-otel-collector-test upload_to_latest=0 bash tools/release/image-binary-release/s3-release.sh | |||
|
|
|||
| - name: Build Image | |||
There was a problem hiding this comment.
Think we can just use https://github.com/marketplace/actions/build-and-push-docker-images
There was a problem hiding this comment.
Agreed. Seems the load tar involves in the CD so I should do all of this in one PR. Will post another PR for this step soon.
|
Description:
Currently, the CI publishes the image to private repo and we need to pull it from there for integration test. However, as we discussed with the adot team, converting from private to public ECR would give us two advantages: