fix(ci): get latest created tag for catalog remote tests#576
Conversation
|
ajaskolski
changed the title
ajaskolski
force-pushed
the
fix-use-latest-dynamic-tag-for-catalog-tests
branch
from
153618a to
a9a9ee5
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates the CI workflows to dynamically fetch the latest semantic version tag from ECR instead of relying on the now-immutable latest tag for catalog remote tests.
Key Changes:
- Replaced hardcoded
latesttag with dynamic tag retrieval from AWS ECR - Added AWS credentials configuration and tag fetching logic to both workflows
- Moved
CATALOG_SERVICE_IMAGEenvironment variable to the test step scope
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/schedule-main.yml | Added dynamic ECR tag retrieval and updated image references to use fetched semantic version tag |
| .github/workflows/pull-request-main.yml | Added dynamic ECR tag retrieval and updated image references to use fetched semantic version tag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ajaskolski
force-pushed
the
fix-use-latest-dynamic-tag-for-catalog-tests
branch
from
a9a9ee5 to
7e6c623
Compare
| run: | | ||
| # Fetch all image tags from ECR and get the latest semantic version tag | ||
| # Get all tags, filter for semantic versions (vX.Y.Z), and sort properly | ||
| LATEST_TAG=$(aws ecr describe-images \ |
There was a problem hiding this comment.
Think this will return images from older to latest. Also not sure what will happen when there are too many (eg will we use pagination?)
You could use something like this to sort then based on push date in reverse (latest first)
aws ecr describe-images --repository-name op-catalog-service \
--query 'reverse(sort_by(imageDetails,& imagePushedAt))'
There was a problem hiding this comment.
actually good idea there I will update it like that ! thx, updated !
There was a problem hiding this comment.
ok now it works, simplified a bit.
ajaskolski
force-pushed
the
fix-use-latest-dynamic-tag-for-catalog-tests
branch
from
7e6c623 to
60afe27
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ajaskolski
force-pushed
the
fix-use-latest-dynamic-tag-for-catalog-tests
branch
from
60afe27 to
088d05a
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| ] | first | ||
| ') | ||
|
|
||
| if [[ -z "${LATEST_TAG}" ]] || [[ "${LATEST_TAG}" == "null" ]]; then |
There was a problem hiding this comment.
hmm i wonder if this approach of running integration test on latest docker image is the right appraoch? Shouldn we be building the docker image using the PR branch instead?
What if i updated or made a breaking change and modified the intergration tests, i would always get a failure here right?
There was a problem hiding this comment.
We spoke with James about it. So we own both products if we do introduce breaking changes we update upstream and downstream (cldf) in sync. You can test locally your breaking changes with local service version and update cldf when upstream service is released.
There was a problem hiding this comment.
Also if somebody make breaking changes here we should fail it imo, otherwise someone can update breaking version of the CLDF in projects importing it that won't work with the running service.
There was a problem hiding this comment.
Oh wait i was confused, this makes sense!
We dont use
latesttag anymore as we have settings for immutable tags. We need to get latest tag dynamically.