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.

Sign up for GitHub

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

Jump to bottom

[2.12] Add 1.16.14 and 1.16.15 support #6583

Merged
merged 3 commits into from
Sep 18, 2020
Merged

[2.12] Add 1.16.14 and 1.16.15 support #6583

merged 3 commits into from
Sep 18, 2020

Conversation

linkvt
Copy link
Contributor

@linkvt linkvt commented Aug 25, 2020 •
edited
Loading

What type of PR is this?
/kind feature

What this PR does / why we need it:
Add hashes for K8s 1.16.14, fixes a bug with kubeadm setting incorrect livenessProbes to insecure controller-manager and scheduler ports (10252/10251) instead of the secure ones (10259/10257).
Also switch to k8s.gcr.io as in the master branch.
Edit also adds 1.16.15 hashes now.

See #6506, this is maybe also needed for 1.17 and 1.18?

Which issue(s) this PR fixes:
Only partially solves #6506 for 1.16.x
Fixes #6653

Special notes for your reviewer:
I did the same changes as with the 1.16.13 versions in #6477
I used the download_hash.py script from the master branch.
Besides that, the 1.16.14 image seems to be available only in k8s.gcr.io: https://console.cloud.google.com/gcr/images/google-containers/GLOBAL/kube-apiserver?tag=v1.16.14&gcrImageListsize=30
Will test this in our DEV cluster on the next working day but the changes should work in theory as they are the same as in the referenced PR for 1.16.13.

Does this PR introduce a user-facing change?:

None

EppO and others added 2 commits August 25, 2020 09:32
@EppO @linkvt
Use k8s.gcr.io for kubernetes related images (#5764)
8d56ae3 
* Use k8s.gcr.io for kubernetes related images

* Use k8s.gcr.io in inventory sample
@linkvt
[2.12] Update hashes and set default version to 1.16.14
efd6242
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 25, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @linkvt. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 25, 2020
@floryut
Copy link
Member

floryut commented Aug 25, 2020 •
edited
Loading

/ok-to-test
But I'll move this to hold as 1.16.15, which is the final 1.16.x version, will be release in a few day, let's bundle this with 2.12 branch and be done with it 👍
1.16.15 | 2020-08-28 | 2020-09-02

/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 25, 2020
@linkvt
Copy link
Contributor Author

linkvt commented Aug 25, 2020 •
edited
Loading

@floryut Thanks I didn't know that!
I mean I wouldn't wait as everybody with the 1.16.13 has currently a broken installation due to the kubeadm bug with the incorrect livenessprobe, and IMO waiting doesn't give us any benefit, generating the hashes is not really a problem and will need another commit anyways.

I understand you want to wait for 1.16.15 for the last 2.12.X release, but I think it doesn't hurt to also have 1.16.14 support in release-2.12 branch.

@floryut
Copy link
Member

floryut commented Aug 26, 2020

@floryut Thanks I didn't know that!
I mean I wouldn't wait as everybody with the 1.16.13 has currently a broken installation due to the kubeadm bug with the incorrect livenessprobe, and IMO waiting doesn't give us any benefit, generating the hashes is not really a problem and will need another commit anyways.

I understand you want to wait for 1.16.15 for the last 2.12.X release, but I think it doesn't hurt to also have 1.16.14 support in release-2.12 branch.

Well we can merge it, but I'm not sure maintainers wants to trigger a new tag and everything, but up to them indeed

@floryut
Copy link
Member

floryut commented Aug 27, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 27, 2020
@Miouge1 Miouge1 changed the title Add 1.16.14 support [2.12] Add 1.16.14 support Aug 27, 2020
@Miouge1
Copy link
Contributor

Miouge1 commented Aug 27, 2020

This is a backport PR, therefore we nee to merge it first in master, and release-2.13.

@floryut
Copy link
Member

floryut commented Aug 27, 2020

This is a backport PR, therefore we nee to merge it first in master, and release-2.13.

1.16.14 hashes are already in master :)

@Miouge1
Copy link
Contributor

Miouge1 commented Aug 31, 2020

1.16.14 hashes are in master since #6532, but not in release-2.13, so that's still a blocker to merge this PR.

@floryut
Copy link
Member

floryut commented Aug 31, 2020

1.16.14 hashes are in master since #6532, but not in release-2.13, so that's still a blocker to merge this PR.

Oh indeed, I was lazy 😄

@floryut
Copy link
Member

floryut commented Sep 7, 2020

@linkvt Could you please add 1.16.15 and set it as default ?
I've merged them in master and 2.13

@floryut
Copy link
Member

floryut commented Sep 10, 2020

@linkvt Are you available ? Otherwise we can close and reopen a PR

@linkvt
Copy link
Contributor Author

linkvt commented Sep 10, 2020

Hi @floryut, sorry for the late reply, I'm unavailable till Sunday. If you think that's too late feel free to modify this or reopen it. Thanks!

@floryut
Copy link
Member

floryut commented Sep 10, 2020

Hi @floryut, sorry for the late reply, I'm unavailable till Sunday. If you think that's too late feel free to modify this or reopen it. Thanks!

No worries that can wait for a few days, thank you

@floryut floryut mentioned this pull request Sep 10, 2020
Closed
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 13, 2020
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 13, 2020
@linkvt
Copy link
Contributor Author

linkvt commented Sep 13, 2020

@floryut done, I didn't test it myself but I guess the CI should setup a cluster with v1.16.15.

@linkvt linkvt changed the title [2.12] Add 1.16.14 support [2.12] Add 1.16.15 support Sep 13, 2020
@linkvt linkvt changed the title [2.12] Add 1.16.15 support [2.12] Add 1.16.14 and 1.16.15 support Sep 13, 2020
@floryut
Copy link
Member

floryut commented Sep 14, 2020 •
edited
Loading

@floryut done, I didn't test it myself but I guess the CI should setup a cluster with v1.16.15.

It does set it up and test it, all good for me, thank you
/unhold
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Sep 14, 2020
@floryut
Copy link
Member

floryut commented Sep 14, 2020

/assign @Miouge1
up to you :)

@Miouge1
Copy link
Contributor

Miouge1 commented Sep 18, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: linkvt, Miouge1

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 Sep 18, 2020
@k8s-ci-robot k8s-ci-robot merged commit 093d75f into kubernetes-sigs:release-2.12 Sep 18, 2020
@BirgerK
Copy link

BirgerK commented Oct 26, 2020

When will this change result in a new v2.12.X-tag?

@BirgerK
Copy link

BirgerK commented Oct 30, 2020 •
edited
Loading

When will this change result in a new v2.12.X-tag? @Miouge1

@oomichi oomichi mentioned this pull request May 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chadswen chadswen Awaiting requested review from chadswen

@holmsten holmsten Awaiting requested review from holmsten

Assignees

@Miouge1 Miouge1

@floryut floryut

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. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@linkvt @k8s-ci-robot @floryut @Miouge1 @BirgerK @EppO