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.

Sign up for GitHub

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

Jump to bottom

Fix kata node selector handling #275

Merged
merged 4 commits into from
Mar 10, 2023
Merged

Fix kata node selector handling #275

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af48735
introduce helper functions for two types of kata node selector
Mar 9, 2023
1e1f013
update KataConfigNodeSelector-using call sites to use new helpers
Mar 9, 2023
2805eb0
update NodeSelector-using sites to use new helpers
Mar 9, 2023
af1441c
clean up by removing obsolete functions
Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
115 changes: 61 additions & 54 deletions controllers/openshift_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
labelsutil "k8s.io/kubernetes/pkg/util/labels"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/util/workqueue"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -404,8 +405,6 @@ func (r *KataConfigOpenShiftReconciler) newMCPforCR() *mcfgv1.MachineConfigPool
Values: []string{"kata-oc", "worker"},
}

mcpNodeSelector := &metav1.LabelSelector{MatchLabels: map[string]string{"node-role.kubernetes.io/kata-oc": ""}}

mcp := &mcfgv1.MachineConfigPool{
TypeMeta: metav1.TypeMeta{
APIVersion: "machineconfiguration.openshift.io/v1",
Expand All @@ -427,7 +426,7 @@ func (r *KataConfigOpenShiftReconciler) newMCPforCR() *mcfgv1.MachineConfigPool
MachineConfigSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{lsr},
},
NodeSelector: mcpNodeSelector,
NodeSelector: r.getNodeSelectorAsLabelSelector(),
},
}

Expand Down Expand Up @@ -559,51 +558,6 @@ func (r *KataConfigOpenShiftReconciler) checkNodeEligibility() error {
return nil
}

func (r *KataConfigOpenShiftReconciler) getNodeSelectorAsMap() map[string]string {
r.Log.Info("Getting NodeSelector")

// NodeSelector field in RuntimeClass and PodSpec is key:value map
nodeSelector := make(map[string]string)

isConvergedCluster, err := r.checkConvergedCluster()
if err == nil && isConvergedCluster {
// master MCP cannot be customized
nodeSelector["node-role.kubernetes.io/master"] = ""
} else {
nodeSelector["node-role.kubernetes.io/kata-oc"] = ""

if r.kataConfig.Spec.CheckNodeEligibility {
nodeSelector["feature.node.kubernetes.io/runtime.kata"] = "true"
}

if r.kataConfig.Spec.KataConfigPoolSelector != nil {
r.Log.Info("KataConfigPoolSelector:", "r.kataConfig.Spec.KataConfigPoolSelector", r.kataConfig.Spec.KataConfigPoolSelector)
lsMap, err := metav1.LabelSelectorAsMap(r.kataConfig.Spec.KataConfigPoolSelector)
if err != nil {
r.Log.Error(err, "Unable to get nodeSelector from KataConfigPoolSelector ")
} else {
// Add the labels to nodeSelector
for k, v := range lsMap {
nodeSelector[k] = v
}
}
}
}
r.Log.Info("Nodeselector", "nodeSelector", nodeSelector)
return nodeSelector
}

func (r *KataConfigOpenShiftReconciler) getNodeSelectorAsMapNoKataOc() map[string]string {
nodeSelector := r.getNodeSelectorAsMap()
delete(nodeSelector, "node-role.kubernetes.io/kata-oc")
return nodeSelector
}

func (r *KataConfigOpenShiftReconciler) getNodeSelectorAsSelector() (labels.Selector, error) {
nodeSelector := r.getNodeSelectorAsMapNoKataOc()
return metav1.LabelSelectorAsSelector( &metav1.LabelSelector { MatchLabels: nodeSelector, })
}

