Skip to content

Commit

Permalink
Changing the Module-NMC reconciliation logic (rh-ecosystem-edge#521) (r…
Browse files Browse the repository at this point in the history
…h-ecosystem-edge#748)

Previous logic:
get all nodes, and per node decide if Module should be running or
not and update NMC accordingly
New logic:
1. get nodes by module selector.
2. get NMCs based on the label of the Module (in case module is/should
   be deployed on the node, a label will be set in NMC).
3. Per node, get the kernel mapping.
4  If kernel mapping exists, then module should be run on the node.
5. Otherwise, check if NMC exists for that node.
6. If it does, then module should be deleted from the node
7. After all the nodes has been process, look at the remaining NMCs:
   in all those node, module should be deleted

In order to support this flow, whenever NMC is configured to deploy the
module, a label: beta.kmm.node.kubernetes.io/nmc.<namespace>.name is
added to the NMC. whenever NMC is configured to undeploy the module, the
label is removed
  • Loading branch information
yevgeny-shnaidman committed Sep 4, 2023
1 parent 5ad5f2a commit 262ed40
Show file tree
Hide file tree
Showing 8 changed files with 367 additions and 224 deletions.
3 changes: 2 additions & 1 deletion internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const (
DevicePluginVersionLabelPrefix = "beta.kmm.node.kubernetes.io/version-device-plugin"
ModuleVersionLabelPrefix = "kmm.node.kubernetes.io/version-module"

ModuleFinalizer = "kmm.node.kubernetes.io/module-finalizer"
ModuleFinalizer = "kmm.node.kubernetes.io/module-finalizer"
ModuleNMCLabelPrefix = "beta.kmm.node.kubernetes.io/nmc"

ManagedClusterModuleNameLabel = "kmm.node.kubernetes.io/managedclustermodule.name"
KernelVersionsClusterClaimName = "kernel-versions.kmm.node.kubernetes.io"
Expand Down
66 changes: 41 additions & 25 deletions internal/controllers/mock_module_nmc_reconciler.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

154 changes: 101 additions & 53 deletions internal/controllers/module_nmc_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -37,8 +38,13 @@ const (
ModuleNMCReconcilerName = "ModuleNMCReconciler"
)

type schedulingData struct {
mld *api.ModuleLoaderData
node *v1.Node
nmcExists bool
}

type ModuleNMCReconciler struct {
kernelAPI module.KernelMapper
filter *filter.Filter
reconHelper moduleNMCReconcilerHelperAPI
}
Expand All @@ -50,9 +56,8 @@ func NewModuleNMCReconciler(client client.Client,
filter *filter.Filter,
authFactory auth.RegistryAuthGetterFactory,
scheme *runtime.Scheme) *ModuleNMCReconciler {
reconHelper := newModuleNMCReconcilerHelper(client, registryAPI, nmcHelper, authFactory, scheme)
reconHelper := newModuleNMCReconcilerHelper(client, kernelAPI, registryAPI, nmcHelper, authFactory, scheme)
return &ModuleNMCReconciler{
kernelAPI: kernelAPI,
filter: filter,
reconHelper: reconHelper,
}
Expand Down Expand Up @@ -85,31 +90,26 @@ func (mnr *ModuleNMCReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, fmt.Errorf("failed to set finalizer on %s Module: %v", req.NamespacedName, err)
}

// get all nodes
nodes, err := mnr.reconHelper.getNodesList(ctx)
// get nodes targeted by selector
targetedNodes, err := mnr.reconHelper.getNodesListBySelector(ctx, mod)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get list of nodes by selector: %v", err)
}

currentNMCs, err := mnr.reconHelper.getNMCsByModuleSet(ctx, mod)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get nodes list: %v", err)
return ctrl.Result{}, fmt.Errorf("failed to get NMCs for %s Module: %v", req.NamespacedName, err)
}

sdMap, prepareErrs := mnr.reconHelper.prepareSchedulingData(ctx, mod, targetedNodes, currentNMCs)
var sumErr *multierror.Error
for _, node := range nodes {
kernelVersion := strings.TrimSuffix(node.Status.NodeInfo.KernelVersion, "+")
mld, err := mnr.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion)
if err != nil && !errors.Is(err, module.ErrNoMatchingKernelMapping) {
logger.Info(utils.WarnString(fmt.Sprintf("internal errors while fetching kernel mapping for version %s: %v", kernelVersion, err)))
sumErr = multierror.Append(sumErr, err)
continue
}
shouldBeOnNode, err := mnr.reconHelper.shouldModuleRunOnNode(node, mld)
if err != nil {
logger.Info(utils.WarnString(fmt.Sprintf("failed to determine if module %s/%s should be on node %s: %v", mld.Namespace, mld.Name, node.Name, err)))
sumErr = multierror.Append(sumErr, err)
continue
}
if shouldBeOnNode {
err = mnr.reconHelper.enableModuleOnNode(ctx, mld, &node, kernelVersion)
} else {
err = mnr.reconHelper.disableModuleOnNode(ctx, mod.Namespace, mod.Name, node.Name)
sumErr = multierror.Append(sumErr, prepareErrs...)

for nodeName, sd := range sdMap {
if sd.mld != nil {
err = mnr.reconHelper.enableModuleOnNode(ctx, sd.mld, sd.node)
} else if sd.nmcExists {
err = mnr.reconHelper.disableModuleOnNode(ctx, mod.Namespace, mod.Name, nodeName)
}
sumErr = multierror.Append(sumErr, err)
}
Expand All @@ -125,29 +125,33 @@ func (mnr *ModuleNMCReconciler) Reconcile(ctx context.Context, req ctrl.Request)

type moduleNMCReconcilerHelperAPI interface {
setFinalizer(ctx context.Context, mod *kmmv1beta1.Module) error
getRequestedModule(ctx context.Context, namespacedName types.NamespacedName) (*kmmv1beta1.Module, error)
getNodesList(ctx context.Context) ([]v1.Node, error)
finalizeModule(ctx context.Context, mod *kmmv1beta1.Module) error
shouldModuleRunOnNode(node v1.Node, mld *api.ModuleLoaderData) (bool, error)
enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node, kernelVersion string) error
getRequestedModule(ctx context.Context, namespacedName types.NamespacedName) (*kmmv1beta1.Module, error)
getNodesListBySelector(ctx context.Context, mod *kmmv1beta1.Module) ([]v1.Node, error)
getNMCsByModuleSet(ctx context.Context, mod *kmmv1beta1.Module) (sets.Set[string], error)
prepareSchedulingData(ctx context.Context, mod *kmmv1beta1.Module, targetedNodes []v1.Node, currentNMCs sets.Set[string]) (map[string]schedulingData, []error)
enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node) error
disableModuleOnNode(ctx context.Context, modNamespace, modName, nodeName string) error
}

