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

[Remove Default Objs 2/4] Set OwnerReference to default SriovOperatorConfig #621

Merged
merged 2 commits into from
Feb 19, 2024
Merged

[Remove Default Objs 2/4] Set OwnerReference to default SriovOperatorConfig #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94c57aa
Change sriov device plugin owner ref
adrianchiris Feb 5, 2024
5b1bdda
Change OwnerReference of SriovNetworkNodeState
adrianchiris Feb 6, 2024
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
21 changes: 13 additions & 8 deletions controllers/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ func syncPluginDaemonObjs(ctx context.Context,
client k8sclient.Client,
scheme *runtime.Scheme,
dc *sriovnetworkv1.SriovOperatorConfig,
dp *sriovnetworkv1.SriovNetworkNodePolicy,
pl *sriovnetworkv1.SriovNetworkNodePolicyList) error {
logger := log.Log.WithName("syncPluginDaemonObjs")
logger.V(1).Info("Start to sync sriov daemons objects")
Expand Down Expand Up @@ -129,7 +128,7 @@ func syncPluginDaemonObjs(ctx context.Context,
return err
}
}
err = syncDsObject(ctx, client, scheme, dp, pl, obj)
err = syncDsObject(ctx, client, scheme, dc, pl, obj)
if err != nil {
logger.Error(err, "Couldn't sync SR-IoV daemons objects")
return err
Expand All @@ -146,13 +145,13 @@ func deleteK8sResource(ctx context.Context, client k8sclient.Client, in *uns.Uns
return nil
}

func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dp *sriovnetworkv1.SriovNetworkNodePolicy, pl *sriovnetworkv1.SriovNetworkNodePolicyList, obj *uns.Unstructured) error {
func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dc *sriovnetworkv1.SriovOperatorConfig, pl *sriovnetworkv1.SriovNetworkNodePolicyList, obj *uns.Unstructured) error {
logger := log.Log.WithName("syncDsObject")
kind := obj.GetKind()
logger.V(1).Info("Start to sync Objects", "Kind", kind)
switch kind {
case constants.ServiceAccount, constants.Role, constants.RoleBinding:
if err := controllerutil.SetControllerReference(dp, obj, scheme); err != nil {
if err := controllerutil.SetControllerReference(dc, obj, scheme); err != nil {
return err
}
if err := apply.ApplyObject(ctx, client, obj); err != nil {
Expand All @@ -166,7 +165,7 @@ func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime.
logger.Error(err, "Fail to convert to DaemonSet")
return err
}
err = syncDaemonSet(ctx, client, scheme, dp, pl, ds)
err = syncDaemonSet(ctx, client, scheme, dc, pl, ds)
if err != nil {
logger.Error(err, "Fail to sync DaemonSet", "Namespace", ds.Namespace, "Name", ds.Name)
return err
Expand All @@ -175,7 +174,7 @@ func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime.
return nil
}

func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, cr *sriovnetworkv1.SriovNetworkNodePolicy, pl *sriovnetworkv1.SriovNetworkNodePolicyList, in *appsv1.DaemonSet) error {
func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dc *sriovnetworkv1.SriovOperatorConfig, pl *sriovnetworkv1.SriovNetworkNodePolicyList, in *appsv1.DaemonSet) error {
logger := log.Log.WithName("syncDaemonSet")
logger.V(1).Info("Start to sync DaemonSet", "Namespace", in.Namespace, "Name", in.Name)
var err error
Expand All @@ -185,7 +184,7 @@ func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime
return err
}
}
if err = controllerutil.SetControllerReference(cr, in, scheme); err != nil {
if err = controllerutil.SetControllerReference(dc, in, scheme); err != nil {
return err
}
ds := &appsv1.DaemonSet{}
Expand All @@ -207,7 +206,13 @@ func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime
// DeepDerivative checks for changes only comparing non-zero fields in the source struct.
// This skips default values added by the api server.
// References in https://github.com/kubernetes-sigs/kubebuilder/issues/592#issuecomment-625738183
if equality.Semantic.DeepDerivative(in.Spec, ds.Spec) {

// Note(Adrianc): we check Equality of OwnerReference as we changed sriov-device-plugin owner ref
// from SriovNetworkNodePolicy to SriovOperatorConfig, hence even if there is no change in spec,
// we need to update the obj's owner reference.

if equality.Semantic.DeepEqual(in.OwnerReferences, ds.OwnerReferences) &&
equality.Semantic.DeepDerivative(in.Spec, ds.Spec) {
// DeepDerivative has issue detecting nodeAffinity change
// https://bugzilla.redhat.com/show_bug.cgi?id=1914066
// DeepDerivative doesn't detect changes in containers args section
Expand Down
20 changes: 13 additions & 7 deletions controllers/sriovnetworknodepolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"fmt"
"reflect"
"sort"
"strings"
"time"
Expand Down Expand Up @@ -149,15 +150,15 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct
// Sort the policies with priority, higher priority ones is applied later
sort.Sort(sriovnetworkv1.ByPriority(policyList.Items))
// Sync SriovNetworkNodeState objects
if err = r.syncAllSriovNetworkNodeStates(ctx, defaultPolicy, policyList, nodeList); err != nil {
if err = r.syncAllSriovNetworkNodeStates(ctx, defaultOpConf, policyList, nodeList); err != nil {
return reconcile.Result{}, err
}
// Sync Sriov device plugin ConfigMap object
if err = r.syncDevicePluginConfigMap(ctx, defaultOpConf, policyList, nodeList); err != nil {
return reconcile.Result{}, err
}
// Render and sync Daemon objects
if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultOpConf, defaultPolicy, policyList); err != nil {
if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultOpConf, policyList); err != nil {
return reconcile.Result{}, err
}

Expand Down Expand Up @@ -255,7 +256,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncDevicePluginConfigMap(ctx context
return nil
}

func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx context.Context, np *sriovnetworkv1.SriovNetworkNodePolicy, npl *sriovnetworkv1.SriovNetworkNodePolicyList, nl *corev1.NodeList) error {
func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx context.Context, dc *sriovnetworkv1.SriovOperatorConfig, npl *sriovnetworkv1.SriovNetworkNodePolicyList, nl *corev1.NodeList) error {
logger := log.Log.WithName("syncAllSriovNetworkNodeStates")
logger.V(1).Info("Start to sync all SriovNetworkNodeState custom resource")
found := &corev1.ConfigMap{}
Expand All @@ -269,7 +270,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con
ns.Namespace = vars.Namespace
j, _ := json.Marshal(ns)
logger.V(2).Info("SriovNetworkNodeState CR", "content", j)
if err := r.syncSriovNetworkNodeState(ctx, np, npl, ns, &node, utils.HashConfigMap(found)); err != nil {
if err := r.syncSriovNetworkNodeState(ctx, dc, npl, ns, &node, utils.HashConfigMap(found)); err != nil {
logger.Error(err, "Fail to sync", "SriovNetworkNodeState", ns.Name)
return err
}
Expand Down Expand Up @@ -303,11 +304,11 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con
return nil
}

func (r *SriovNetworkNodePolicyReconciler) syncSriovNetworkNodeState(ctx context.Context, np *sriovnetworkv1.SriovNetworkNodePolicy, npl *sriovnetworkv1.SriovNetworkNodePolicyList, ns *sriovnetworkv1.SriovNetworkNodeState, node *corev1.Node, cksum string) error {
func (r *SriovNetworkNodePolicyReconciler) syncSriovNetworkNodeState(ctx context.Context, dc *sriovnetworkv1.SriovOperatorConfig, npl *sriovnetworkv1.SriovNetworkNodePolicyList, ns *sriovnetworkv1.SriovNetworkNodeState, node *corev1.Node, cksum string) error {
logger := log.Log.WithName("syncSriovNetworkNodeState")
logger.V(1).Info("Start to sync SriovNetworkNodeState", "Name", ns.Name, "cksum", cksum)

if err := controllerutil.SetControllerReference(np, ns, r.Scheme); err != nil {
if err := controllerutil.SetControllerReference(dc, ns, r.Scheme); err != nil {
return err
}
found := &sriovnetworkv1.SriovNetworkNodeState{}
Expand All @@ -334,6 +335,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncSriovNetworkNodeState(ctx context
logger.V(1).Info("SriovNetworkNodeState already exists, updating")
newVersion := found.DeepCopy()
newVersion.Spec = ns.Spec
newVersion.OwnerReferences = ns.OwnerReferences

// Previous Policy Priority(ppp) records the priority of previous evaluated policy in node policy list.
// Since node policy list is already sorted with priority number, comparing current priority with ppp shall
Expand All @@ -359,7 +361,11 @@ func (r *SriovNetworkNodePolicyReconciler) syncSriovNetworkNodeState(ctx context
}
}
newVersion.Spec.DpConfigVersion = cksum
if equality.Semantic.DeepEqual(newVersion.Spec, found.Spec) {
// Note(adrianc): we check same ownerReferences since SriovNetworkNodeState
// was owned by a default SriovNetworkNodePolicy. if we encounter a descripancy
// we need to update.
if reflect.DeepEqual(newVersion.OwnerReferences, found.OwnerReferences) &&
equality.Semantic.DeepEqual(newVersion.Spec, found.Spec) {
logger.V(1).Info("SriovNetworkNodeState did not change, not updating")
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/sriovoperatorconfig_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
return reconcile.Result{}, err
}

if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, defaultPolicy, policyList); err != nil {
if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, policyList); err != nil {
return reconcile.Result{}, err
}

Expand Down
20 changes: 20 additions & 0 deletions controllers/sriovoperatorconfig_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
. "github.com/onsi/gomega"

sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
mock_platforms "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/mock"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/openshift"
Expand Down Expand Up @@ -56,6 +57,21 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
Expect(err).ToNot(HaveOccurred())
})

otherPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{}
otherPolicy.SetNamespace(testNamespace)
otherPolicy.SetName("other-policy")
otherPolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{
NumVfs: 5,
NodeSelector: map[string]string{"foo": "bar"},
NicSelector: sriovnetworkv1.SriovNetworkNicSelector{},
Priority: 20,
}
Expect(k8sClient.Create(context.Background(), otherPolicy)).ToNot(HaveOccurred())
DeferCleanup(func() {
err := k8sClient.Delete(context.Background(), otherPolicy)
Expect(err).ToNot(HaveOccurred())
})

// setup controller manager
By("Setup controller manager")
k8sManager, err := setupK8sManagerForTest()
Expand Down Expand Up @@ -125,10 +141,14 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
daemonSet := &appsv1.DaemonSet{}
err := util.WaitForNamespacedObject(daemonSet, k8sClient, testNamespace, dsName, util.RetryInterval, util.APITimeout)
Expect(err).NotTo(HaveOccurred())
Expect(daemonSet.OwnerReferences).To(HaveLen(1))
Expect(daemonSet.OwnerReferences[0].Kind).To(Equal("SriovOperatorConfig"))
Expect(daemonSet.OwnerReferences[0].Name).To(Equal(consts.DefaultConfigName))
},
Entry("operator-webhook", "operator-webhook"),
Entry("network-resources-injector", "network-resources-injector"),
Entry("sriov-network-config-daemon", "sriov-network-config-daemon"),
Entry("sriov-device-plugin", "sriov-device-plugin"),
)

It("should be able to turn network-resources-injector on/off", func() {
Expand Down
Loading