Skip to content

Commit

Permalink
Improve node matching (#98)
Browse files Browse the repository at this point in the history
* Improve node matching during Kubernetes upgrades

Signed-off-by: Atanas Dinov <[email protected]>

* Improve node matching during OS upgrades

Signed-off-by: Atanas Dinov <[email protected]>

---------

Signed-off-by: Atanas Dinov <[email protected]>
  • Loading branch information
atanasdinov authored Oct 10, 2024
1 parent fbb06fb commit 705093f
Show file tree
Hide file tree
Showing 4 changed files with 238 additions and 222 deletions.
36 changes: 27 additions & 9 deletions internal/controller/reconcile_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ func (r *UpgradePlanReconciler) reconcileKubernetes(
return ctrl.Result{}, r.createObject(ctx, upgradePlan, controlPlanePlan)
}

selector, err := metav1.LabelSelectorAsSelector(controlPlanePlan.Spec.NodeSelector)
nodes, err := findMatchingNodes(nodeList, controlPlanePlan.Spec.NodeSelector)
if err != nil {
return ctrl.Result{}, fmt.Errorf("parsing node selector: %w", err)
return ctrl.Result{}, err
}

if !isKubernetesUpgraded(nodeList, selector, kubernetesVersion) {
if !isKubernetesUpgraded(nodes, kubernetesVersion) {
setInProgressCondition(upgradePlan, conditionType, "Control plane nodes are being upgraded")
return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil
} else if controlPlaneOnlyCluster(nodeList) {
Expand All @@ -66,12 +66,12 @@ func (r *UpgradePlanReconciler) reconcileKubernetes(
return ctrl.Result{}, r.createObject(ctx, upgradePlan, workerPlan)
}

selector, err = metav1.LabelSelectorAsSelector(workerPlan.Spec.NodeSelector)
nodes, err = findMatchingNodes(nodeList, workerPlan.Spec.NodeSelector)
if err != nil {
return ctrl.Result{}, fmt.Errorf("parsing node selector: %w", err)
return ctrl.Result{}, err
}

if !isKubernetesUpgraded(nodeList, selector, kubernetesVersion) {
if !isKubernetesUpgraded(nodes, kubernetesVersion) {
setInProgressCondition(upgradePlan, conditionType, "Worker nodes are being upgraded")
return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil
}
Expand All @@ -97,12 +97,30 @@ func targetKubernetesVersion(nodeList *corev1.NodeList, kubernetes *lifecyclev1a
}
}

func isKubernetesUpgraded(nodeList *corev1.NodeList, selector labels.Selector, kubernetesVersion string) bool {
func findMatchingNodes(nodeList *corev1.NodeList, nodeSelector *metav1.LabelSelector) ([]corev1.Node, error) {
selector, err := metav1.LabelSelectorAsSelector(nodeSelector)
if err != nil {
return nil, fmt.Errorf("parsing node selector: %w", err)
}

var targetNodes []corev1.Node

for _, node := range nodeList.Items {
if !selector.Matches(labels.Set(node.Labels)) {
continue
if selector.Matches(labels.Set(node.Labels)) {
targetNodes = append(targetNodes, node)
}
}

if len(targetNodes) == 0 {
return nil, fmt.Errorf("none of the nodes match label selector: MatchLabels: %s, MatchExpressions: %s",
nodeSelector.MatchLabels, nodeSelector.MatchExpressions)
}

return targetNodes, nil
}

func isKubernetesUpgraded(nodes []corev1.Node, kubernetesVersion string) bool {
for _, node := range nodes {
var nodeReadyStatus corev1.ConditionStatus

for _, condition := range node.Status.Conditions {
Expand Down
235 changes: 136 additions & 99 deletions internal/controller/reconcile_kubernetes_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controller

import (
"slices"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -9,139 +10,175 @@ import (

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
)

func TestIsKubernetesUpgraded(t *testing.T) {
const kubernetesVersion = "v1.30.3+k3s1"

func TestFindMatchingNodes(t *testing.T) {
nodeLabels := map[string]string{
"node-x": "z",
}
nodeSelector := &metav1.LabelSelector{
MatchLabels: nodeLabels,
}

tests := []struct {
name string
nodeList *corev1.NodeList
expectedNodes []string
expectedErr string
}{
{
name: "All nodes match",
nodeList: &corev1.NodeList{
Items: []corev1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "node-1", Labels: nodeLabels}},
{ObjectMeta: metav1.ObjectMeta{Name: "node-2", Labels: nodeLabels}},
{ObjectMeta: metav1.ObjectMeta{Name: "node-3", Labels: nodeLabels}},
},
},
expectedNodes: []string{"node-1", "node-2", "node-3"},
},
{
name: "Some nodes match",
nodeList: &corev1.NodeList{
Items: []corev1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "node-1", Labels: nodeLabels}},
{ObjectMeta: metav1.ObjectMeta{Name: "node-2"}},
{ObjectMeta: metav1.ObjectMeta{Name: "node-3", Labels: nodeLabels}},
},
},
expectedNodes: []string{"node-1", "node-3"},
},
{
name: "No nodes match",
nodeList: &corev1.NodeList{
Items: []corev1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "node-1"}},
{ObjectMeta: metav1.ObjectMeta{Name: "node-2"}},
{ObjectMeta: metav1.ObjectMeta{Name: "node-3"}},
},
},
expectedErr: "none of the nodes match label selector: MatchLabels: map[node-x:z], MatchExpressions: []",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
nodes, err := findMatchingNodes(test.nodeList, nodeSelector)
if test.expectedErr != "" {
require.EqualError(t, err, test.expectedErr)
assert.Nil(t, nodes)
return
}

require.NoError(t, err)
require.Len(t, nodes, len(test.expectedNodes))
for _, expected := range test.expectedNodes {
assert.True(t, slices.ContainsFunc(nodes, func(actual corev1.Node) bool {
return actual.Name == expected
}))
}
})
}
}

