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

Default to AWS when possible #147

Merged
merged 7 commits into from
Feb 8, 2023
Merged

Conversation

BenTheElder
Copy link
Member

@BenTheElder BenTheElder commented Feb 8, 2023

Implements #143

TODO: Configure default s3 backend per cloud run region in the production config via env before we ship this. Possibly use cloudfront for that purpose instead in the future.
EDIT: see kubernetes/k8s.io#4739

This is a messy change, a lot of things needed renaming / refactoring.
The pre-parsed generator is still a bit gross, but fairly well tested and covers GCP + AWS now.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 8, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 8, 2023
@BenTheElder BenTheElder force-pushed the gcptoo branch 2 times, most recently from af4b051 to acdd4eb Compare February 8, 2023 06:44
@BenTheElder BenTheElder changed the title [WIP] default to AWS when possible Default to AWS when possible Feb 8, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 8, 2023
@@ -45,6 +45,7 @@ func main() {
UpstreamRegistryPath: getEnv("UPSTREAM_REGISTRY_PATH", "k8s-artifacts-prod/images"),
InfoURL: "https://github.com/kubernetes/registry.k8s.io",
PrivacyURL: "https://www.linuxfoundation.org/privacy-policy/",
DefaultAWSBaseURL: getEnv("DEFAULT_AWS_BASE_URL", "https://prod-registry-k8s-io-us-east-1.s3.dualstack.us-east-1.amazonaws.com"),
Copy link
Member Author

Choose a reason for hiding this comment

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

notable change: we have this env / default URL now xref kubernetes/k8s.io#4739

awsRegion, ipIsKnown := regionMapper.GetIP(clientIP)
if !ipIsKnown {
// no region match, redirect to main upstream registry
// if client is coming from GCP, stay in GCP
Copy link
Member Author

Choose a reason for hiding this comment

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

notable redirect logic change here

@BenTheElder
Copy link
Member Author

The rest of the diff is renaming things to be multi-cloud instead of AWS specific, and then refactoring the generator to support GCP data.

I also pulled the downloaded data out to unmodified files instead of inlining them as go strings, it's slightly more code but I think cleaner and if anyone vendors this package they can keep only the go files with processed data not the raw strings.

@ameukam
Copy link
Member

ameukam commented Feb 8, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 8, 2023
@k8s-ci-robot k8s-ci-robot merged commit 59e0fac into kubernetes:main Feb 8, 2023
@BenTheElder BenTheElder deleted the gcptoo branch February 8, 2023 21:11
@BenTheElder
Copy link
Member Author

@BenTheElder
Copy link
Member Author

gcr.io/k8s-staging-infra-tools/archeio:v20230208-v0.0.4-40-g59e0fac@sha256:43904e32c58803bed1dbf6457e1b5abdd1409c976bb951f5bb32cf3b37772b2f is live

@BenTheElder
Copy link
Member Author

tagged v0.1.0 (spoke to arnaud)

@BenTheElder
Copy link
Member Author

https://testgrid.k8s.io/sig-k8s-infra-canaries#kops-grid-gcr-mirror-canary looks good

@BenTheElder
Copy link
Member Author

gcr.io/k8s-staging-infra-tools/archeio:v20230208-v0.1.0@sha256:43904e32c58803bed1dbf6457e1b5abdd1409c976bb951f5bb32cf3b37772b2f after manual rerun trigger, not sure why tag push didn't trigger.

https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-oci-proxy-push-images/1623444771215249408

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants