Skip to content

Commit

Permalink
Merge pull request #1114 from openshift-kni/plat-cleanup-objstate
Browse files Browse the repository at this point in the history
objstate: rte: isolate platform-specific code
  • Loading branch information
openshift-merge-bot[bot] authored Jan 7, 2025
2 parents 35e0c36 + 835a9a8 commit e70e002
Show file tree
Hide file tree
Showing 6 changed files with 296 additions and 179 deletions.
27 changes: 13 additions & 14 deletions controllers/numaresourcesoperator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,10 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Conte
return intreconcile.StepSuccess()
}

func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) intreconcile.Step {
func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx context.Context, instance *nropv1.NUMAResourcesOperator, existing *rtestate.ExistingManifests, trees []nodegroupv1.Tree) intreconcile.Step {
// we need to sync machine configs first and wait for the MachineConfigPool updates
// before checking additional components for updates
mcpUpdatedFunc, err := r.syncMachineConfigs(ctx, instance, trees)
mcpUpdatedFunc, err := r.syncMachineConfigs(ctx, instance, existing, trees)
if err != nil {
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedMCSync", "Failed to set up machine configuration for worker nodes: %v", err)
err = fmt.Errorf("failed to sync machine configs: %w", err)
Expand All @@ -264,8 +264,8 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx con
return intreconcile.StepSuccess()
}

func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) ([]poolDaemonSet, intreconcile.Step) {
daemonSetsInfoPerPool, err := r.syncNUMAResourcesOperatorResources(ctx, instance, trees)
func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context.Context, instance *nropv1.NUMAResourcesOperator, existing *rtestate.ExistingManifests, trees []nodegroupv1.Tree) ([]poolDaemonSet, intreconcile.Step) {
daemonSetsInfoPerPool, err := r.syncNUMAResourcesOperatorResources(ctx, instance, existing, trees)
if err != nil {
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedRTECreate", "Failed to create Resource-Topology-Exporter DaemonSets: %v", err)
err = fmt.Errorf("FailedRTESync: %w", err)
Expand Down Expand Up @@ -297,14 +297,16 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResource(ctx context.Context,
return step
}

existing := rtestate.FromClient(ctx, r.Client, r.Platform, r.RTEManifests, instance, trees, r.Namespace)

if r.Platform == platform.OpenShift {
if step := r.reconcileResourceMachineConfig(ctx, instance, trees); step.EarlyStop() {
if step := r.reconcileResourceMachineConfig(ctx, instance, existing, trees); step.EarlyStop() {
updateStatusConditionsIfNeeded(instance, step.ConditionInfo)
return step
}
}

dsPerPool, step := r.reconcileResourceDaemonSet(ctx, instance, trees)
dsPerPool, step := r.reconcileResourceDaemonSet(ctx, instance, existing, trees)
if step.EarlyStop() {
updateStatusConditionsIfNeeded(instance, step.ConditionInfo)
return step
Expand Down Expand Up @@ -401,12 +403,10 @@ func (r *NUMAResourcesOperatorReconciler) syncNodeResourceTopologyAPI(ctx contex
return (updatedCount == len(objStates)), err
}

func (r *NUMAResourcesOperatorReconciler) syncMachineConfigs(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (rtestate.MCPWaitForUpdatedFunc, error) {
func (r *NUMAResourcesOperatorReconciler) syncMachineConfigs(ctx context.Context, instance *nropv1.NUMAResourcesOperator, existing *rtestate.ExistingManifests, trees []nodegroupv1.Tree) (rtestate.MCPWaitForUpdatedFunc, error) {
klog.V(4).InfoS("Machine Config Sync start", "trees", len(trees))
defer klog.V(4).Info("Machine Config Sync stop")

existing := rtestate.FromClient(ctx, r.Client, r.Platform, r.RTEManifests, instance, trees, r.Namespace)

var err error
// Since 4.18 we're using a built-in SELinux policy,
// so the MachineConfig which applies the custom policy is no longer necessary.
Expand Down Expand Up @@ -505,7 +505,7 @@ func getMachineConfigPoolStatusByName(mcpStatuses []nropv1.MachineConfigPool, na
return nropv1.MachineConfigPool{Name: name}
}

func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) ([]poolDaemonSet, error) {
func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx context.Context, instance *nropv1.NUMAResourcesOperator, existing *rtestate.ExistingManifests, trees []nodegroupv1.Tree) ([]poolDaemonSet, error) {
klog.V(4).InfoS("RTESync start", "trees", len(trees))
defer klog.V(4).Info("RTESync stop")

Expand Down Expand Up @@ -548,17 +548,16 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx
}
rteupdate.SecurityContextConstraint(r.RTEManifests.Core.SecurityContextConstraint, annotations.IsCustomPolicyEnabled(instance.Annotations))

processor := func(poolName string, gdm *rtestate.GeneratedDesiredManifest) error {
existing = existing.WithManifestsUpdater(func(poolName string, gdm *rtestate.GeneratedDesiredManifest) error {
err := daemonsetUpdater(poolName, gdm)
if err != nil {
return err
}
dsPoolPairs = append(dsPoolPairs, poolDaemonSet{poolName, nropv1.NamespacedNameFromObject(gdm.DaemonSet)})
return nil
}
})

existing := rtestate.FromClient(ctx, r.Client, r.Platform, r.RTEManifests, instance, trees, r.Namespace)
for _, objState := range existing.State(r.RTEManifests, processor, annotations.IsCustomPolicyEnabled(instance.Annotations)) {
for _, objState := range existing.State(r.RTEManifests) {
if objState.Error != nil {
// We are likely in the bootstrap scenario. In this case, which is expected once, everything is fine.
// If it happens past bootstrap, still carry on. We know what to do, and we do want to enforce the desired state.
Expand Down
6 changes: 3 additions & 3 deletions pkg/objectstate/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type ExistingManifests struct {
CrdError error
}

func (em ExistingManifests) State(mf apimanifests.Manifests) []objectstate.ObjectState {
func (em *ExistingManifests) State(mf apimanifests.Manifests) []objectstate.ObjectState {
return []objectstate.ObjectState{
{
Existing: em.Existing.Crd,
Expand All @@ -48,13 +48,13 @@ func (em ExistingManifests) State(mf apimanifests.Manifests) []objectstate.Objec
}
}

func FromClient(ctx context.Context, cli client.Client, plat platform.Platform, mf apimanifests.Manifests) ExistingManifests {
func FromClient(ctx context.Context, cli client.Client, plat platform.Platform, mf apimanifests.Manifests) *ExistingManifests {
ret := ExistingManifests{
Existing: apimanifests.New(plat),
}
crd := apiextensionv1.CustomResourceDefinition{}
if ret.CrdError = cli.Get(ctx, client.ObjectKeyFromObject(mf.Crd), &crd); ret.CrdError == nil {
ret.Existing.Crd = &crd
}
return ret
return &ret
}
6 changes: 3 additions & 3 deletions pkg/objectstate/cfg/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type ExistingManifests struct {
ConfigError error
}

func (em ExistingManifests) State(mf Manifests) []objectstate.ObjectState {
func (em *ExistingManifests) State(mf Manifests) []objectstate.ObjectState {
return []objectstate.ObjectState{
{
Existing: em.Existing.Config,
Expand All @@ -49,7 +49,7 @@ func (em ExistingManifests) State(mf Manifests) []objectstate.ObjectState {
}
}

func FromClient(ctx context.Context, cli client.Client, namespace, name string) ExistingManifests {
func FromClient(ctx context.Context, cli client.Client, namespace, name string) *ExistingManifests {
ret := ExistingManifests{}
key := client.ObjectKey{
Name: name,
Expand All @@ -59,5 +59,5 @@ func FromClient(ctx context.Context, cli client.Client, namespace, name string)
if ret.ConfigError = cli.Get(ctx, key, &config); ret.ConfigError == nil {
ret.Existing.Config = &config
}
return ret
return &ret
}
121 changes: 121 additions & 0 deletions pkg/objectstate/rte/machineconfigpool.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* Copyright 2025 Red Hat, Inc.
*/

package rte

import (
"context"
"fmt"

appsv1 "k8s.io/api/apps/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

machineconfigv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"

nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1"
nodegroupv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/nodegroup"
"github.com/openshift-kni/numaresources-operator/pkg/objectnames"
"github.com/openshift-kni/numaresources-operator/pkg/objectstate"
"github.com/openshift-kni/numaresources-operator/pkg/objectstate/compare"
"github.com/openshift-kni/numaresources-operator/pkg/objectstate/merge"
)

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(obj.instance.Name, mcp.Name)
key := client.ObjectKey{
Name: generatedName,
Namespace: obj.namespace,
}
ds := &appsv1.DaemonSet{}
dsm := daemonSetManifest{}
if dsm.daemonSetError = cli.Get(ctx, key, ds); dsm.daemonSetError == nil {
dsm.daemonSet = ds
}
obj.em.daemonSets[generatedName] = dsm

mcName := objectnames.GetMachineConfigName(obj.instance.Name, mcp.Name)
mckey := client.ObjectKey{
Name: mcName,
}
mc := &machineconfigv1.MachineConfig{}
mcm := machineConfigManifest{}
if mcm.machineConfigError = cli.Get(ctx, mckey, mc); mcm.machineConfigError == nil {
mcm.machineConfig = mc
}
obj.em.machineConfigs[mcName] = mcm
}
}

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(obj.instance.Name, mcp.Name)
existingDaemonSet, ok := obj.em.daemonSets[generatedName]
if ok {
existingDs = existingDaemonSet.daemonSet
loadError = existingDaemonSet.daemonSetError
} else {
loadError = fmt.Errorf("failed to find daemon set %s/%s", mf.Core.DaemonSet.Namespace, mf.Core.DaemonSet.Name)
}

desiredDaemonSet := mf.Core.DaemonSet.DeepCopy()
desiredDaemonSet.Name = generatedName

var updateError error
if mcp.Spec.NodeSelector != nil {
desiredDaemonSet.Spec.Template.Spec.NodeSelector = mcp.Spec.NodeSelector.MatchLabels
} else {
updateError = fmt.Errorf("the machine config pool %q does not have node selector", mcp.Name)
}

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

err := obj.em.updater(mcp.Name, &gdm)
if err != nil {
updateError = fmt.Errorf("daemonset for MCP %q: update failed: %w", mcp.Name, err)
}

ret = append(ret, objectstate.ObjectState{
Existing: existingDs,
Error: loadError,
UpdateError: updateError,
Desired: desiredDaemonSet,
Compare: compare.Object,
Merge: merge.ObjectForUpdate,
})
}
return ret
}
104 changes: 104 additions & 0 deletions pkg/objectstate/rte/nodegroup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* Copyright 2025 Red Hat, Inc.
*/

package rte

import (
"context"
"fmt"

appsv1 "k8s.io/api/apps/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1"
nodegroupv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/nodegroup"
"github.com/openshift-kni/numaresources-operator/pkg/objectnames"
"github.com/openshift-kni/numaresources-operator/pkg/objectstate"
"github.com/openshift-kni/numaresources-operator/pkg/objectstate/compare"
"github.com/openshift-kni/numaresources-operator/pkg/objectstate/merge"
)

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: obj.namespace,
}
ds := &appsv1.DaemonSet{}
dsm := daemonSetManifest{}
if dsm.daemonSetError = cli.Get(ctx, key, ds); dsm.daemonSetError == nil {
dsm.daemonSet = ds
}
obj.em.daemonSets[generatedName] = dsm
}

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(obj.instance.Name, poolName)
existingDaemonSet, ok := obj.em.daemonSets[generatedName]
if ok {
existingDs = existingDaemonSet.daemonSet
loadError = existingDaemonSet.daemonSetError
} else {
loadError = fmt.Errorf("failed to find daemon set %s/%s", mf.Core.DaemonSet.Namespace, mf.Core.DaemonSet.Name)
}

desiredDaemonSet := mf.Core.DaemonSet.DeepCopy()
desiredDaemonSet.Name = generatedName

var updateError error
desiredDaemonSet.Spec.Template.Spec.NodeSelector = map[string]string{
HyperShiftNodePoolLabel: poolName,
}

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

err := obj.em.updater(poolName, &gdm)
if err != nil {
updateError = fmt.Errorf("daemonset for pool %q: update failed: %w", poolName, err)
}

ret = append(ret, objectstate.ObjectState{
Existing: existingDs,
Error: loadError,
UpdateError: updateError,
Desired: desiredDaemonSet,
Compare: compare.Object,
Merge: merge.ObjectForUpdate,
})
return ret
}
Loading

0 comments on commit e70e002

Please sign in to comment.