nephio-controller-manager initial start

[johnbelamaric]

Create the controller manager for the following Nephio components on management cluster and create a single binary

• Specializers (IPAM, VLAN)
• NF Topology
• Status Aggregator

Tasks for this issue are

• Crete a design proposal with a digram explaning the purpose and components of the nephio-controller-manager
• Implement the nephio-controller-manager

Document and create the initial binary
Unit test coverage.

[johnbelamaric]

/label area/package-management
/label sig/automation

[nephio-prow]

@johnbelamaric: The label(s) /label area/package-management , /label sig/automation cannot be applied. These labels are supported: ``. Is this label configured under labels -> additional_labels or `labels -> restricted_labels` in `plugin.yaml`?

In response to this:

/label area/package-management
/label sig/automation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[johnbelamaric]

@radoslawc see above ^

[johnbelamaric]

/area package-managment
/sig automation

trying these prow commands...

[nephio-prow]

@johnbelamaric: The label(s) area/package-managment cannot be applied, because the repository doesn't have them.

In response to this:

/area package-managment
/sig automation

trying these prow commands...

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[johnbelamaric]

/area package-management

helps if I can spell correctly

[johnbelamaric]

Nice, that worked :)

[johnbelamaric]

@ssharmasandeep We would want a one-pager design/diagram explaining what this is and what it contains, along with code to build the actual binary - the specific reconcilers are mostly not-yet-available, although the IPAM specializer should be.

See the kubernetes-controller-manager and porch-controllers binaries for examples of similar things in other projects.

[ssharmasandeep]

@johnbelamaric please review. I have started to put together the following with https://github.com/nephio-project/edge-status-aggregator to start with.

The content of the nephio-controller-manager will be,

• main.go: The main package will declare an interface called Reconciler, which will have the method SetupWithManager along with other methods like InitDefaults, and BindFlags.
• controllers: These will be the NF topology, Specialisers and Status aggregator controllers. Each controller will implement the Reconcilers interface.
• Dockerfile: To create a docker image of the nephio-controller-manager. It will build a single binary For example go build -o nephio-controller-manager -v .
• Makefile:

Purpose of this controller: The purpose of this controller is to create a single binary for all the nephio management controllers.

Implementation Flow:

• main.go will import all the controller packages that will be part of the controller-manager.
• The package main will define an interface which will be implemented by all the controllers part of it.
• A map will be created in the package main, which will have controller name to controller's Reconciler mapping.
• Example var ( reconcilers = map[string]Reconciler{ "nfdeploy": &nfdeploy.NfDeployReconciler{}, } )
• The logic in package main will basically allocate controllerruntime NewManager with manager options.
• Then the logic will loop over each controller Reconciler interface and call the SetupWIthManager for each controller.

Each Controller will implement the SetupWIthManager

• Add GV to the scheme of manager created in package main.
• Allocate a controllerruntime NewControllerManagedBy(mgr) for the controller, manager by the main manager.
• This just describes the interface of each controller, on how it integrates with the main manager.

The implementation plan is to create the main framework, and then add each controller to it by implementing the Reconciler interface.

Started creating the framework with https://github.com/nephio-project/edge-status-aggregator to start with, once it is working then adding the rest of the controller will be a repeat of this for each controller.

In the last meeting you had asked me to put the doc in some repo, I will do it if this looks fine or after addressing your review comments.

[johnbelamaric]

This looks good to me, I don't think we need a doc beyond this.

[gvbalaji]

Thanks @ssharmasandeep . This is a good readme for the manager when we merge .

[henderiw]

yes looks good.

Here is some code I use to setup various controllers within a given manager, if it is useful. It even uses generic Events, but I don't believe we need it here.

// Setup package controllers.
func Setup(ctx context.Context, mgr ctrl.Manager, opts *shared.Options) error {
ipamproxyClient := clientproxy.New(ctx, &clientproxy.Config{
Registrator: opts.Registrator,
})
opts.IpamClientProxy = ipamproxyClient

eventChs := map[schema.GroupVersionKind]chan event.GenericEvent{}
for _, setup := range []func(ctrl.Manager, *shared.Options) (schema.GroupVersionKind, chan event.GenericEvent, error){
	networkinstance.Setup,
	prefix.Setup,
	allocation.Setup,
} {
	gvk, geCh, err := setup(mgr, opts)
	if err != nil {
		return err
	}
	eventChs[gvk] = geCh
}
for _, setup := range []func(ctrl.Manager, *shared.Options) error{
	injector.Setup,
} {
	if err := setup(mgr, opts); err != nil {
		return err
	}
}

ipamproxyClient.AddEventChs(eventChs)

return nil

}

This is the injector which will become specialised:

// SetupWithManager sets up the controller with the Manager.
func Setup(mgr ctrl.Manager, options *shared.Options) error {
ge := make(chan event.GenericEvent)
r := &reconciler{
kind: "ipam",
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
porchClient: options.PorchClient,
IpamClientProxy: options.IpamClientProxy,

	pollInterval: options.Poll,
	finalizer:    resource.NewAPIFinalizer(mgr.GetClient(), finalizer),
}

return ctrl.NewControllerManagedBy(mgr).
	For(&porchv1alpha1.PackageRevision{}).
	Watches(&source.Channel{Source: ge}, &handler.EnqueueRequestForObject{}).
	Complete(r)

}

