Skip to content

Commit

Permalink
Merge pull request #493 from zeeke/ocpbugs-8266
Browse files Browse the repository at this point in the history
SriovNetwork: react on namespace creation
  • Loading branch information
SchSeba committed Sep 14, 2023
2 parents 9214273 + 66a50a1 commit 08badf6
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 3 deletions.
33 changes: 32 additions & 1 deletion controllers/sriovibnetwork_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ 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/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/workqueue"
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 @@ -136,10 +139,16 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}
// Check if this NetworkAttachmentDefinition already exists
found := &netattdefv1.NetworkAttachmentDefinition{}

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) {
reqLogger.Info("Target namespace doesn't exist, NetworkAttachmentDefinition will be created when namespace is available", "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 @@ -172,8 +181,30 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque

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

for _, network := range ibNetworkList.Items {
q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
Namespace: network.Namespace,
Name: network.Name,
}})
}
},
}
return ctrl.NewControllerManagedBy(mgr).
For(&sriovnetworkv1.SriovIBNetwork{}).
Watches(&netattdefv1.NetworkAttachmentDefinition{}, &handler.EnqueueRequestForObject{}).
Watches(&corev1.Namespace{}, &namespaceHandler).
Complete(r)
}
47 changes: 47 additions & 0 deletions controllers/sriovibnetwork_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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"
Expand Down Expand Up @@ -189,6 +190,52 @@ var _ = Describe("SriovIBNetwork 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.SriovIBNetwork{
ObjectMeta: metav1.ObjectMeta{
Name: "test-missing-namespace",
Namespace: testNamespace,
},
Spec: sriovnetworkv1.SriovIBNetworkSpec{
NetworkNamespace: "ib-ns-xxx",
ResourceName: "resource_missing_namespace",
IPAM: `{"type":"dhcp"}`,
},
}
var err error
expect := generateExpectedIBNetConfig(&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 SriovIBNetwork.Status field.
time.Sleep(3 * time.Second)

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

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

err = util.WaitForNamespacedObject(netAttDef, k8sClient, "ib-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))
})
})
})

func generateExpectedIBNetConfig(cr *sriovnetworkv1.SriovIBNetwork) string {
Expand Down
34 changes: 33 additions & 1 deletion controllers/sriovnetwork_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ 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/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"

"k8s.io/client-go/util/workqueue"
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 @@ -140,6 +142,13 @@ 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) {
reqLogger.Info("Target namespace doesn't exist, NetworkAttachmentDefinition will be created when namespace is available", "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 @@ -173,8 +182,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(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) {
networkList := sriovnetworkv1.SriovNetworkList{}
err := r.List(ctx,
&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(&netattdefv1.NetworkAttachmentDefinition{}, &handler.EnqueueRequestForObject{}).
Watches(&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
16 changes: 15 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,14 @@ 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}
})

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

err = (&SriovNetworkReconciler{
Client: k8sManager.GetClient(),
Scheme: k8sManager.GetScheme(),
Expand Down
8 changes: 8 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ 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}
})

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

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

0 comments on commit 08badf6

Please sign in to comment.