-
Notifications
You must be signed in to change notification settings - Fork 716
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
fix: use dl.k8s.io, not kubernetes-release bucket #2872
Conversation
This commit improves the URL handling for resource extraction and version filtering processes. The following changes were made: In the kinder/pkg/extract/extract.go file: The releaseBuildURepository constant was updated to use the new "https://dl.k8s.io/release" URL for release builds. In the tests/e2e/manifests/verify_manifest_lists.go file: The URL used in the filterVersions() function was updated to use the new "https://dl.k8s.io/release" URL for retrieving stable versions. Signed-off-by: Ricky Sadowski <[email protected]>
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.
There are still references to https://storage.googleapis.com/kubernetes-release instead of https://dl.k8s.io/
is it possible to change all k/kubeadm references in this PR?
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.
There are still references to https://storage.googleapis.com/kubernetes-release instead of https://dl.k8s.io/
is it possible to change all k/kubeadm references in this PR?
Yes! Totally, can you help me understand what other references you're seeing? I believe this PR already covers changing all the old references. There are still some instances to |
you can also add them here in a separate commit |
Updated with the |
b10028b
to
3aa32d8
Compare
Signed-off-by: rsadowsk <[email protected]>
@neolit123, I reworked the |
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.
/lgtm
| **GA release** | from .deb or .rpm repository | from [github release page](https://github.com/kubernetes/kubernetes/releases) or from `gs://kubernetes-release/release/` GCS bucket | from `k8s.gcr.io` container registry or from [github release page](https://github.com/kubernetes/kubernetes/releases) | | ||
| **alpha/beta release*** | not available. | from [github release page](https://github.com/kubernetes/kubernetes/releases) or from `gs://kubernetes-release/release/` GCS bucket | from `k8s.gcr.io` container registry or from [github release page](https://github.com/kubernetes/kubernetes/releases) | | ||
| **GA release** | from .deb or .rpm repository | from [github release page](https://github.com/kubernetes/kubernetes/releases) or from `https://dl.k8s.io/` release bucket | from `k8s.gcr.io` container registry or from [github release page](https://github.com/kubernetes/kubernetes/releases) | | ||
| **alpha/beta release*** | not available. | from [github release page](https://github.com/kubernetes/kubernetes/releases) or from `https://dl.k8s.io/` release bucket | from `k8s.gcr.io` container registry or from [github release page](https://github.com/kubernetes/kubernetes/releases) | | ||
| **CI/CD release*** | not available. | from `gs://k8s-release-dev/ci/` GCS bucket (built every merge) | from `gcr.io/k8s-staging-ci-images` container registry (built every few hours, not by PR) | |
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.
what is the plan for
gs://k8s-release-dev/ci/
, can we change it with its pot. replacement in this PR?
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.
Asking kubernetes/k8s.io#2396 (comment), rework based on the sig's recommendation.
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.
k8s-release-dev will be tracked separately and isn't meant to be end-user facing (unlike dl.k8s.io), it's meant for contributors to the project
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.
ack
|
||
Pre-compiled GA, alpha/beta versions of kubeadm binary are deployed into `gs://kubernetes-release/release/` GCS bucket, | ||
Pre-compiled GA, alpha/beta versions of kubeadm binary are deployed into `https://dl.k8s.io/release/` release bucket, |
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.
@pacoxu mentioned this URL is a 404.
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.
It should be a 302 if done with curl
. Through the reverse proxy you dan't browse directories which may be why you're getting a 404
but if you look at a specific file, it should be there.
$ curl -s -o /dev/null -w "%{http_code}\n" https://dl.k8s.io/release/
302
$ curl -sL -o /dev/null -w "%{http_code}\n" https://dl.k8s.io/release/
404
$ curl -sL -o /dev/null -w "%{http_code}\n" https://dl.k8s.io/release/stable.txt
200
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.
sgtm,
@pacoxu please let us know if you see a problem with the doc text, as is
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.
/lgtm
|
||
Pre-compiled GA, alpha/beta versions of kubeadm binary are deployed into `gs://kubernetes-release/release/` GCS bucket, | ||
Pre-compiled GA, alpha/beta versions of kubeadm binary are deployed into `https://dl.k8s.io/release/` release bucket, | ||
while CI/CD versions are deployed into `gs://k8s-release-dev/ci/` bucket. |
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.
same Q as above for the CI bucket
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.
k8s-release-dev will be tracked separately and isn't meant to be end-user facing (unlike dl.k8s.io), it's meant for contributors to the project
@@ -167,8 +154,8 @@ Run `cluster/get-kube-binaries.sh` to download the tarball with server binaries. | |||
|
|||
> Inside release notes, usually there is a direct link for getting server binaries directly | |||
|
|||
> `cluster/get-kube-binaries.sh` retrieves server binaries from `gs://kubernetes-release/release/{release}` | |||
GCS bucket; you can use `gsutil` to get server binaries directly. | |||
> `cluster/get-kube-binaries.sh` retrieves server binaries from `https://dl.k8s.io/release/{release}` |
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.
note, in the future we should clean all /cluster/x referenced
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.
Were those fully depricated or were they moved somewhere else?
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.
i haven't checked. we should not ref /cluster tools from k/kubeadm, since /cluster should not be used directly by users. instead users should curl, perhaps.
Co-authored-by: Lubomir I. Ivanov <[email protected]>
so since the scope of the PR is to change release bucket refs we could merge it early, but it means we need to log a k/kubeadm issue with remaining work. e.g.
|
Given the advice from sig-k8s-infra, I think a separate issue would be appropriate. The transition will be a phased effort so I think it's premature to attempt to change all references at once. |
ack, i will log the issue |
I opened #2875, feel free to close. |
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.
/lgtm
/approve
/hold
@pacoxu PTAL and remove the hold if this seems ok to you
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neolit123, pacoxu, rjsadow 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 |
/unhold |
This commit improves the URL handling for resource extraction and version filtering processes. The following changes were made:
In the kinder/pkg/extract/extract.go file:
The releaseBuildURepository constant was updated to use the new "https://dl.k8s.io/release" URL for release builds. In the tests/e2e/manifests/verify_manifest_lists.go file:
The URL used in the filterVersions() function was updated to use the new "https://dl.k8s.io/release" URL for retrieving stable versions.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
There are still references to https://storage.googleapis.com/kubernetes-release instead of https://dl.k8s.io/
dl.k8s.io is the correct advertised download host and will eventually move to be fastly shielding a fully community-owned bucket
ref: kubernetes/k8s.io#2396