This repository has been archived by the owner on Jun 29, 2022. It is now read-only.
Fix controlplane running as DaemonSet on single node clusters #1193
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
3 times, most recently
from
8f885cd
to
47e315b
Compare
|
Had to fix testing logic a bit and extend wait times to make sure we test properly. it should work fine now. |
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
from
47e315b
to
7578210
Compare
|
Tests failed once on AWS, but then passed 3 times, so I think they are stable enough. |
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
from
f374fc6
to
a72fb81
Compare
5 tasks
|
@invidian I can't review this right now due to load, sorry. Please assign someone else. |
surajssd
reviewed
Nov 20, 2020
| @@ -164,7 +164,9 @@ func WaitForDaemonSet(t *testing.T, client kubernetes.Interface, ns, name string | |||
| return false, nil | |||
| } | |||
|
|
|||
| return false, fmt.Errorf("getting DaemonSet %q: %w", name, err) | |||
| t.Logf("getting DaemonSet %q: %v", name, err) | |||
There was a problem hiding this comment.
Depending on the main-retry-loop to retry can elongate the time for test if there actually is error and not something transient. We might think about introducing shorter retry for such stuff. This does not have to be here, could reside outside as a helper func.
There was a problem hiding this comment.
Right. But on the other hand, it also gives more time to investigate when you see that it loops here.
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
from
a72fb81
to
5e102c3
Compare
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
2 times, most recently
from
64373f7
to
29ff284
Compare
iaguis
suggested changes
Nov 24, 2020
test/components/kubernetes/controlplane-multi-node_disruptive_test.go
Outdated
Show resolved
Hide resolved
test/components/kubernetes/controlplane-multi-node_disruptive_test.go
Outdated
Show resolved
Hide resolved
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
from
29ff284
to
191d089
Compare
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
2 times, most recently
from
8b1cccf
to
8bc1cc5
Compare
Thanks @iaguis :)
I'll actually test that before merging. |
Tested, it magically works completely fine 😄 |
surajssd
previously approved these changes
Nov 27, 2020
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
4 times, most recently
from
aea0ded
to
b5bc81d
Compare
surajssd
reviewed
Dec 3, 2020
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
from
58c6cdf
to
a5e256d
Compare
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
To support verifying that given DaemonSet or Deployment has been updated, this commit adds extra condition for WaitForDaemonSet() and WaitForDeployment() functions before converging and extends log format to also include new information. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
This commit fixes passed 'control_plane_replicas' value to Kubernetes Helm chart which caused kube-scheduler and kube-controller-manager to run as DaemonSet on single controlplane node clusters, which breaks the ability to update it gracefully. It also adds tests that controlplane is using right resource type on different controlplane sizes and that both can be gracefully updated without breaking cluster functionality. Closes #1097 Closes #90 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
If we fail to get it and we get some error from API server, hope that this is transient and retry. This should mitigate hitting errors like 'etcdserver: leader changed'. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian
force-pushed
the
invidian/fix-updating-kubernetes-controlplane
branch
from
a5e256d
to
87fde27
Compare
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes passed 'control_plane_replicas' value to Kubernetes
Helm chart which caused kube-scheduler and kube-controller-manager to
run as DaemonSet on single controlplane node clusters, which breaks the
ability to update it gracefully.
It also adds tests that controlplane is using right resource type on
different controlplane sizes and that both can be gracefully updated
without breaking cluster functionality.
Closes #1097
Closes #90
Signed-off-by: Mateusz Gozdek mateusz@kinvolk.io