Skip to content

Commit

Permalink
SriovNetwork: react on namespace creation
Browse files Browse the repository at this point in the history
If a user creates an SriovNetwork with a network namespace
that doesn't exist, retrying reconciling with an exponential
backoff is not efficient, as the routing will fail until the namespace
is created.

This patch makes the controller watch Namespace resource
creation and reconcile the related SriovNetworks if needed.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
  • Loading branch information
zeeke committed Aug 21, 2023
1 parent 99a78b7 commit e8ea99e
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 1 deletion.
36 changes: 36 additions & 0 deletions controllers/sriovnetwork_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,18 @@ import (
"reflect"

netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"k8s.io/client-go/util/workqueue"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -141,6 +146,14 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request
err = r.Get(ctx, types.NamespacedName{Name: netAttDef.Name, Namespace: netAttDef.Namespace}, found)
if err != nil {
if errors.IsNotFound(err) {
targetNamespace := &corev1.Namespace{}
err = r.Get(ctx, types.NamespacedName{Name: netAttDef.Namespace}, targetNamespace)
if errors.IsNotFound(err) {
// Target namespace doesn't exist, avoid retrying with exponential backoff
reqLogger.Info("Target namespace doesn't exist, cannot create NetworkAttachmentDefinition", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name)
return reconcile.Result{}, nil
}

reqLogger.Info("NetworkAttachmentDefinition CR not exist, creating")
err = r.Create(ctx, netAttDef)
if err != nil {
Expand Down Expand Up @@ -174,8 +187,31 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request

// SetupWithManager sets up the controller with the Manager.
func (r *SriovNetworkReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Reconcile when the target namespace is created after the SriovNetwork object.
namespaceHandler := handler.Funcs{
CreateFunc: func(e event.CreateEvent, q workqueue.RateLimitingInterface) {
networkList := sriovnetworkv1.SriovNetworkList{}
err := r.List(context.Background(),
&networkList,
client.MatchingFields{"spec.networkNamespace": e.Object.GetName()},
)
if err != nil {
log.Log.WithName("SriovNetworkReconciler").
Info("Can't list SriovNetworks for namespace", "resource", e.Object.GetName(), "error", err)
}

for _, network := range networkList.Items {
q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
Namespace: network.Namespace,
Name: network.Name,
}})
}
},
}

return ctrl.NewControllerManagedBy(mgr).
For(&sriovnetworkv1.SriovNetwork{}).
Watches(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForObject{}).
Watches(&source.Kind{Type: &corev1.Namespace{}}, &namespaceHandler).
Complete(r)
}
50 changes: 50 additions & 0 deletions controllers/sriovnetwork_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"time"

netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/retry"
dynclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -214,6 +216,54 @@ var _ = Describe("SriovNetwork Controller", func() {
Expect(err).NotTo(HaveOccurred())
})
})

Context("When the target NetworkNamespace doesn't exists", func() {
It("should create the NetAttachDef when the namespace is created", func() {
cr := sriovnetworkv1.SriovNetwork{
ObjectMeta: metav1.ObjectMeta{
Name: "test-missing-namespace",
Namespace: testNamespace,
},
Spec: sriovnetworkv1.SriovNetworkSpec{
NetworkNamespace: "ns-xxx",
ResourceName: "resource_missing_namespace",
IPAM: `{"type":"dhcp"}`,
Vlan: 200,
},
}
var err error
expect := generateExpectedNetConfig(&cr)

err = k8sClient.Create(goctx.TODO(), &cr)
Expect(err).NotTo(HaveOccurred())

DeferCleanup(func() {
err = k8sClient.Delete(goctx.TODO(), &cr)
Expect(err).NotTo(HaveOccurred())
})

// Sleep 3 seconds to be sure the Reconcile loop has been invoked. This can be improved by exposing some information (e.g. the error)
// in the SriovNetwork.Status field.
time.Sleep(3 * time.Second)

netAttDef := &netattdefv1.NetworkAttachmentDefinition{}
err = k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: "ns-xxx"}, netAttDef)
Expect(err).To(HaveOccurred())

err = k8sClient.Create(goctx.TODO(), &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "ns-xxx"},
})
Expect(err).NotTo(HaveOccurred())

err = util.WaitForNamespacedObject(netAttDef, k8sClient, "ns-xxx", cr.GetName(), util.RetryInterval, util.Timeout)
Expect(err).NotTo(HaveOccurred())

anno := netAttDef.GetAnnotations()
Expect(anno["k8s.v1.cni.cncf.io/resourceName"]).To(Equal("openshift.io/" + cr.Spec.ResourceName))
Expect(strings.TrimSpace(netAttDef.Spec.Config)).To(Equal(expect))
})
})

})
})

Expand Down
12 changes: 11 additions & 1 deletion controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
. "github.com/onsi/gomega"
openshiftconfigv1 "github.com/openshift/api/config/v1"
mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
"go.uber.org/zap/zapcore"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -64,7 +65,12 @@ const (
)

var _ = BeforeSuite(func(done Done) {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
logf.SetLogger(zap.New(
zap.WriteTo(GinkgoWriter),
zap.UseDevMode(true),
func(o *zap.Options) {
o.TimeEncoder = zapcore.RFC3339NanoTimeEncoder
}))

// Go to project root directory
os.Chdir("..")
Expand Down Expand Up @@ -103,6 +109,10 @@ var _ = BeforeSuite(func(done Done) {
})
Expect(err).ToNot(HaveOccurred())

k8sManager.GetCache().IndexField(context.Background(), &sriovnetworkv1.SriovNetwork{}, "spec.networkNamespace", func(o client.Object) []string {
return []string{o.(*sriovnetworkv1.SriovNetwork).Spec.NetworkNamespace}
})

err = (&SriovNetworkReconciler{
Client: k8sManager.GetClient(),
Scheme: k8sManager.GetScheme(),
Expand Down
4 changes: 4 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ func main() {
os.Exit(1)
}

mgrGlobal.GetCache().IndexField(context.Background(), &sriovnetworkv1.SriovNetwork{}, "spec.networkNamespace", func(o client.Object) []string {
return []string{o.(*sriovnetworkv1.SriovNetwork).Spec.NetworkNamespace}
})

if err := initNicIDMap(); err != nil {
setupLog.Error(err, "unable to init NicIdMap")
os.Exit(1)
Expand Down

0 comments on commit e8ea99e

Please sign in to comment.