Skip to content

Commit

Permalink
Use finalizer to clean net-att-def CR
Browse files Browse the repository at this point in the history
  • Loading branch information
@pliurh
pliurh committed Dec 30, 2019
1 parent a2b6100 commit cdd0ba2
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 45 deletions.
2 changes: 1 addition & 1 deletion hack/run-locally.sh
View file
@@ -1,4 +1,4 @@
#!/bin/bash
EXCLUSIONS=(operator.yaml) hack/deploy-setup.sh ${NAMESPACE}
source hack/env.sh
operator-sdk up local --namespace ${NAMESPACE}
operator-sdk up local --namespace ${NAMESPACE} --enable-delve
10 changes: 10 additions & 0 deletions pkg/apis/sriovnetwork/v1/helper.go
View file
Expand Up @@ -64,6 +64,16 @@ func StringInArray(val string, array []string) bool {
return false
}

func RemoveString(s string, slice []string) (result []string) {
for _, item := range slice {
if item == s {
continue
}
result = append(result, item)
}
return
}

func UniqueAppend(inSlice []string, strings ...string) []string {
for _, s := range strings {
if !StringInArray(s, inSlice) {
Expand Down
103 changes: 70 additions & 33 deletions pkg/controller/sriovnetwork/sriovnetwork_controller.go
View file
Expand Up @@ -9,13 +9,13 @@ import (

"github.com/operator-framework/operator-sdk/pkg/k8sutil"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1"
uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
kscheme "k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"
Expand All @@ -28,7 +28,9 @@ import (
)

const (
MANIFESTS_PATH = "./bindata/manifests/cni-config"
MANIFESTS_PATH = "./bindata/manifests/cni-config"
LASTNETWORKNAMESPACE = "operator.sriovnetwork.openshift.io/last-network-namespace"
FINALIZERNAME = "netattdef.finalizers.sriovnetwork.openshift.io"
)

var log = logf.Log.WithName("controller_sriovnetwork")
Expand Down Expand Up @@ -63,11 +65,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
return err
}
// Watch for changes to secondary resource NetworkAttachmentDefinition
err = c.Watch(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForOwner{
IsController: true,
OwnerType: &sriovnetworkv1.SriovNetwork{},
})

err = c.Watch(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForObject{})
if err != nil {
return err
}
Expand Down Expand Up @@ -116,6 +114,37 @@ func (r *ReconcileSriovNetwork) Reconcile(request reconcile.Request) (reconcile.
// Error reading the object - requeue the request.
return reconcile.Result{}, err
}
// name of our custom finalizer

// examine DeletionTimestamp to determine if object is under deletion
if instance.ObjectMeta.DeletionTimestamp.IsZero() {
// The object is not being deleted, so if it does not have our finalizer,
// then lets add the finalizer and update the object. This is equivalent
// registering our finalizer.
if !sriovnetworkv1.StringInArray(FINALIZERNAME, instance.ObjectMeta.Finalizers) {
instance.ObjectMeta.Finalizers = append(instance.ObjectMeta.Finalizers, FINALIZERNAME)
if err := r.client.Update(context.Background(), instance); err != nil {
return reconcile.Result{}, err
}
}
} else {
// The object is being deleted
if sriovnetworkv1.StringInArray(FINALIZERNAME, instance.ObjectMeta.Finalizers) {
// our finalizer is present, so lets handle any external dependency
reqLogger.Info("delete NetworkAttachmentDefinition CR", "Namespace", instance.Spec.NetworkNamespace, "Name", instance.Name)
if err := r.deleteNetAttDef(instance); err != nil {
// if fail to delete the external dependency here, return with error
// so that it can be retried
return reconcile.Result{}, err
}
// remove our finalizer from the list and update it.
instance.ObjectMeta.Finalizers = sriovnetworkv1.RemoveString(FINALIZERNAME, instance.ObjectMeta.Finalizers)
if err := r.client.Update(context.Background(), instance); err != nil {
return reconcile.Result{}, err
}
}
return reconcile.Result{}, err
}
raw, err := renderNetAttDef(instance)
if err != nil {
return reconcile.Result{}, err
Expand All @@ -126,11 +155,18 @@ func (r *ReconcileSriovNetwork) Reconcile(request reconcile.Request) (reconcile.
if err != nil {
return reconcile.Result{}, err
}
// Set SriovNetwork instance as the owner and controller
if err := controllerutil.SetControllerReference(instance, netAttDef, r.scheme); err != nil {
return reconcile.Result{}, err
if lnns, ok := instance.GetAnnotations()[LASTNETWORKNAMESPACE]; ok && netAttDef.GetNamespace() != lnns {
err = r.client.Delete(context.TODO(), &netattdefv1.NetworkAttachmentDefinition{
ObjectMeta: v1.ObjectMeta{
Name: instance.GetName(),
Namespace: lnns,
},
})
if err != nil {
reqLogger.Error(err, "Couldn't delete NetworkAttachmentDefinition CR", "Namespace", instance.GetName(), "Name", lnns)
return reconcile.Result{}, err
}
}

// Check if this NetworkAttachmentDefinition already exists
found := &netattdefv1.NetworkAttachmentDefinition{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: netAttDef.Name, Namespace: netAttDef.Namespace}, found)
Expand All @@ -142,6 +178,11 @@ func (r *ReconcileSriovNetwork) Reconcile(request reconcile.Request) (reconcile.
reqLogger.Error(err, "Couldn't create NetworkAttachmentDefinition CR", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name)
return reconcile.Result{}, err
}
anno := map[string]string{LASTNETWORKNAMESPACE: netAttDef.Namespace}
instance.SetAnnotations(anno)
if err := r.client.Update(context.Background(), instance); err != nil {
return reconcile.Result{}, err
}
} else {
reqLogger.Error(err, "Couldn't get NetworkAttachmentDefinition CR", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name)
return reconcile.Result{}, err
Expand All @@ -158,32 +199,28 @@ func (r *ReconcileSriovNetwork) Reconcile(request reconcile.Request) (reconcile.
}
}
}
return reconcile.Result{}, nil
}

// Check if there are more than one children for one SriovNetwork CR.
nadList := &netattdefv1.NetworkAttachmentDefinitionList{}
r.client.List(context.TODO(), nadList, &client.ListOptions{})
sriovNads := []netattdefv1.NetworkAttachmentDefinition{}
for _, cr := range nadList.Items {
refs := cr.GetOwnerReferences()
if refs != nil && refs[0].UID == instance.UID {
sriovNads = append(sriovNads, cr)
}
func (r *ReconcileSriovNetwork) deleteNetAttDef(sn *sriovnetworkv1.SriovNetwork) error {
// Fetch the NetworkAttachmentDefinition instance
instance := &netattdefv1.NetworkAttachmentDefinition{}
namespace := sn.GetNamespace()
if sn.Spec.NetworkNamespace != "" {
namespace = sn.Spec.NetworkNamespace
}
reqLogger.Info("NetworkAttachmentDefinition", "list", sriovNads)
if len(sriovNads) > 1 {
reqLogger.Info("more than one NetworkAttachmentDefinition CR exists for one SriovNetwork CR", "Namespace", instance.GetNamespace(), "Name", instance.GetName())
for _, nad := range sriovNads {
if nad.GetNamespace() != netAttDef.GetNamespace() {
reqLogger.Info("delete the NetworkAttachmentDefinition", "Namespace", nad.GetNamespace(), "Name", nad.GetName())
err = r.client.Delete(context.TODO(), &nad)
if err != nil {
reqLogger.Error(err, "Couldn't delete NetworkAttachmentDefinition", "Namespace", nad.GetNamespace(), "Name", nad.GetName())
return reconcile.Result{}, err
}
}
err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: sn.GetName()}, instance)
if err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
}
return reconcile.Result{}, nil
err = r.client.Delete(context.TODO(), instance)
if err != nil {
return err
}
return nil
}

// renderNetAttDef returns a busybox pod with the same name/namespace as the cr
Expand Down
27 changes: 16 additions & 11 deletions test/e2e/e2e_test.go
View file
Expand Up @@ -188,13 +188,18 @@ func testWithOneSriovNetworkNodePolicyCR(t *testing.T, ctx *framework.TestCtx) {
}

cm := &corev1.ConfigMap{}
err = waitForNamespacedObject(cm, t, f.Client, namespace, "device-plugin-config", retryInterval, timeout)
if err != nil {
t.Fatalf("fail to get ConfigMap: %v", err)
}
for i := 0; i < 3; i++ {
err = waitForNamespacedObject(cm, t, f.Client, namespace, "device-plugin-config", retryInterval, timeout)
if err != nil {
t.Fatalf("fail to get ConfigMap: %v", err)
}

if err = validateDevicePluginConfig(policy, cm.Data["config.json"]); err != nil {
t.Fatalf("failed to validate ConfigMap : %v", err)
if err = validateDevicePluginConfig(policy, cm.Data["config.json"]); err != nil && i == 2 {
t.Fatalf("failed to validate ConfigMap : %v", err)
} else if err == nil {
break
}
time.Sleep(300 * time.Millisecond)
}

daemon := &appsv1.DaemonSet{}
Expand Down Expand Up @@ -239,8 +244,8 @@ func testWithSriovNetworkCRDeletion(t *testing.T, ctx *framework.TestCtx, cr *sr
if err != nil {
t.Fatalf("fail to Delete SriovNetwork CR: %v", err)
}
// wait 100ms for object get deleted
time.Sleep(300 * time.Millisecond)
// wait 600ms for object get deleted
time.Sleep(600 * time.Millisecond)
nad := &netattdefv1.NetworkAttachmentDefinition{}
namespace := cr.GetNamespace()
if cr.Spec.NetworkNamespace != "" {
Expand All @@ -250,7 +255,7 @@ func testWithSriovNetworkCRDeletion(t *testing.T, ctx *framework.TestCtx, cr *sr
if err != nil && errors.IsNotFound(err) {
return
}
t.Fatalf("fail to Delete NetworkAttachmentDefinition CR: %v", err)
t.Fatalf("fail to Delete NetworkAttachmentDefinition CR: %s/%s: %v", namespace, cr.Name, err)
}

func testWithSriovNetworkCRUpdate(t *testing.T, ctx *framework.TestCtx, cr *sriovnetworkv1.SriovNetwork) {
Expand Down Expand Up @@ -285,8 +290,8 @@ func testWithSriovNetworkCRUpdate(t *testing.T, ctx *framework.TestCtx, cr *srio
if err != nil {
t.Fatalf("fail to update SriovNetwork CR: %v", err)
}
// wait 100ms for object get update
time.Sleep(300 * time.Millisecond)
// wait 600ms for object get update
time.Sleep(600 * time.Millisecond)
netAttDefCR, err := WaitForNetworkAttachmentDefinition(t, f.Client, cr.GetName(), cr.GetNamespace(), retryInterval, timeout)
if err != nil {
t.Fatalf("fail to get NetworkAttachmentDefinition after update: %v", err)
Expand Down

0 comments on commit cdd0ba2

Please sign in to comment.