[johnbelamaric]

Any update here, @ssharmasandeep ?

[ssharmasandeep]

@johnbelamaric I started to code the framework with https://github.com/nephio-project/edge-status-aggregator , but there is no tangible status as of now. I will try my best to create a PR this week.

[ssharmasandeep]

@henderiw Please point me to the controllers which we have to include in the controller manager. I have the framework ready, which I tested with the nephio-poc controller.
I tried to look it up in nephio-project but was not able to find the controllers.
Also, I need to understand the configmap requirement you mentioned in last week's call. Let's catch up after the call today ?
-Sandeep

[henderiw]

#177
provides an operator for the specializers only. This also provides a mechanism to add multiple controllers as I shared before.

[ssharmasandeep]

I have added the specializers to the controller manager as per the flow that was sent earlier to this ticket.
@henderiw the logic is similar to what you suggested above, but I had to move around a little code in order to integrate the specialzers. I will raise the PR once I know the location to checkin this code ( by tomorrow), for now please look at https://github.com/ssharmasandeep/nephio-controller-manager.git and let me know if the changes done to specializers look ok ?

@johnbelamaric

1. Where should this operator be checked in? Or are we going to make the current nephio repo as this manager?
2. I will add the Dockerfile/Makefile and raise PR by tomorrow, after unit testing.
3. Please confirm the following list is all we need,
   specializer controller ( this is added)
   edge-status-aggregator (this is added)
   repo-controller
   bootstrap-controller

ignore the nephio-poc-controller, it will not be part of the PR.

-Sandeep

[henderiw]

Awesome thx, I will look at it later today

…

On 10 May 2023, at 09:50, Sandeep Sharma ***@***.***> wrote: I have added the specializers to the controller manager as per the flow that was sent earlier to this ticket. @henderiw <https://github.com/henderiw> the logic is similar to what you suggested above, but I had to move around a little code in order to integrate the specialzers. I will raise the PR once I know the location to checkin this code ( by tomorrow), for now please look at https://github.com/ssharmasandeep/nephio-controller-manager.git and let me know if the changes done to specializers look ok ? @johnbelamaric <https://github.com/johnbelamaric> Where should this operator be checked in? Or are we going to make the current nephio repo as this manager? I will add the Dockerfile/Makefile and raise PR by tomorrow, after unit testing. Please confirm the following list is all we need, specializer controller ( this is added) edge-status-aggregator (this is added) repo-controller bootstrap-controller ignore the nephio-poc-controller, it will not be part of the PR. -Sandeep — Reply to this email directly, view it on GitHub <#91 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACMXXVGFF5SCTIJCELLGM33XFOMLTANCNFSM6AAAAAAWS66T4M>. You are receiving this because you were mentioned.

[johnbelamaric]

• Where should this operator be checked in? Or are we going to make the current nephio repo as this manager?

I was thinking a nephio-controller-manager directory (and go.mod) in the nephio repo.

[johnbelamaric]

Vish noted this the other day, perhaps we should take some inspiration from it: https://github.com/golang-standards/project-layout

So may something like:

nephio/
  cmd/
    pkg/
      go.mod
    nephio-controller-manager/
      go.mod
    another-binary/
      go.mod
  controllers
    pkg/
      go.mod
    controller-a
      go.mod
    controller-b
      go.mod
    specializer-a
      go.mod
  krm-functions/
    pkg/ (probably should rename lib this, but not urgent, could be post R1)
      go.mod
    function-a
      go.mod
    function-b
      go.mod

[henderiw]

is it not better to make an operators directory if we decide to have multiple operators at some point.

nephio/
operators
controllers
krm-functions

[johnbelamaric]

The cmd directory usually is used for building binaries. But if we want to put "operator" binaries in operators instead, I am good with that.

[henderiw]

yes I know but I would like to have the ability to create multiple operators and within the operator we could have a local structure.

[henderiw]

some comments on the remote code

• do we believe global var is a good practice?
• the ctx I would from the mgr as he is the owner of the ctx.
• Is it not better to have a setup method that does the specifics for each controller and attach them to the manager and that's it.

[ssharmasandeep]

do we believe global var is a good practice?
Sandeep: No, should be avoided.. I will fix it.
the ctx I would from the mgr as he is the owner of the ctx.
Sandeep: agree, will make the required change.
Is it not better to have a setup method that does the specifics for each controller and attach them to the manager and that's it.
Sandeep: That is the intention of the setupwithmanager method implemented by each controller.

[ssharmasandeep]

So looks like we agreed on the following structure.. I will follow this.
nephio/
operators
controllers
krm-functions

[henderiw]

@ssharmasandeep #197 have a look. I added a prototype and I want all the controllers to use it as it eases integration.
it allows controllers to register and enable them independently.

Have a look.

[johnbelamaric]

#208

[johnbelamaric]

merged