Skip to content

Commit 985e8d2

Browse files
fabriziopandinifabriziopandini
committed
More comments
1 parent db202e3 commit 985e8d2

File tree

3 files changed

+71
-45
lines changed

3 files changed

+71
-45
lines changed

‎controlplane/kubeadm/api/v1beta1/v1beta2_condition_consts.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@ import clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2020

2121
// KubeadmControlPlane's Available condition and corresponding reasons that will be used in v1Beta2 API version.
2222
const (
23-
// KubeadmControlPlaneAvailableV1Beta2Condition is true if KubeadmControlPlane not deleted, `CertificatesAvailable` is true,
24-
// at least one Machine with Kubernetes API server, scheduler and controller manager healthy,
25-
// and etcd has enough operational members to meet quorum requirements.
23+
// KubeadmControlPlaneAvailableV1Beta2Condition is true if KubeadmControlPlane is not deleted, `CertificatesAvailable` is true,
24+
// at least one Machine with healthy control plane components, and etcd has enough operational members to meet quorum requirements.
2625
// More specifically, considering how kubeadm layouts components:
2726
// - Kubernetes API server, scheduler and controller manager health is inferred by the status of
2827
// the corresponding Pods hosted on each machine.
29-
// - In case of managed etcd, also an healthy etcd Pod and an healthy etcd member must exist on the same
28+
// - In case of managed etcd, also a healthy etcd Pod and a healthy etcd member must exist on the same
3029
// machine with the healthy Kubernetes API server, scheduler and controller manager, otherwise the k8s control
31-
// plane cannot be considered operational (if etcd is not operational on machine, most likely also API server,
30+
// plane cannot be considered operational (if etcd is not operational on a machine, most likely also API server,
3231
// scheduler and controller manager on the same machine will be impacted).
3332
// - In case of external etcd, KCP cannot make any assumption on etcd status, so all the etcd checks are skipped.
3433
KubeadmControlPlaneAvailableV1Beta2Condition = clusterv1.AvailableV1Beta2Condition

‎controlplane/kubeadm/internal/workload_cluster_conditions.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,13 @@ func compareMachinesAndMembers(controlPlane *ControlPlane, nodes *corev1.NodeLis
354354
// Instead, surface there is a machine without etcd member on kcp's' Available condition
355355
// only if the machine is not deleting and the node exists by more than two minutes
356356
// (this prevents the condition to flick during scale up operations).
357-
// Note: Two minutes is an arbitrary interval where we expect the system to detect the new etcd member on the machine.
357+
// Note: Two minutes is the time after which we expect the system to detect the new etcd member on the machine.
358358
if machine.DeletionTimestamp.IsZero() {
359-
oldNode := true
359+
oldNode := false
360360
if nodes != nil {
361361
for _, node := range nodes.Items {
362-
if machine.Status.NodeRef.Name == node.Name && time.Since(node.CreationTimestamp.Time) <= 2*time.Minute {
363-
oldNode = false
362+
if machine.Status.NodeRef.Name == node.Name && time.Since(node.CreationTimestamp.Time) > 2*time.Minute {
363+
oldNode = true
364364
}
365365
}
366366
}
@@ -386,7 +386,7 @@ func compareMachinesAndMembers(controlPlane *ControlPlane, nodes *corev1.NodeLis
386386
}
387387
}
388388
if !found {
389-
// Surface there is an etcd member without a machine on into the EtcdClusterHealthy condition on kcp.
389+
// Surface there is an etcd member without a machine into the EtcdClusterHealthy condition on kcp.
390390
name := member.Name
391391
if name == "" {
392392
name = fmt.Sprintf("%d (Name not yet assigned)", member.ID)

‎controlplane/kubeadm/internal/workload_cluster_conditions_test.go

+62-35
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func TestUpdateEtcdConditions(t *testing.T) {
5252
expectedKCPV1Beta2Condition *metav1.Condition
5353
expectedMachineConditions map[string]clusterv1.Conditions
5454
expectedMachineV1Beta2Conditions map[string][]metav1.Condition
55-
expectedEtcdMembers []*etcd.Member
55+
expectedEtcdMembers []string
5656
expectedEtcdMembersAgreeOnMemberList bool
5757
expectedEtcdMembersAgreeOnClusterID bool
5858
expectedEtcdMembersAndMachinesAreMatching bool
@@ -82,9 +82,9 @@ func TestUpdateEtcdConditions(t *testing.T) {
8282
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionUnknown, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberInspectionFailedV1Beta2Reason, Message: "Failed to get the Node hosting the etcd member"},
8383
},
8484
},
85-
expectedEtcdMembersAgreeOnMemberList: false, // without reading notes, we can make assumptions.
86-
expectedEtcdMembersAgreeOnClusterID: false, // without reading notes, we can make assumptions.
87-
expectedEtcdMembersAndMachinesAreMatching: false, // without reading notes, we can make assumptions.
85+
expectedEtcdMembersAgreeOnMemberList: false, // without reading nodes, we can not make assumptions.
86+
expectedEtcdMembersAgreeOnClusterID: false, // without reading nodes, we can not make assumptions.
87+
expectedEtcdMembersAndMachinesAreMatching: false, // without reading nodes, we can not make assumptions.
8888
},
8989
{
9090
name: "If there are provisioning machines, a node without machine should be ignored in v1beta1, reported in v1beta2",
@@ -111,9 +111,9 @@ func TestUpdateEtcdConditions(t *testing.T) {
111111
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionUnknown, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberInspectionFailedV1Beta2Reason, Message: "Node does not exist"},
112112
},
113113
},
114-
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can make assumptions.
115-
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can make assumptions.
116-
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can make assumptions.
114+
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can not make assumptions.
115+
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can not make assumptions.
116+
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can not make assumptions.
117117
},
118118
{
119119
name: "If there are no provisioning machines, a node without machine should be reported as False condition at KCP level",
@@ -130,9 +130,9 @@ func TestUpdateEtcdConditions(t *testing.T) {
130130
Reason: controlplanev1.KubeadmControlPlaneEtcdClusterNotHealthyV1Beta2Reason,
131131
Message: "Control plane Node n1 does not have a corresponding Machine",
132132
},
133-
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can make assumptions.
134-
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can make assumptions.
135-
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can make assumptions.
133+
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can not make assumptions.
134+
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can not make assumptions.
135+
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can not make assumptions.
136136
},
137137
{
138138
name: "failure creating the etcd client should report unknown condition",
@@ -164,9 +164,9 @@ func TestUpdateEtcdConditions(t *testing.T) {
164164
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionUnknown, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberInspectionFailedV1Beta2Reason, Message: "Failed to connect to the etcd Pod on the n1 Node: failed to get client for node"},
165165
},
166166
},
167-
expectedEtcdMembersAgreeOnMemberList: false, // failure in reading members, we can make assumptions.
168-
expectedEtcdMembersAgreeOnClusterID: false, // failure in reading members, we can make assumptions.
169-
expectedEtcdMembersAndMachinesAreMatching: false, // failure in reading members, we can make assumptions.
167+
expectedEtcdMembersAgreeOnMemberList: false, // failure in reading members, we can not make assumptions.
168+
expectedEtcdMembersAgreeOnClusterID: false, // failure in reading members, we can not make assumptions.
169+
expectedEtcdMembersAndMachinesAreMatching: false, // failure in reading members, we can not make assumptions.
170170
},
171171
{
172172
name: "etcd client reporting status errors should be reflected into a false condition",
@@ -203,9 +203,9 @@ func TestUpdateEtcdConditions(t *testing.T) {
203203
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberNotHealthyV1Beta2Reason, Message: "Etcd reports errors: some errors"},
204204
},
205205
},
206-
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can make assumptions.
207-
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can make assumptions.
208-
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can make assumptions.
206+
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can not make assumptions.
207+
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can not make assumptions.
208+
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can not make assumptions.
209209
},
210210
{
211211
name: "failure listing members should report false condition in v1beta1, unknown in v1beta2",
@@ -242,9 +242,9 @@ func TestUpdateEtcdConditions(t *testing.T) {
242242
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionUnknown, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberInspectionFailedV1Beta2Reason, Message: "Failed to get answer from the etcd member on the n1 Node: failed to get list of members for etcd cluster: failed to list members"},
243243
},
244244
},
245-
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can make assumptions.
246-
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can make assumptions.
247-
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can make assumptions.
245+
expectedEtcdMembersAgreeOnMemberList: false, // without reading members, we can not make assumptions.
246+
expectedEtcdMembersAgreeOnClusterID: false, // without reading members, we can not make assumptions.
247+
expectedEtcdMembersAndMachinesAreMatching: false, // without reading members, we can not make assumptions.
248248
},
249249
{
250250
name: "an etcd member with alarms should report false condition",
@@ -290,6 +290,7 @@ func TestUpdateEtcdConditions(t *testing.T) {
290290
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberNotHealthyV1Beta2Reason, Message: "Etcd reports alarms: NOSPACE"},
291291
},
292292
},
293+
expectedEtcdMembers: []string{"n1"},
293294
expectedEtcdMembersAgreeOnMemberList: true,
294295
expectedEtcdMembersAgreeOnClusterID: true,
295296
expectedEtcdMembersAndMachinesAreMatching: true,
@@ -375,6 +376,7 @@ func TestUpdateEtcdConditions(t *testing.T) {
375376
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberNotHealthyV1Beta2Reason, Message: "Etcd member has cluster ID 2, but all previously seen etcd members have cluster ID 1"},
376377
},
377378
},
379+
expectedEtcdMembers: []string{"n1", "n2"},
378380
expectedEtcdMembersAgreeOnMemberList: true,
379381
expectedEtcdMembersAgreeOnClusterID: false,
380382
expectedEtcdMembersAndMachinesAreMatching: false,
@@ -460,6 +462,7 @@ func TestUpdateEtcdConditions(t *testing.T) {
460462
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberNotHealthyV1Beta2Reason, Message: "The etcd member hosted on this Machine reports the cluster is composed by [n2 n3], but all previously seen etcd members are reporting [n1 n2]"},
461463
},
462464
},
465+
expectedEtcdMembers: []string{"n1", "n2"},
463466
expectedEtcdMembersAgreeOnMemberList: false,
464467
expectedEtcdMembersAgreeOnClusterID: true,
465468
expectedEtcdMembersAndMachinesAreMatching: false,
@@ -527,6 +530,7 @@ func TestUpdateEtcdConditions(t *testing.T) {
527530
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberNotHealthyV1Beta2Reason, Message: "Etcd doesn't have an etcd member for Node n2"},
528531
},
529532
},
533+
expectedEtcdMembers: []string{"n1"},
530534
expectedEtcdMembersAgreeOnMemberList: true,
531535
expectedEtcdMembersAgreeOnClusterID: true,
532536
expectedEtcdMembersAndMachinesAreMatching: false,
@@ -611,6 +615,7 @@ func TestUpdateEtcdConditions(t *testing.T) {
611615
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition, Status: metav1.ConditionTrue, Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Reason, Message: ""},
612616
},
613617
},
618+
expectedEtcdMembers: []string{"n1", "n2"},
614619
expectedEtcdMembersAgreeOnMemberList: true,
615620
expectedEtcdMembersAgreeOnClusterID: true,
616621
expectedEtcdMembersAndMachinesAreMatching: true,
@@ -668,6 +673,12 @@ func TestUpdateEtcdConditions(t *testing.T) {
668673
g.Expect(controlPane.EtcdMembersAgreeOnMemberList).To(Equal(tt.expectedEtcdMembersAgreeOnMemberList), "EtcdMembersAgreeOnMemberList does not match")
669674
g.Expect(controlPane.EtcdMembersAgreeOnClusterID).To(Equal(tt.expectedEtcdMembersAgreeOnClusterID), "EtcdMembersAgreeOnClusterID does not match")
670675
g.Expect(controlPane.EtcdMembersAndMachinesAreMatching).To(Equal(tt.expectedEtcdMembersAndMachinesAreMatching), "EtcdMembersAndMachinesAreMatching does not match")
676+
677+
var membersNames []string
678+
for _, m := range controlPane.EtcdMembers {
679+
membersNames = append(membersNames, m.Name)
680+
}
681+
g.Expect(membersNames).To(Equal(tt.expectedEtcdMembers))
671682
})
672683
}
673684
}
@@ -1672,7 +1683,6 @@ func TestCompareMachinesAndMembers(t *testing.T) {
16721683
controlPlane *ControlPlane
16731684
nodes *corev1.NodeList
16741685
members []*etcd.Member
1675-
kcpErrors []string
16761686
expectMembersAndMachinesAreMatching bool
16771687
expectKCPErrors []string
16781688
}{
@@ -1684,7 +1694,6 @@ func TestCompareMachinesAndMembers(t *testing.T) {
16841694
},
16851695
members: nil,
16861696
nodes: nil,
1687-
kcpErrors: nil,
16881697
expectMembersAndMachinesAreMatching: true,
16891698
expectKCPErrors: nil,
16901699
},
@@ -1696,7 +1705,6 @@ func TestCompareMachinesAndMembers(t *testing.T) {
16961705
},
16971706
members: nil,
16981707
nodes: nil,
1699-
kcpErrors: nil,
17001708
expectMembersAndMachinesAreMatching: false,
17011709
expectKCPErrors: nil,
17021710
},
@@ -1713,51 +1721,52 @@ func TestCompareMachinesAndMembers(t *testing.T) {
17131721
{Name: "m1"},
17141722
{Name: "m2"},
17151723
},
1716-
kcpErrors: nil,
17171724
nodes: nil,
17181725
expectMembersAndMachinesAreMatching: true,
17191726
expectKCPErrors: nil,
17201727
},
17211728
{
1722-
name: "false if there is a machine without a member",
1729+
name: "true if there is a machine without a member but at least a machine is still provisioning",
17231730
controlPlane: &ControlPlane{
17241731
KCP: &controlplanev1.KubeadmControlPlane{},
17251732
Machines: collections.FromMachines(
17261733
fakeMachine("m1", withNodeRef("m1")),
17271734
fakeMachine("m2", withNodeRef("m2")),
1735+
fakeMachine("m3"), // m3 is still provisioning
17281736
),
17291737
},
17301738
members: []*etcd.Member{
1731-
// m1 is missing
1739+
{Name: "m1"},
17321740
{Name: "m2"},
1741+
// m3 is missing
17331742
},
1734-
kcpErrors: nil,
17351743
nodes: nil,
1736-
expectMembersAndMachinesAreMatching: false,
1744+
expectMembersAndMachinesAreMatching: true,
17371745
expectKCPErrors: nil,
17381746
},
17391747
{
1740-
name: "true if there is a machine without a member but at least a machine is still provisioning",
1748+
name: "true if there is a machine without a member but node on this machine does not exist yet",
17411749
controlPlane: &ControlPlane{
17421750
KCP: &controlplanev1.KubeadmControlPlane{},
17431751
Machines: collections.FromMachines(
17441752
fakeMachine("m1", withNodeRef("m1")),
17451753
fakeMachine("m2", withNodeRef("m2")),
1746-
fakeMachine("m3"), // m3 is still provisioning
1754+
fakeMachine("m3", withNodeRef("m3")),
17471755
),
17481756
},
17491757
members: []*etcd.Member{
17501758
{Name: "m1"},
17511759
{Name: "m2"},
17521760
// m3 is missing
17531761
},
1754-
kcpErrors: nil,
1755-
nodes: nil,
1762+
nodes: &corev1.NodeList{Items: []corev1.Node{
1763+
// m3 is missing
1764+
}},
17561765
expectMembersAndMachinesAreMatching: true,
17571766
expectKCPErrors: nil,
17581767
},
17591768
{
1760-
name: "true if there is a machine without a member bu node on this machine has been just created",
1769+
name: "true if there is a machine without a member but node on this machine has been just created",
17611770
controlPlane: &ControlPlane{
17621771
KCP: &controlplanev1.KubeadmControlPlane{},
17631772
Machines: collections.FromMachines(
@@ -1774,7 +1783,27 @@ func TestCompareMachinesAndMembers(t *testing.T) {
17741783
nodes: &corev1.NodeList{Items: []corev1.Node{
17751784
{ObjectMeta: metav1.ObjectMeta{Name: "m3", CreationTimestamp: metav1.Time{Time: time.Now().Add(-110 * time.Second)}}}, // m3 is just provisioned
17761785
}},
1777-
kcpErrors: nil,
1786+
expectMembersAndMachinesAreMatching: true,
1787+
expectKCPErrors: nil,
1788+
},
1789+
{
1790+
name: "false if there is a machine without a member and node on this machine is old",
1791+
controlPlane: &ControlPlane{
1792+
KCP: &controlplanev1.KubeadmControlPlane{},
1793+
Machines: collections.FromMachines(
1794+
fakeMachine("m1", withNodeRef("m1")),
1795+
fakeMachine("m2", withNodeRef("m2")),
1796+
fakeMachine("m3", withNodeRef("m3")),
1797+
),
1798+
},
1799+
members: []*etcd.Member{
1800+
{Name: "m1"},
1801+
{Name: "m2"},
1802+
// m3 is missing
1803+
},
1804+
nodes: &corev1.NodeList{Items: []corev1.Node{
1805+
{ObjectMeta: metav1.ObjectMeta{Name: "m3", CreationTimestamp: metav1.Time{Time: time.Now().Add(10 * time.Minute)}}}, // m3 is old
1806+
}},
17781807
expectMembersAndMachinesAreMatching: true,
17791808
expectKCPErrors: nil,
17801809
},
@@ -1791,7 +1820,6 @@ func TestCompareMachinesAndMembers(t *testing.T) {
17911820
{Name: "m1"},
17921821
{Name: "m2"},
17931822
},
1794-
kcpErrors: nil,
17951823
nodes: nil,
17961824
expectMembersAndMachinesAreMatching: false,
17971825
expectKCPErrors: []string{"Etcd member m2 does not have a corresponding Machine"},
@@ -1809,7 +1837,6 @@ func TestCompareMachinesAndMembers(t *testing.T) {
18091837
{Name: "m1"},
18101838
{Name: "m2"},
18111839
},
1812-
kcpErrors: nil,
18131840
nodes: nil,
18141841
expectMembersAndMachinesAreMatching: true,
18151842
expectKCPErrors: []string{"Etcd member m2 does not have a corresponding Machine"},

0 commit comments

Comments
 (0)