diff --git a/api/v1beta1/nodenetworkconfigurationenactment_types.go b/api/v1beta1/nodenetworkconfigurationenactment_types.go index 4632ad8c9..c7b77c309 100644 --- a/api/v1beta1/nodenetworkconfigurationenactment_types.go +++ b/api/v1beta1/nodenetworkconfigurationenactment_types.go @@ -33,12 +33,12 @@ type NodeNetworkConfigurationEnactment struct { Status shared.NodeNetworkConfigurationEnactmentStatus `json:"status,omitempty"` } -func NewEnactment(nodeName string, policy NodeNetworkConfigurationPolicy) NodeNetworkConfigurationEnactment { +func NewEnactment(node *corev1.Node, policy NodeNetworkConfigurationPolicy) NodeNetworkConfigurationEnactment { enactment := NodeNetworkConfigurationEnactment{ ObjectMeta: metav1.ObjectMeta{ - Name: shared.EnactmentKey(nodeName, policy.Name).Name, + Name: shared.EnactmentKey(node.Name, policy.Name).Name, OwnerReferences: []metav1.OwnerReference{ - {Name: policy.Name, Kind: policy.TypeMeta.Kind, APIVersion: policy.TypeMeta.APIVersion, UID: policy.UID}, + {Name: node.Name, Kind: "Node", APIVersion: "v1", UID: node.UID}, }, // Associate policy with the enactment using labels Labels: names.IncludeRelationshipLabels(map[string]string{shared.EnactmentPolicyLabel: policy.Name}), diff --git a/api/v1beta1/nodenetworkconfigurationenactment_types_test.go b/api/v1beta1/nodenetworkconfigurationenactment_types_test.go new file mode 100644 index 000000000..a57715d21 --- /dev/null +++ b/api/v1beta1/nodenetworkconfigurationenactment_types_test.go @@ -0,0 +1,41 @@ +package v1beta1 + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("NodeNetworkEnactment", func() { + var ( + nncp = NodeNetworkConfigurationPolicy{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "nmstate.io/v1beta1", + Kind: "NodeNetworkConfigurationPolicy", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "policy1", + UID: "12345", + }, + } + node = corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + UID: "54321", + }, + } + ) + + Context("NewEnactment", func() { + It("should have the node as the owner reference of the created enactment", func() { + nnce := NewEnactment(&node, nncp) + desiredOnwerRefs := []metav1.OwnerReference{ + {Name: node.Name, Kind: "Node", APIVersion: "v1", UID: node.UID}, + } + Expect(nnce.OwnerReferences).To(Equal(desiredOnwerRefs)) + }) + }) + +}) diff --git a/controllers/nodenetworkconfigurationpolicy_controller.go b/controllers/nodenetworkconfigurationpolicy_controller.go index 34e3e6aad..b34ff8ac2 100644 --- a/controllers/nodenetworkconfigurationpolicy_controller.go +++ b/controllers/nodenetworkconfigurationpolicy_controller.go @@ -57,14 +57,14 @@ import ( ) var ( - nodeName string - nodeRunningUpdateRetryTime = 5 * time.Second - onCreateOrUpdateWithDifferentGeneration = predicate.Funcs{ + nodeName string + nodeRunningUpdateRetryTime = 5 * time.Second + onCreateOrUpdateWithDifferentGenerationOrDelete = predicate.Funcs{ CreateFunc: func(createEvent event.CreateEvent) bool { return true }, DeleteFunc: func(deleteEvent event.DeleteEvent) bool { - return false + return true }, UpdateFunc: func(updateEvent event.UpdateEvent) bool { // [1] https://blog.openshift.com/kubernetes-operators-best-practices/ @@ -125,10 +125,9 @@ func (r *NodeNetworkConfigurationPolicyReconciler) Reconcile(ctx context.Context err := r.Client.Get(context.TODO(), request.NamespacedName, instance) if err != nil { if apierrors.IsNotFound(err) { - // Request object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. - // Return and don't requeue - return ctrl.Result{}, nil + log.Info("Policy is not found, removing previous enactment if any") + err = r.deleteEnactmentForPolicy(request.NamespacedName.Name) + return ctrl.Result{}, err } log.Error(err, "Error retrieving policy") // Error reading the object - requeue the request. @@ -150,8 +149,8 @@ func (r *NodeNetworkConfigurationPolicyReconciler) Reconcile(ctx context.Context } if len(unmatchingNodeLabels) > 0 { - log.Info("Policy node selectors does not match node, removing previous enactments if any") - err = r.deleteEnactmentForPolicy(instance) + log.Info("Policy node selectors does not match node, removing previous enactment if any") + err = r.deleteEnactmentForPolicy(request.NamespacedName.Name) return ctrl.Result{}, err } @@ -227,7 +226,7 @@ func (r *NodeNetworkConfigurationPolicyReconciler) SetupWithManager(mgr ctrl.Man // Reconcile NNCP if they are created or updated err := ctrl.NewControllerManagedBy(mgr). For(&nmstatev1beta1.NodeNetworkConfigurationPolicy{}). - WithEventFilter(onCreateOrUpdateWithDifferentGeneration). + WithEventFilter(onCreateOrUpdateWithDifferentGenerationOrDelete). Complete(r) if err != nil { return errors.Wrap(err, "failed to add controller to NNCP Reconciler listening NNCP events") @@ -256,7 +255,13 @@ func (r *NodeNetworkConfigurationPolicyReconciler) initializeEnactment(policy nm } if err != nil && apierrors.IsNotFound(err) { log.Info("creating enactment") - enactment = nmstatev1beta1.NewEnactment(nodeName, policy) + // Fetch the Node instance + nodeInstance := &corev1.Node{} + err = r.APIClient.Get(context.TODO(), types.NamespacedName{Name: nodeName}, nodeInstance) + if err != nil { + return nil, errors.Wrap(err, "failed getting node") + } + enactment = nmstatev1beta1.NewEnactment(nodeInstance, policy) err = r.APIClient.Create(context.TODO(), &enactment) if err != nil { return nil, errors.Wrapf(err, "error creating NodeNetworkConfigurationEnactment: %+v", enactment) @@ -294,8 +299,8 @@ func (r *NodeNetworkConfigurationPolicyReconciler) waitEnactmentCreated(enactmen return pollErr } -func (r *NodeNetworkConfigurationPolicyReconciler) deleteEnactmentForPolicy(policy *nmstatev1beta1.NodeNetworkConfigurationPolicy) error { - enactmentKey := nmstateapi.EnactmentKey(nodeName, policy.Name) +func (r *NodeNetworkConfigurationPolicyReconciler) deleteEnactmentForPolicy(policyName string) error { + enactmentKey := nmstateapi.EnactmentKey(nodeName, policyName) enactment := nmstatev1beta1.NodeNetworkConfigurationEnactment{ ObjectMeta: metav1.ObjectMeta{ Name: enactmentKey.Name, diff --git a/controllers/nodenetworkconfigurationpolicy_controller_test.go b/controllers/nodenetworkconfigurationpolicy_controller_test.go index a88f32570..4a5978d0b 100644 --- a/controllers/nodenetworkconfigurationpolicy_controller_test.go +++ b/controllers/nodenetworkconfigurationpolicy_controller_test.go @@ -24,7 +24,6 @@ var _ = Describe("NodeNetworkConfigurationPolicy controller predicates", func() type predicateCase struct { GenerationOld int64 GenerationNew int64 - ReconcileCreate bool ReconcileUpdate bool } DescribeTable("testing predicates", @@ -40,12 +39,12 @@ var _ = Describe("NodeNetworkConfigurationPolicy controller predicates", func() }, } - predicate := onCreateOrUpdateWithDifferentGeneration + predicate := onCreateOrUpdateWithDifferentGenerationOrDelete Expect(predicate. CreateFunc(event.CreateEvent{ Object: &newNNCP, - })).To(Equal(c.ReconcileCreate)) + })).To(BeTrue()) Expect(predicate. UpdateFunc(event.UpdateEvent{ ObjectOld: &oldNNCP, @@ -53,21 +52,19 @@ var _ = Describe("NodeNetworkConfigurationPolicy controller predicates", func() })).To(Equal(c.ReconcileUpdate)) Expect(predicate. DeleteFunc(event.DeleteEvent{ - Object: &newNNCP, - })).To(BeFalse()) + Object: &oldNNCP, + })).To(BeTrue()) }, Entry("generation remains the same", predicateCase{ GenerationOld: 1, GenerationNew: 1, - ReconcileCreate: true, ReconcileUpdate: false, }), Entry("generation is different", predicateCase{ GenerationOld: 1, GenerationNew: 2, - ReconcileCreate: true, ReconcileUpdate: true, }), )