func TestIsKubernetesUpgraded(t *testing.T) {
const kubernetesVersion = "v1.30.3+k3s1"

tests := []struct {
name string
nodes *corev1.NodeList
selector labels.Selector
nodes []corev1.Node
expectedUpgrade bool
}{
{
name: "All matching nodes upgraded",
nodes: &corev1.NodeList{
Items: []corev1.Node{
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
name: "All nodes upgraded",
nodes: []corev1.Node{
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
},
selector: labels.SelectorFromSet(nodeLabels),
expectedUpgrade: true,
},
{
name: "Unschedulable matching node",
nodes: &corev1.NodeList{
Items: []corev1.Node{
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: true},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
name: "Unschedulable node",
nodes: []corev1.Node{
{
Spec: corev1.NodeSpec{Unschedulable: true},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
},
selector: labels.SelectorFromSet(nodeLabels),
expectedUpgrade: false,
},
{
name: "Not ready matching node",
nodes: &corev1.NodeList{
Items: []corev1.Node{
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionFalse}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
name: "Not ready node",
nodes: []corev1.Node{
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionFalse}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
},
selector: labels.SelectorFromSet(nodeLabels),
expectedUpgrade: false,
},
{
name: "Matching node on older Kubernetes version",
nodes: &corev1.NodeList{
Items: []corev1.Node{
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
{
ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels},
Spec: corev1.NodeSpec{Unschedulable: false},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
name: "Node on older Kubernetes version",
nodes: []corev1.Node{
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3+k3s1"}},
},
{
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}},
},
},
selector: labels.SelectorFromSet(nodeLabels),
expectedUpgrade: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.expectedUpgrade, isKubernetesUpgraded(test.nodes, test.selector, kubernetesVersion))
assert.Equal(t, test.expectedUpgrade, isKubernetesUpgraded(test.nodes, kubernetesVersion))
})
}
}
Expand Down
Loading

0 comments on commit 705093f

Please sign in to comment.