func (r *KataConfigOpenShiftReconciler) getMcpName() (string, error) {
r.Log.Info("Getting MachineConfigPool Name")

Expand Down Expand Up @@ -710,6 +664,53 @@ func (r *KataConfigOpenShiftReconciler) createRuntimeClass() error {
return nil
}

// "KataConfigNodeSelector" in the names of the following couple of helper
// functions refers to the value of KataConfig.spec.kataConfigPoolSelector,
// i.e. the original selector supplied by the user of KataConfig.
func (r *KataConfigOpenShiftReconciler) getKataConfigNodeSelectorAsLabelSelector() *metav1.LabelSelector {

isConvergedCluster, err := r.checkConvergedCluster()
if err == nil && isConvergedCluster {
// master MCP cannot be customized
return &metav1.LabelSelector { MatchLabels: map[string]string{ "node-role.kubernetes.io/master": "" }}
}

nodeSelector := r.kataConfig.Spec.KataConfigPoolSelector.DeepCopy()
if r.kataConfig.Spec.CheckNodeEligibility {
nodeSelector = labelsutil.AddLabelToSelector(nodeSelector, "feature.node.kubernetes.io/runtime.kata", "true")
}
return nodeSelector
}

func (r *KataConfigOpenShiftReconciler) getKataConfigNodeSelectorAsSelector() (labels.Selector, error) {
selector, err := metav1.LabelSelectorAsSelector(r.getKataConfigNodeSelectorAsLabelSelector())
r.Log.Info("getKataConfigNodeSelectorAsSelector()", "selector", selector)
return selector, err
}

// "NodeSelector" in the names of the following couple of helper
// functions refers to the selector we pass to resources we create that
// need to select kata-enabled nodes (currently the "kata-oc" MCP, the pod
// template in the monitor daemonset and the runtimeclass). It's guaranteed
// to be a simple map[string]string (AKA MatchLabels) which is good because the
// pod template's and runtimeclass' node selectors don't support
// MatchExpressions and thus cannot hold the full value of
// KataConfig.spec.kataConfigPoolSelector.
func (r *KataConfigOpenShiftReconciler) getNodeSelectorAsMap() map[string]string {

isConvergedCluster, err := r.checkConvergedCluster()
if err == nil && isConvergedCluster {
// master MCP cannot be customized
return map[string]string{ "node-role.kubernetes.io/master": "" }
} else {
return map[string]string{"node-role.kubernetes.io/kata-oc": ""}
}
}

func (r *KataConfigOpenShiftReconciler) getNodeSelectorAsLabelSelector() *metav1.LabelSelector {
return &metav1.LabelSelector{MatchLabels: r.getNodeSelectorAsMap()}
}

func (r *KataConfigOpenShiftReconciler) processKataConfigDeleteRequest() (ctrl.Result, error) {
r.Log.Info("KataConfig deletion in progress: ")
machinePool, err := r.getMcpName()
Expand Down Expand Up @@ -744,7 +745,14 @@ func (r *KataConfigOpenShiftReconciler) processKataConfigDeleteRequest() (ctrl.R
}
}
}
err = r.unlabelNodes(&metav1.LabelSelector{MatchLabels: r.getNodeSelectorAsMapNoKataOc()})

kataNodeSelector, err := r.getKataConfigNodeSelectorAsSelector()
if err != nil {
r.Log.Info("Couldn't get node selector for unlabelling nodes", "err", err)
return ctrl.Result{Requeue: true}, nil
}
err = r.unlabelNodes(kataNodeSelector)

if err != nil {
if k8serrors.IsConflict(err) {
return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, nil
Expand Down Expand Up @@ -1184,7 +1192,7 @@ func (eh *McpEventHandler) Generic(event event.GenericEvent, queue workqueue.Rat


func (r *KataConfigOpenShiftReconciler) nodeMatchesKataSelector(nodeLabels map[string]string) bool {
nodeSelector, err := r.getNodeSelectorAsSelector()
nodeSelector, err := r.getKataConfigNodeSelectorAsSelector()

if err != nil {
r.Log.Info("couldn't get kata node selector", "err", err)
Expand Down Expand Up @@ -1387,7 +1395,7 @@ func (r *KataConfigOpenShiftReconciler) updateNodeLabels() (err error) {
return err
}

kataNodeSelector, _ := r.getNodeSelectorAsSelector()
kataNodeSelector, _ := r.getKataConfigNodeSelectorAsSelector()

for _, worker := range workerNodeList.Items {
workerMatchesKata := kataNodeSelector.Matches(labels.Set(worker.Labels))
Expand Down Expand Up @@ -1417,11 +1425,10 @@ func (r *KataConfigOpenShiftReconciler) updateNodeLabels() (err error) {
return nil
}

func (r *KataConfigOpenShiftReconciler) unlabelNodes(nodeSelector *metav1.LabelSelector) (err error) {
labelSelector, _ := metav1.LabelSelectorAsSelector(nodeSelector)
func (r *KataConfigOpenShiftReconciler) unlabelNodes(nodeSelector labels.Selector) (err error) {
nodeList := &corev1.NodeList{}
listOpts := []client.ListOption{
client.MatchingLabelsSelector{Selector: labelSelector},
client.MatchingLabelsSelector{Selector: nodeSelector},
}

if err := r.Client.List(context.TODO(), nodeList, listOpts...); err != nil {
Expand Down