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

🌱 Use dl.k8s.io instead of hardcoded GCS URIs #4958

Merged
merged 1 commit into from
Jul 19, 2021

Conversation

spiffxp
Copy link
Member

@spiffxp spiffxp commented Jul 16, 2021

What this PR does / why we need it:
Sort of a follow-on to #4420

The only time a kubernetes GCS bucket name should be showing up in a
hardcoded URI is if gsutil is being used (e.g. gsutil cp gs://foo/bar .)

Otherwise, for tools like curl or wget, dl.k8s.io is much nicer for us
as a project, since we can transparently change where that redirects to
without having to change code everywhere

These changes will mean no changes will be necessary to accommodate a
gs://kubernetes-release -> gs://k8s-release migration equivalent of the
CI migration we're going through right now
(kubernetes/k8s.io#2318)

EDIT: this also catches one last mention of kubernetes-release-dev which I've called out in a review comment below

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

The only time a kubernetes GCS bucket name should be showing up in a
hardcoded URI is if gsutil is being used (e.g. gsutil cp gs://foo/bar .)

Otherwise, for tools like curl or wget, dl.k8s.io is much nicer for us
as a project, since we can transparently change where that redirects to
without having to change code everywhere
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 16, 2021
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 16, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jul 16, 2021

/cc @CecileRobertMichon @sbueringer @randomvariable
folks I saw working on / reviewing #4420

@spiffxp
Copy link
Member Author

spiffxp commented Jul 16, 2021

/test pull-cluster-api-test-main
unsure if flake: sigs.k8s.io/cluster-api/controllers/remote: TestClusterCacheHealthCheck

resolveVersion=$(curl -LsS "http://gcsweb.k8s.io/gcs/kubernetes-release-dev/ci/${version#ci/}.txt")
resolveVersion=$(curl -LsS "http://dl.k8s.io/ci/${version#ci/}.txt")
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI: this might still be causing you to pickup stale builds depending on how racy/flaky the build and build-deprecated jobs are. You're getting the version marker from gs://kubernetes-release-dev, but the binaries from gs://k8s-release-dev

Using dl.k8s.io for both means you don't have to think about it

Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm
/milestone v0.4

Thank you @spiffxp !

@k8s-ci-robot k8s-ci-robot added this to the v0.4 milestone Jul 19, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 19, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 19, 2021
@vincepri
Copy link
Member

/test pull-cluster-api-e2e-workload-upgrade-1-21-latest-main

@k8s-ci-robot k8s-ci-robot merged commit 925ed4b into kubernetes-sigs:master Jul 19, 2021
@spiffxp spiffxp deleted the use-dl-k8s-io branch July 19, 2021 13:50
@sbueringer
Copy link
Member

@spiffxp @CecileRobertMichon You probably also want to do this in CAPZ: https://github.com/kubernetes-sigs/cluster-api-provider-azure/search?q=gs%3A%2F%2F

@spiffxp
Copy link
Member Author

spiffxp commented Jul 26, 2021

@sbueringer Thanks for pointing it out. Moving to dl.k8s.io is not as urgent this month, but I'll be tracking that work under kubernetes/k8s.io#2396 as part of v1.23. I just thought I'd take care of it while I was handling the kubernetes-release-dev / k8s-release-dev stuff here.

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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants