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.

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

Improve node matching #98

Merged
merged 2 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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