Repository navigation
use AWS ERC - #2985
use AWS ERC#2985
Conversation
🦋 Changeset detectedLatest commit: fa0fd66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
3cc80ed to
18bd523
Compare
|
🐥 |
2e5a04c to
3809790
Compare
|
🐥 |
|
🐥 |
|
🐥 |
|
🐥 |
🚀 Image pushed to AWS ECRImage digest: 🐛 Run the image locallyThe following can be used to run the image locally: # Refer to image using the immutable digest. Find alternatives below.
IMAGE_IDENTIFIER="@sha256:6427503bcd59bdaeb79ddcc0df279acdc88955bb52e8e9fcd035b8a242288784"
# Refer to image using branch tag
# IMAGE_IDENTIFIER=":branch-a777m-devx-logs-changes"
# Refer to image using build tag
# IMAGE_IDENTIFIER=":build-14"
# Set environment variables for the AWS CLI
AWS_PROFILE="<A_PROFILE_FROM_JANUS>"
AWS_DEFAULT_REGION="eu-west-1"
IMAGE_ACCOUNT_ID=$(aws ssm get-parameter --name /organisation/accounts/artifacts --query "Parameter.Value" --output text)
REGISTRY="${IMAGE_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com"
IMAGE="${REGISTRY}/guardian/cdk/logging${IMAGE_IDENTIFIER}"
# Login to AWS ECR https://docs.aws.amazon.com/AmazonECR/latest/userguide/registry_auth.html
aws ecr get-login-password | docker login --username AWS --password-stdin $REGISTRY
# Pull the image
docker pull $IMAGE
# Run the image. You'll likely need to set additional flags. See https://docs.docker.com/reference/cli/docker/container/run.
docker run $IMAGEFrom guardian/actions-ecr. |
564634f to
8466cfe
Compare
| run: docker buildx build --load --platform linux/amd64,linux/arm64 -t logging:latest tools/logging | ||
|
|
||
| - name: Publish image to ECR | ||
| uses: guardian/actions-ecr/push@a777m/support-multi-arch-builds |
There was a problem hiding this comment.
Blocking: Will update this when he PR is merged in the action repo.
| repositoryName: "guardian/cdk/logging", | ||
| }), | ||
| // branch-a777m-devx-logs-changes, will be updated when merged and built on main | ||
| "sha256:32dce4e7a9e628b053ca42c271d7bbdf3697b4980c4612e845a15257c2bd1d99", |
There was a problem hiding this comment.
Non-blocking: Will update this when we have a build off main (separate PR)
|
I also need to investigate the merge conflict. Seems like the inline repo definition has moved 😭 |
8466cfe to
82a777d
Compare
82a777d to
c911f13
Compare
5086ccc to
aeeb2a1
Compare
|
|
||
| const loggingImage = new GuRepository(scope, "loggingRepo", { | ||
| name: "guardian/cdk/logging", | ||
| }).getImage("sha256:32dce4e7a9e628b053ca42c271d7bbdf3697b4980c4612e845a15257c2bd1d99"); |
There was a problem hiding this comment.
Do we need to think about how to make this value dynamic? That is, if a new image is created, how would we know and ensure its digest is added here?
There was a problem hiding this comment.
I think the scope of this PR is just moving the image from GHCR to ECR, in which case a fixed digest is what we used for GHCR.
I would definitely prefer updating this digest to always point to the newest build, whether that be via a tag or something automatically updating the digest, but maybe lets wait till a follow up PR.
There was a problem hiding this comment.
Its worth noting that we currently disable version consistency in favour of always using the image digest as it improved deployment speed. See also https://aws.amazon.com/blogs/containers/announcing-software-version-consistency-for-amazon-ecs-services. I think every container in the task would need to use a digest to see the deployment speed benefits.
I'd be tempted to create some stacked PRs which implements automatically updating the digest before we merge this PR in. It would be a shame to merge in a load of Go code and remove it again if we discover the auto-update isn't possible. WDYT?
There was a problem hiding this comment.
I don't necessarily think this PR is a bad PR if we can't do the auto-update, this is still an image that will primarily be used by CDK and should live alongside its usage!
There was a problem hiding this comment.
Decision after talking to Akash: We'll keep this PR as is and merge it. Spin off a separate piece of work which covers automating the digest so it points to the newest build. We have a couple different approaches which we would like to explore.
| > **Warning** | ||
| > This is a work in progress. |
There was a problem hiding this comment.
| > **Warning** | |
| > This is a work in progress. |
I don't think this README has been updated in a while, we might want to change some bits. Considering this as a "WIP" doesn't make much sense anymore considering its used by a few PROD ecs projects now.
| which adds some configuration to apply a tag to all logs. | ||
|
|
||
| ## Usage | ||
| TBD - GuCDK is being updated to support this. |
There was a problem hiding this comment.
Same here, GuCDK already supports this.
There was a problem hiding this comment.
I've spruced up the docs here: cdaeb52 :)
GuCDK applications gives user the option to ship a app logging along with the service itself. As part of opening up the ECS capabilities, users are able to run the logging service as a sidecar to the main ECS task. Due to the logging being a part of infrastructure rather than a strictly standalone service, it was deemed that it would be simpler to ship the logs from the GuCDK repository. Furthermore, the original devx-logs repo[1] built and pushed the logging image to GHCR which is not where we will be storing images going forward. Building the image from this repo allows devx to ensure that the correct registry is use - while also minimising registry login and setup. This commit moves the devx-logs source code from its own repo to the tools directory in the GuCDK repo. [1] https://github.com/guardian/devx-logs Co-authored-by: AshCorr <ashcorr20@gmail.com>
cdaeb52 to
331b984
Compare
The commit adds a new workflow which builds and publish the logging image to AWS ECR. The logging image does a multi-arch build as its used across all services and application in the guardian estate. It has also been deemed to acceptable to tag the resulting image with `lastest` as it simplifies user interaction with the service. Co-authored-by: AshCorr <ashcorr20@gmail.com>
This commit ensure the logging sidecar uses the image stored in ECR - which is now built from this repository - rather than using GHCR which is no longer a supported registry. Co-authored-by: AshCorr <ashcorr20@gmail.com>
This commit updates the logging README's so they correctly match the current state of the the repo.
331b984 to
fa0fd66
Compare
What does this change?
This PR allows allows us to build and push the logging sidecar to AWS ECR from within the GuCDK libraray. We then consume the image from ECR instead of the previously used GHCR.
How to test
This has successfully been deployed to CDK playground via a canary.
How can we measure success?
Have we considered potential risks?
Checklist
Footnotes
Consider whether this is something that will mean changes to projects that have already been migrated, or to the CDK CLI tool. If changes are required, consider adding a checklist here and/or linking to related PRs. ↩
If you are adding a new construct or pattern, has new documentation been added? If you are amending defaults or changing behaviour, are the existing docs still valid? ↩