type moduleNMCReconcilerHelper struct {
client client.Client
kernelAPI module.KernelMapper
registryAPI registry.Registry
nmcHelper nmc.Helper
authFactory auth.RegistryAuthGetterFactory
scheme *runtime.Scheme
}

func newModuleNMCReconcilerHelper(client client.Client,
kernelAPI module.KernelMapper,
registryAPI registry.Registry,
nmcHelper nmc.Helper,
authFactory auth.RegistryAuthGetterFactory,
scheme *runtime.Scheme) moduleNMCReconcilerHelperAPI {
return &moduleNMCReconcilerHelper{
client: client,
kernelAPI: kernelAPI,
registryAPI: registryAPI,
nmcHelper: nmcHelper,
authFactory: authFactory,
Expand Down Expand Up @@ -205,33 +209,82 @@ func (mnrh *moduleNMCReconcilerHelper) finalizeModule(ctx context.Context, mod *
return mnrh.client.Patch(ctx, mod, client.MergeFrom(modCopy))
}

func (mnrh *moduleNMCReconcilerHelper) getNodesList(ctx context.Context) ([]v1.Node, error) {
nodes := v1.NodeList{}
err := mnrh.client.List(ctx, &nodes)
if err != nil {
return nil, fmt.Errorf("failed to get list of nodes: %v", err)
func (mnrh *moduleNMCReconcilerHelper) getNodesListBySelector(ctx context.Context, mod *kmmv1beta1.Module) ([]v1.Node, error) {
logger := log.FromContext(ctx)
logger.V(1).Info("Listing nodes", "selector", mod.Spec.Selector)

selectedNodes := v1.NodeList{}
opt := client.MatchingLabels(mod.Spec.Selector)
if err := mnrh.client.List(ctx, &selectedNodes, opt); err != nil {
return nil, fmt.Errorf("could not list nodes: %v", err)
}
return nodes.Items, nil
}
nodes := make([]v1.Node, 0, len(selectedNodes.Items))

func (mnrh *moduleNMCReconcilerHelper) shouldModuleRunOnNode(node v1.Node, mld *api.ModuleLoaderData) (bool, error) {
if mld == nil {
return false, nil
for _, node := range selectedNodes.Items {
if utils.IsNodeSchedulable(&node) {
nodes = append(nodes, node)
}
}
return nodes, nil
}

nodeKernelVersion := strings.TrimSuffix(node.Status.NodeInfo.KernelVersion, "+")
if nodeKernelVersion != mld.KernelVersion {
return false, nil
func (mnrh *moduleNMCReconcilerHelper) getNMCsByModuleSet(ctx context.Context, mod *kmmv1beta1.Module) (sets.Set[string], error) {
nmcNamesList, err := mnrh.getNMCsNamesForModule(ctx, mod)
if err != nil {
return nil, fmt.Errorf("failed to get list of %s/%s module's NMC for map: %v", mod.Namespace, mod.Name, err)
}

if !utils.IsNodeSchedulable(&node) {
return false, nil
return sets.New[string](nmcNamesList...), nil
}

func (mnrh *moduleNMCReconcilerHelper) getNMCsNamesForModule(ctx context.Context, mod *kmmv1beta1.Module) ([]string, error) {
logger := log.FromContext(ctx)
moduleNMCLabel := utils.GetModuleNMCLabel(mod.Namespace, mod.Name)
logger.V(1).Info("Listing nmcs", "selector", moduleNMCLabel)
selectedNMCs := kmmv1beta1.NodeModulesConfigList{}
opt := client.MatchingLabels(map[string]string{moduleNMCLabel: ""})
if err := mnrh.client.List(ctx, &selectedNMCs, opt); err != nil {
return nil, fmt.Errorf("could not list NMCs: %v", err)
}
result := make([]string, len(selectedNMCs.Items))
for i := range selectedNMCs.Items {
result[i] = selectedNMCs.Items[i].Name
}
return result, nil
}

// prepareSchedulingData prepare data needed to scheduling enable/disable module per node
// in case there is an error during handling one of the nodes, function continues to the next node
// It returns the map of scheduling data per successfully processed node, and slice of errors
// per unsuccessfuly processed nodes
func (mnrh *moduleNMCReconcilerHelper) prepareSchedulingData(ctx context.Context,
mod *kmmv1beta1.Module,
targetedNodes []v1.Node,
currentNMCs sets.Set[string]) (map[string]schedulingData, []error) {

return utils.IsObjectSelectedByLabels(node.GetLabels(), mld.Selector)
logger := log.FromContext(ctx)
result := make(map[string]schedulingData)
errs := make([]error, 0, len(targetedNodes))
for _, node := range targetedNodes {
kernelVersion := strings.TrimSuffix(node.Status.NodeInfo.KernelVersion, "+")
mld, err := mnrh.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion)
if err != nil && !errors.Is(err, module.ErrNoMatchingKernelMapping) {
// deleting earlier, so as not to change NMC in case we failed to determine mld
currentNMCs.Delete(node.Name)
logger.Info(utils.WarnString(fmt.Sprintf("internal errors while fetching kernel mapping for version %s: %v", kernelVersion, err)))
errs = append(errs, err)
continue
}
result[node.Name] = schedulingData{mld: mld, node: &node, nmcExists: currentNMCs.Has(node.Name)}
currentNMCs.Delete(node.Name)
}
for _, nmcName := range currentNMCs.UnsortedList() {
result[nmcName] = schedulingData{mld: nil, nmcExists: true}
}
return result, errs
}

func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node, kernelVersion string) error {
func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node) error {
logger := log.FromContext(ctx)
exists, err := module.ImageExists(ctx, mnrh.authFactory, mnrh.registryAPI, mld, mld.ContainerImage)
if err != nil {
Expand All @@ -242,7 +295,7 @@ func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, m
return nil
}
moduleConfig := kmmv1beta1.ModuleConfig{
KernelVersion: kernelVersion,
KernelVersion: mld.KernelVersion,
ContainerImage: mld.ContainerImage,
InTreeModuleToRemove: mld.InTreeModuleToRemove,
Modprobe: mld.Modprobe,
Expand Down Expand Up @@ -272,13 +325,8 @@ func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, m
}

func (mnrh *moduleNMCReconcilerHelper) disableModuleOnNode(ctx context.Context, modNamespace, modName, nodeName string) error {
nmc, err := mnrh.nmcHelper.Get(ctx, nodeName)
if err != nil {
if k8serrors.IsNotFound(err) {
// NodeModulesConfig does not exists, module was never running on the node, we are good
return nil
}
return fmt.Errorf("failed to get the NodeModulesConfig for node %s: %v", nodeName, err)
nmc := &kmmv1beta1.NodeModulesConfig{
ObjectMeta: metav1.ObjectMeta{Name: nodeName},
}

return mnrh.removeModuleFromNMC(ctx, nmc, modNamespace, modName)
Expand Down
Loading

0 comments on commit 262ed40

Please sign in to comment.