-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 Github download url for nodeup #8468
Conversation
Currently this generates: ``` NODEUP_URL=https://artifacts.k8s.io/binaries/kops/1.15.1/linux/amd64/nodeup,https://github.com/kubernetes/kops/releases/download/1.15.1/linux-amd64-nodeup,https://kubeupv2.s3.amazonaws.com/kops/1.15.1/linux/amd64/nodeup NODEUP_HASH=de4939eadb6e4d89fcf608b1f632e770bcce521d6dc5c45d76d2c4608ad23db4 ``` However for the Github URL a `v` is missing in front of the version tag. Returns a 404: ``` curl https://github.com/kubernetes/kops/releases/download/1.15.1/linux-amd64-nodeup ``` Downloads the file: ``` curl https://github.com/kubernetes/kops/releases/download/v1.15.1/linux-amd64-nodeup ```
Welcome @adri! |
Hi @adri. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
Yeah the name of releases changed to prefix a v. Does this matter for older releases @rifelpet ? |
yes, we updated the tag in 1.15.1 to be prefixed with |
I think the underlying issue is correct - we are moving to (I assume I should be uploading releases against the |
I was wondering if we needed to have logic only to include the @adri if you run |
An alternative but potentially more invasive solution is that we include Lines 24 to 28 in 6c202ef
for reference, both |
From a user perspective: In my logs I saw issues fetching the first 2 of the 3 URLs.
Whatever the version strategy will be, I think the URLs for 1. and 2. should be somehow aligned. |
@rifelpet I don't mind changing the version number to include the I do need to publish these files to artifacts.k8s.io; that's more of a WIP, but having both artifacts.k8s.io and github.com not available will put more load on the 3rd bucket which I'd prefer not to use as our sole source. I think I've just added the result of running |
OK this is passing tests after running the script, so proceeding so we can get the releases going. Thanks for spotting and fixing @adri /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adri, justinsb 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 |
I agree this looks good 👍 |
Cherry pick of #8468 onto release-1.17
Cherry pick of #8468 onto release-1.15
Cherry pick of #8468 onto release-1.16
Currently this generates:
However for the Github URL a
v
is missing in front of the version tag.Returns a 404:
Downloads the file: