-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore: refactored docker images replication module #282
chore: refactored docker images replication module #282
Conversation
a2cc0fc
to
fbc0c42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, are we now defferring to CONTIRBUTING, LICENSE etc at the root of repo?
And looks like there's some static checks to fix
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Fixed the static checks. Also, from my knowledge, we always refer to the license, contributing.md at the root of the repo, right? I've checked the IDF modules and that's what i observed. Do you have any suggestion @malachi-constant ? |
@@ -19,7 +19,7 @@ deploy: | |||
fi | |||
sleep 15 | |||
fi | |||
- python3 get-list-of-eks-images.py --eks-version ${SEEDFARMER_PARAMETER_EKS_VERSION} --versions-directory data/eks_dockerimage-replication/versions --update-helm-repos --registry-prefix "${AWS_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com/${AWS_CODESEEDER_NAME}-" | |||
- python3 get-list-of-eks-images.py --eks-version ${SEEDFARMER_PARAMETER_EKS_VERSION} --versions-directory data/eks_dockerimage-replication/versions --update-helm-repos --registry-prefix "${AWS_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com/${SEEDFARMER_PROJECT_NAME}-" | |||
- chmod +x replication.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit (not related to change) - is this chmod required? Can we check-in the file with the correct permissions?
@@ -11,6 +11,7 @@ charts: | |||
tag: | |||
location: chart | |||
path: appVersion | |||
prefix: v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirming if this is intentional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This is intentional. Some helm charts are tagged by prefixing v
to their versions, and their corresponding docker images never have that prefix.
example is load balancer controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
Issue #, if available:
Description of changes:
idf
as project nameBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.