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

Allow change pause container image #1527

Closed
wants to merge 5 commits into from
Closed

Conversation

DingGGu
Copy link

@DingGGu DingGGu commented Nov 23, 2023

Issue #1514, if available:

Description of changes:

  • Pause container image can be change.
  • Changed to not use the ECR API if it is not ECR.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done

  • Add test for pause-container

See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.

@DingGGu DingGGu marked this pull request as draft November 23, 2023 09:51
files/bootstrap.sh Outdated Show resolved Hide resolved
@DingGGu DingGGu marked this pull request as ready for review December 11, 2023 03:00
@DingGGu
Copy link
Author

DingGGu commented Dec 11, 2023

If this change is applied, it can be easily changed through argument manipulation if the pause container is deployed in a separate registry.

Currently, If cluster is running without internet acess, you need to configure VPC Endpoint (PrivateLink) of ecr.api, ecr.dkr and s3 to download pause container image, which is a waste of resource.

Copy link
Member

@cartermckinnon cartermckinnon left a comment

Choose a reason for hiding this comment

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

Looks fine to me, just suggested a tweak to the flag doc.

files/bootstrap.sh Outdated Show resolved Hide resolved
@cartermckinnon
Copy link
Member

@DingGGu you'll need to fix the lint findings, then I'll kick off integration tests 👍

@DingGGu
Copy link
Author

DingGGu commented Dec 22, 2023

@cartermckinnon Is there anything else I need to change?

@cartermckinnon cartermckinnon deleted the branch awslabs:master May 8, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants