Skip to content

Commit

Permalink
objstate: rte: factorize code
Browse files Browse the repository at this point in the history 
the helpers we extracted in previous commits still share some state;
pack them in a struct, which makes also nicer to consume them from
the caller site(s), acting almost like a type.

In the (far?) future, when we will move completely to nodegroups
(if ever), this code should be dissolved back in the callers.

Signed-off-by: Francesco Romani <[email protected]>
(cherry picked from commit 860ffd1)
  • Loading branch information
@ffromani
ffromani committed Jan 13, 2025
1 parent 21a4b09 commit a63b7c8
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 46 deletions.
34 changes: 22 additions & 12 deletions pkg/objectstate/rte/machineconfigpool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,31 @@ import (
"github.com/openshift-kni/numaresources-operator/pkg/objectstate/merge"
)

func updateFromClientTreeMachineConfigPool(ret *ExistingManifests, ctx context.Context, cli client.Client, instance *nropv1.NUMAResourcesOperator, tree nodegroupv1.Tree, namespace string) {
type machineConfigPoolFinder struct {
em *ExistingManifests
instance *nropv1.NUMAResourcesOperator
namespace string
}

func (obj machineConfigPoolFinder) Name() string {
return "machineConfigPool"
}

func (obj machineConfigPoolFinder) UpdateFromClient(ctx context.Context, cli client.Client, tree nodegroupv1.Tree) {
for _, mcp := range tree.MachineConfigPools {
generatedName := objectnames.GetComponentName(instance.Name, mcp.Name)
generatedName := objectnames.GetComponentName(obj.instance.Name, mcp.Name)
key := client.ObjectKey{
Name: generatedName,
Namespace: namespace,
Namespace: obj.namespace,
}
ds := &appsv1.DaemonSet{}
dsm := daemonSetManifest{}
if dsm.daemonSetError = cli.Get(ctx, key, ds); dsm.daemonSetError == nil {
dsm.daemonSet = ds
}
ret.daemonSets[generatedName] = dsm
obj.em.daemonSets[generatedName] = dsm

mcName := objectnames.GetMachineConfigName(instance.Name, mcp.Name)
mcName := objectnames.GetMachineConfigName(obj.instance.Name, mcp.Name)
mckey := client.ObjectKey{
Name: mcName,
}
Expand All @@ -56,18 +66,18 @@ func updateFromClientTreeMachineConfigPool(ret *ExistingManifests, ctx context.C
if mcm.machineConfigError = cli.Get(ctx, mckey, mc); mcm.machineConfigError == nil {
mcm.machineConfig = mc
}
ret.machineConfigs[mcName] = mcm
obj.em.machineConfigs[mcName] = mcm
}
}

func stateFromMachineConfigPools(em *ExistingManifests, mf Manifests, tree nodegroupv1.Tree) []objectstate.ObjectState {
func (obj machineConfigPoolFinder) FindState(mf Manifests, tree nodegroupv1.Tree) []objectstate.ObjectState {
var ret []objectstate.ObjectState
for _, mcp := range tree.MachineConfigPools {
var existingDs client.Object
var loadError error

generatedName := objectnames.GetComponentName(em.instance.Name, mcp.Name)
existingDaemonSet, ok := em.daemonSets[generatedName]
generatedName := objectnames.GetComponentName(obj.instance.Name, mcp.Name)
existingDaemonSet, ok := obj.em.daemonSets[generatedName]
if ok {
existingDs = existingDaemonSet.daemonSet
loadError = existingDaemonSet.daemonSetError
Expand All @@ -86,14 +96,14 @@ func stateFromMachineConfigPools(em *ExistingManifests, mf Manifests, tree nodeg
}

gdm := GeneratedDesiredManifest{
ClusterPlatform: em.plat,
ClusterPlatform: obj.em.plat,
MachineConfigPool: mcp.DeepCopy(),
NodeGroup: tree.NodeGroup.DeepCopy(),
DaemonSet: desiredDaemonSet,
IsCustomPolicyEnabled: em.customPolicyEnabled,
IsCustomPolicyEnabled: obj.em.customPolicyEnabled,
}

err := em.updater(mcp.Name, &gdm)
err := obj.em.updater(mcp.Name, &gdm)
if err != nil {
updateError = fmt.Errorf("daemonset for MCP %q: update failed: %w", mcp.Name, err)
}
Expand Down
30 changes: 20 additions & 10 deletions pkg/objectstate/rte/nodegroup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,39 @@ import (
"github.com/openshift-kni/numaresources-operator/pkg/objectstate/merge"
)

func updateFromClientTreeNodeGroup(ret *ExistingManifests, ctx context.Context, cli client.Client, instance *nropv1.NUMAResourcesOperator, tree nodegroupv1.Tree, namespace string) {
generatedName := objectnames.GetComponentName(instance.Name, *tree.NodeGroup.PoolName)
type nodeGroupFinder struct {
em *ExistingManifests
instance *nropv1.NUMAResourcesOperator
namespace string
}

func (obj nodeGroupFinder) Name() string {
return "nodeGroup"
}

func (obj nodeGroupFinder) UpdateFromClient(ctx context.Context, cli client.Client, tree nodegroupv1.Tree) {
generatedName := objectnames.GetComponentName(obj.instance.Name, *tree.NodeGroup.PoolName)
key := client.ObjectKey{
Name: generatedName,
Namespace: namespace,
Namespace: obj.namespace,
}
ds := &appsv1.DaemonSet{}
dsm := daemonSetManifest{}
if dsm.daemonSetError = cli.Get(ctx, key, ds); dsm.daemonSetError == nil {
dsm.daemonSet = ds
}
ret.daemonSets[generatedName] = dsm
obj.em.daemonSets[generatedName] = dsm
}

func stateFromNodeGroup(em *ExistingManifests, mf Manifests, tree nodegroupv1.Tree) []objectstate.ObjectState {
func (obj nodeGroupFinder) FindState(mf Manifests, tree nodegroupv1.Tree) []objectstate.ObjectState {
var ret []objectstate.ObjectState
var existingDs client.Object
var loadError error

poolName := *tree.NodeGroup.PoolName

generatedName := objectnames.GetComponentName(em.instance.Name, poolName)
existingDaemonSet, ok := em.daemonSets[generatedName]
generatedName := objectnames.GetComponentName(obj.instance.Name, poolName)
existingDaemonSet, ok := obj.em.daemonSets[generatedName]
if ok {
existingDs = existingDaemonSet.daemonSet
loadError = existingDaemonSet.daemonSetError
Expand All @@ -70,14 +80,14 @@ func stateFromNodeGroup(em *ExistingManifests, mf Manifests, tree nodegroupv1.Tr
}

gdm := GeneratedDesiredManifest{
ClusterPlatform: em.plat,
ClusterPlatform: obj.em.plat,
MachineConfigPool: nil,
NodeGroup: tree.NodeGroup.DeepCopy(),
DaemonSet: desiredDaemonSet,
IsCustomPolicyEnabled: em.customPolicyEnabled,
IsCustomPolicyEnabled: obj.em.customPolicyEnabled,
}

err := em.updater(poolName, &gdm)
err := obj.em.updater(poolName, &gdm)
if err != nil {
updateError = fmt.Errorf("daemonset for pool %q: update failed: %w", poolName, err)
}
Expand Down
50 changes: 26 additions & 24 deletions pkg/objectstate/rte/rte.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ const (
HyperShiftNodePoolLabel = "hypershift.openshift.io/nodePool"
)

// TODO: ugly name. At least it's only internal
type rteHelper interface {
Name() string
UpdateFromClient(ctx context.Context, cli client.Client, tree nodegroupv1.Tree)
FindState(mf Manifests, tree nodegroupv1.Tree) []objectstate.ObjectState
}

type daemonSetManifest struct {
daemonSet *appsv1.DaemonSet
daemonSetError error
Expand Down Expand Up @@ -87,6 +94,7 @@ type ExistingManifests struct {
namespace string
customPolicyEnabled bool
updater GenerateDesiredManifestUpdater
helper rteHelper
}

func DaemonSetNamespacedNameFromObject(obj client.Object) (nropv1.NamespacedName, bool) {
Expand Down Expand Up @@ -229,8 +237,6 @@ func SkipManifestUpdate(mcpName string, gdm *GeneratedDesiredManifest) error {
return nil
}

type stateHelperFunc func(em *ExistingManifests, mf Manifests, tree nodegroupv1.Tree) []objectstate.ObjectState

func (em *ExistingManifests) State(mf Manifests) []objectstate.ObjectState {
ret := []objectstate.ObjectState{
{
Expand Down Expand Up @@ -280,18 +286,10 @@ func (em *ExistingManifests) State(mf Manifests) []objectstate.ObjectState {
})
}

method := "nodeGroup"
var stateHelper stateHelperFunc = stateFromNodeGroup

if em.plat == platform.OpenShift {
method = "machineConfigPools"
stateHelper = stateFromMachineConfigPools // backward compatibility
}

klog.V(4).InfoS("RTE manifests processing trees", "method", method)
klog.V(4).InfoS("RTE manifests processing trees", "method", em.helper.Name())

for _, tree := range em.trees {
ret = append(ret, stateHelper(em, mf, tree)...)
ret = append(ret, em.helper.FindState(mf, tree)...)
}

// extra: metrics
Expand All @@ -307,8 +305,6 @@ func (em *ExistingManifests) State(mf Manifests) []objectstate.ObjectState {
return ret
}

type updateFromClientTreeFunc func(ret *ExistingManifests, ctx context.Context, cli client.Client, instance *nropv1.NUMAResourcesOperator, tree nodegroupv1.Tree, namespace string)

func (em *ExistingManifests) WithManifestsUpdater(updater GenerateDesiredManifestUpdater) *ExistingManifests {
em.updater = updater
return em
Expand All @@ -330,6 +326,20 @@ func FromClient(ctx context.Context, cli client.Client, plat platform.Platform,

keyFor := client.ObjectKeyFromObject // shortcut

if plat == platform.OpenShift {
ret.helper = machineConfigPoolFinder{
em: &ret,
instance: instance,
namespace: namespace,
}
} else {
ret.helper = nodeGroupFinder{
em: &ret,
instance: instance,
namespace: namespace,
}
}

// objects that should present in the single replica
ro := &rbacv1.Role{}
if ok := getObject(ctx, cli, keyFor(mf.Core.Role), ro, &ret.errs.Core.Role); ok {
Expand All @@ -356,15 +366,7 @@ func FromClient(ctx context.Context, cli client.Client, plat platform.Platform,
ret.existing.Core.ServiceAccount = sa
}

method := "nodeGroups"
var updateFromClientTree updateFromClientTreeFunc = updateFromClientTreeNodeGroup

if plat == platform.OpenShift {
method = "machineConfigPools"
updateFromClientTree = updateFromClientTreeMachineConfigPool // backward compatibility
}

klog.V(4).InfoS("RTE manifests processing trees", "method", method)
klog.V(4).InfoS("RTE manifests processing trees", "method", ret.helper.Name())

if plat != platform.Kubernetes {
scc := &securityv1.SecurityContextConstraints{}
Expand All @@ -376,7 +378,7 @@ func FromClient(ctx context.Context, cli client.Client, plat platform.Platform,

// should have the amount of resources equals to the amount of node groups
for _, tree := range trees {
updateFromClientTree(&ret, ctx, cli, instance, tree, namespace)
ret.helper.UpdateFromClient(ctx, cli, tree)
}

// extra: metrics
Expand Down

0 comments on commit a63b7c8

Please sign in to comment.