Skip to content
This repository was archived by the owner on Jun 29, 2022. It is now read-only.

Release v0.5.0#1118

Merged
knrt10 merged 3 commits intomasterfrom
knrt10/release-v0.5.0
Oct 27, 2020
Merged

Release v0.5.0#1118
knrt10 merged 3 commits intomasterfrom
knrt10/release-v0.5.0

Conversation

@knrt10
Copy link
Contributor

@knrt10 knrt10 commented Oct 22, 2020

Signed-off-by: knrt10 kautilya@kinvolk.io

@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 681fae9 to 495c6e7 Compare October 22, 2020 14:06
invidian
invidian reviewed Oct 22, 2020
View reviewed changes
invidian
invidian reviewed Oct 22, 2020
View reviewed changes
invidian
invidian reviewed Oct 22, 2020
View reviewed changes
invidian
invidian previously requested changes Oct 23, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

(Reviewed from GitHub app for Android, let's see how it went 😁)

ipochi
ipochi previously requested changes Oct 23, 2020
View reviewed changes
Copy link
Member

@ipochi ipochi left a comment

Choose a reason for hiding this comment

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

Thanks for the work @knrt10

Left some notes and suggestions.

surajssd
surajssd suggested changes Oct 23, 2020
View reviewed changes
@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 495c6e7 to cecd666 Compare October 23, 2020 07:14
@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from cecd666 to b1e00de Compare October 23, 2020 07:18
surajssd
surajssd previously approved these changes Oct 23, 2020
View reviewed changes
Copy link
Member

@surajssd surajssd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work @knrt10 🔥

@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from b1e00de to a6f6948 Compare October 23, 2020 08:32
@knrt10 knrt10 requested a review from surajssd October 23, 2020 08:33
@knrt10 knrt10 dismissed stale reviews from ipochi and invidian October 23, 2020 08:36

re-requested review

surajssd
surajssd previously approved these changes Oct 23, 2020
View reviewed changes
invidian
invidian suggested changes Oct 23, 2020
View reviewed changes
ipochi
ipochi suggested changes Oct 23, 2020
View reviewed changes
@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from a6f6948 to 3b847f6 Compare October 23, 2020 11:27
surajssd
surajssd previously approved these changes Oct 23, 2020
View reviewed changes
@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 3b847f6 to cb81565 Compare October 23, 2020 11:30
@knrt10 knrt10 requested a review from surajssd October 23, 2020 11:31
iaguis
iaguis suggested changes Oct 27, 2020
View reviewed changes
CHANGELOG.md

**NOTE:** On clusters with a single controller node only, delete the old `kube-apiserver` ReplicaSet during cluster update:
```sh
kubectl delete rs -n kube-system $(kubectl get rs -n kube-system -l k8s-app=kube-apiserver --no-headers=true --sort-by=metadata.creationTimestamp | tac | tail -n +2 | awk '{print $1}') || true
Copy link
Contributor

@iaguis iaguis Oct 27, 2020

Choose a reason for hiding this comment

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

How does this work? This will only apply when there are two apiserver replicasets, right? In that case maybe it makes sense to make a loop so the user can run it while the cluster is updating?

Copy link
Contributor

@iaguis iaguis Oct 27, 2020

Choose a reason for hiding this comment

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

Also, when I tried this the update process got stopped twice, I think we should mention to users that if the update process stops they should try again.

Copy link
Member

@invidian invidian Oct 27, 2020

Choose a reason for hiding this comment

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

Yes, it is expected that update process will get interrupted (as we remove old pod and we must wait for new one).

How does this work? This will only apply when there are two apiserver replicasets, right? In that case maybe it makes sense to make a loop so the user can run it while the cluster is updating?

During the upgrade, the new one should get created, but it will never converge. At this point, removing old one allows it to converge.

If there is just one for whatever reason, this comment just do not anything, so it's safe. It's also idempotent, just in case.

@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 3188cbc to 227cc48 Compare October 27, 2020 13:55
@knrt10 knrt10 requested review from iaguis, invidian and ipochi October 27, 2020 13:59
invidian
invidian reviewed Oct 27, 2020
View reviewed changes
@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 227cc48 to 43371cc Compare October 27, 2020 14:08
@knrt10 knrt10 requested a review from invidian October 27, 2020 14:08
@knrt10
Copy link
Contributor Author

knrt10 commented Oct 27, 2020

Updated

invidian
invidian suggested changes Oct 27, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

2 more things 🙈

@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 43371cc to 3d6bc66 Compare October 27, 2020 14:31
@knrt10 knrt10 requested a review from invidian October 27, 2020 14:31
invidian
invidian previously approved these changes Oct 27, 2020
View reviewed changes
@knrt10 knrt10 dismissed johananl’s stale review October 27, 2020 14:43

changes addressed

iaguis
iaguis suggested changes Oct 27, 2020
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

Two last nits!

knrt10 added 3 commits October 27, 2020 21:35
CHANGELOG.md: add changelog for v0.5.0 release
fb1e577 
Signed-off-by: knrt10 <kautilya@kinvolk.io>
version: bump to v0.5.0
d4861e9 
Signed-off-by: knrt10 <kautilya@kinvolk.io>
version: bump to v0.5.0+git
c0f0ea7 
Signed-off-by: knrt10 <kautilya@kinvolk.io>
@knrt10 knrt10 force-pushed the knrt10/release-v0.5.0 branch from 3d6bc66 to c0f0ea7 Compare October 27, 2020 16:05
@knrt10 knrt10 requested review from iaguis and invidian October 27, 2020 16:06
iaguis
iaguis approved these changes Oct 27, 2020
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

LGTM! Great work @knrt10 (and thanks for bearing with the reviews 🙏).

@knrt10
Copy link
Contributor Author

knrt10 commented Oct 27, 2020

Thank you everyone for your review. Once tests passes, will start the release

@knrt10 knrt10 merged commit 7fe10bf into master Oct 27, 2020
@knrt10 knrt10 mentioned this pull request Oct 27, 2020
Closed
@invidian invidian deleted the knrt10/release-v0.5.0 branch October 27, 2020 18:30
@knrt10 knrt10 mentioned this pull request Jan 4, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@johananl johananl Awaiting requested review from johananl

@surajssd surajssd Awaiting requested review from surajssd

@ipochi ipochi Awaiting requested review from ipochi

2 more reviewers

@iaguis iaguis iaguis approved these changes

@invidian invidian invidian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@knrt10 @iaguis @surajssd @ipochi @johananl @invidian