OAI RAN NF custom controller - initial version #1

josephthaliath · 2023-09-28T05:55:03Z

Related issue: nephio-project/nephio#364

Note: Used the initial RANDeployment CRD in this version. Will adapt to common NFDeployment CRD.

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

linux-foundation-easycla · 2023-09-28T05:55:07Z

The committers listed above are authorized under a signed CLA.

✅ login: josephthaliath (2d93d7f, e646057, 97caedc, 9a4eb8c, 1ca2b41, 68bd432, 392f52f, 91696b9, 0e0eca9, 0c00f59, cd4c828, 2599fb2, 2f9e8d3, 487ac57, 9fb28da, ee442ce, 2bb244f, bb05f11, d52becd)

debeaueric

Thanks for this first version.
I believe that we must discuss more on the 3gpp parameters and be aligned with the one used for the core.
For example, we should have a list of cells
Some parameters must be required (eg mcc and mnc and in plmn)

henderiw · 2023-09-29T11:32:58Z

all the api definitions should go to /nephio/api repo. Also there is no point of redefining the nf deployment since it already exists in the api repo

iwojdan · 2023-09-29T14:59:20Z

I have some remarks about interface's name in config/samples/randeploymentcuup.yaml file. In line 48 should be f1u interface name not f1c. It should be checked whether the name of network instance is correct in line 46 -(vpc-f1c). Also there is inconsistency with interface's name in line 45 is "e1" and in line 29 is "e1-cu-up" .

iwojdan · 2023-09-29T15:33:10Z

In file /internal/controller/resources_cuup.go there is inconsistency in messages in log.Error:
Line 200 should be log.Error(err, "Interface f1u not found in RANDeployment Spec"), line 188 should be log.Error(err, "Interface n3 not found in RANDeployment Spec"), line 212 - should not be "true" instead of" false", line 214 is log.Error(err, "AMF IP not found in Config Refs AMFDeployment") but ConfigRef is connected with CUCPDeployment not AMF and not AMF IP.

Removing nf_deployment redefinitions Corrections of logging information and yamls Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

josephthaliath · 2023-10-03T05:57:59Z

all the api definitions should go to /nephio/api repo. Also there is no point of redefining the nf deployment since it already exists in the api repo

Thanks @henderiw for the review comment. I have removed the redefinitions of nf deployment. Regarding API definitions for RAN deployment, I have kept it for temporary reference for the initial commits. Later it will be removed and adapted to the common NF deployment.

josephthaliath · 2023-10-03T06:01:36Z

I have some remarks about interface's name in config/samples/randeploymentcuup.yaml file. In line 48 should be f1u interface name not f1c. It should be checked whether the name of network instance is correct in line 46 -(vpc-f1c). Also there is inconsistency with interface's name in line 45 is "e1" and in line 29 is "e1-cu-up" .

Thanks @iwojdan for the review comments. I have updated the file as per your comments in the latest commit. Also wanted to clarify that the files with network instances and interfaces will be generated during deployment in future. I have added these files only for initial references.

josephthaliath · 2023-10-03T06:02:29Z

In file /internal/controller/resources_cuup.go there is inconsistency in messages in log.Error: Line 200 should be log.Error(err, "Interface f1u not found in RANDeployment Spec"), line 188 should be log.Error(err, "Interface n3 not found in RANDeployment Spec"), line 212 - should not be "true" instead of" false", line 214 is log.Error(err, "AMF IP not found in Config Refs AMFDeployment") but ConfigRef is connected with CUCPDeployment not AMF and not AMF IP.

Thanks @iwojdan for your careful review. I have updated the code as per your comments.

josephthaliath · 2023-10-04T17:22:25Z

Hi @henderiw , @johnbelamaric, @tliron, @s3wong. I have updated the pr based on the comments received. Let me if you have any more comments and if this initial commit can be merged ?

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

nephio-prow · 2023-10-11T05:54:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: josephthaliath

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [josephthaliath]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

arora-sagar · 2023-10-19T13:30:56Z

internal/controller/resources_cuup.go

@@ -33,7 +33,7 @@ type CuUpResources struct {

 func (resource CuUpResources) createNetworkAttachmentDefinitionNetworks(templateName string, ranDeploymentSpec *nephiov1alpha1.NFDeploymentSpec) (string, error) {
 	return free5gccontrollers.CreateNetworkAttachmentDefinitionNetworks(templateName, map[string][]nephiov1alpha1.InterfaceConfig{
-		"e1":  free5gccontrollers.GetInterfaceConfigs(ranDeploymentSpec.Interfaces, "e1-cu-up"),
+		"e1":  free5gccontrollers.GetInterfaceConfigs(ranDeploymentSpec.Interfaces, "e1"),


@josephthaliath I feel there is some sort of linking if we are using code from the free5gc repository (free5gccontrollers library from free5g/controller). I guess the better way would be to abstract this kind of code in a common repo something like utils and then use it between both of the repositories. But I suppose that will be some extra work. We should do in R3

Yes right. We need to propose these functions to a common place. Since now we have common NFDeployment, any functions defined for this, can be reused across all NFs,

arora-sagar · 2023-10-27T10:07:11Z

config/samples/cucp_config.yaml

+      name: oai-ran-cu-cp
+      namespace: oai-ran
+    spec:
+      provider: oai-cucp.nephio.org


@josephthaliath for OAI can we use the same provider? I will use {NF_TYPE}.openairinterface.org. For example, for UPF it will be --> upf.openairinterface.org.

@arora-sagar , Sure I will update it. Also you can check the initial packages PR for reference and review. nephio-project/catalog#3

arora-sagar · 2023-11-28T04:50:29Z

api/v1alpha1/ranconfig_types.go

+
+// NOTE: For R2 the DNN.name here should match with NF deployment DNN name. This is a hack for R2
+// DNN defines the Data Network Names Information
+type DNNInfo struct {


@josephthaliath Are you still using it? I think you can remove the DNN part from your section.

@josephthaliath Are you still using it? I think you can remove the DNN part from your section.

@arora-sagar, I added it initially from the PR#57, in anticipation that will be used. I remove this.

arora-sagar · 2023-11-30T11:04:07Z

/lgtm
/approve

henderiw

some code optimisation proposed

henderiw · 2023-11-30T14:16:52Z

internal/controller/conversions.go

+
+package controller
+
+func int32Ptr(val int) *int32 {


there is a pointer library for this
e.g. "k8s.io/utils/pointer"

henderiw · 2023-11-30T14:16:56Z

internal/controller/conversions.go

+	return &a
+}
+
+func int64Ptr(val int) *int64 {


there is a pointer library for this
e.g. "k8s.io/utils/pointer"

henderiw · 2023-11-30T14:17:03Z

internal/controller/conversions.go

+	return &a
+}
+
+func boolPtr(val bool) *bool {


there is a pointer library for this
e.g. "k8s.io/utils/pointer"

henderiw · 2023-11-30T14:18:02Z

internal/controller/interface_configs.go

+	workloadv1alpha1 "github.com/nephio-project/api/workload/v1alpha1"
+)
+
+func GetInterfaceConfigs(interfaceConfigs []workloadv1alpha1.InterfaceConfig, interfaceName string) []workloadv1alpha1.InterfaceConfig {


we should create a library which we can reuse between all NF implementation that use NF Deployment.

henderiw · 2023-11-30T14:18:23Z

internal/controller/network_attachment_definitions.go

+	Version: "v1",
+}
+
+func CreateNetworkAttachmentDefinitionNetworks(templateName string, interfaceConfigs map[string][]workloadv1alpha1.InterfaceConfig) (string, error) {


a library should be used for reuse between NF(s)

henderiw · 2023-11-30T14:19:14Z

internal/controller/randeployment_controller.go

+
+}
+
+func (r *RANDeploymentReconciler) DeleteAll(log logr.Logger, ctx context.Context, ranDeployment *workloadv1alpha1.NFDeployment, nfResource NfResource, configInfo *ConfigInfo) {


you should use
log.FromContext(ctx) iso passing the logr as a fn param.

henderiw · 2023-11-30T14:20:07Z

internal/controller/randeployment_controller.go

+// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.15.0/pkg/reconcile
+func (r *RANDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+
+	logger := log.FromContext(ctx).WithValues("RANDeployment", req.NamespacedName)


this is how we should use logger iso passing it via the fn param

henderiw · 2023-11-30T14:22:24Z

cmd/main.go

+		setupLog.Error(err, "Not able to register workload/v1alpha1 NFDeployment kind")
+	}
+
+	if err = (&controller.RANDeploymentReconciler{


it might be better ti use the pattern w have in core so we can add/delete reconcilers easily.
https://github.com/nephio-project/nephio/blob/1c3aa4d9435f369f707fe9b5587c680cc0e50b4d/operators/nephio-controller-manager/main.go#L165

henderiw · 2023-11-30T14:23:17Z

internal/controller/resources_du.go

+	return []*corev1.ConfigMap{configMap1}
+}
+
+func (resource DuResources) GetDeployment(log logr.Logger, ranDeployment *workloadv1alpha1.NFDeployment, configInfo *ConfigInfo) []*appsv1.Deployment {


we should make libs for these so people can reuse these

henderiw · 2023-11-30T15:49:33Z

The other part is test coverage need to be improved

josephthaliath added 3 commits September 26, 2023 13:14

Update to README.md

2d93d7f

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

Initial files for oai ran operator

e646057

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

Removing not used functions

97caedc

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

josephthaliath mentioned this pull request Sep 28, 2023

OAI RAN NF custom controller nephio-project/nephio#364

Open

debeaueric reviewed Sep 29, 2023

View reviewed changes

Changes to address pr review comments

9a4eb8c

Removing nf_deployment redefinitions Corrections of logging information and yamls Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

Changes to adapt to common NFDeployment

1ca2b41

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

nephio-prow bot added the approved label Oct 11, 2023

josephthaliath and others added 3 commits October 11, 2023 11:31

Further changes for common NFDeployment changes

68bd432

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

Merge branch 'main' into main

392f52f

Update to Dockefile to remove api folder

91696b9

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

josephthaliath requested review from henderiw and arora-sagar October 14, 2023 08:23

Changes to add provider check

0e0eca9

Signed-off-by: josephthaliath <jo.thaliath@samsung.com>

arora-sagar reviewed Oct 19, 2023

View reviewed changes

josephthaliath added 3 commits October 26, 2023 12:44

Adding prow.yaml and minor changes

0c00f59

Minor updates to du files and Makefile

cd4c828

adding sample unit tests, lint related changes

2599fb2

arora-sagar reviewed Oct 27, 2023

View reviewed changes

josephthaliath added 2 commits October 30, 2023 16:12

adding rf sim interface to du

2f9e8d3

update based on new provider name and crd

487ac57

josephthaliath added 3 commits November 16, 2023 17:25

changes to use du service for ue rfsim

9fb28da

Adding DU configuration

ee442ce

Changes to support new NFConfig API

2bb244f

arora-sagar reviewed Nov 28, 2023

View reviewed changes

josephthaliath assigned josephthaliath and jain-ashish-sam Nov 28, 2023

josephthaliath added 2 commits November 29, 2023 15:45

changes to include vendor specifc config

bb05f11

removing unused structure

d52becd

henderiw mentioned this pull request Nov 29, 2023

Define RAN CRDs + allow vendor specific extensions nephio-project/nephio#362

Closed

nephio-prow bot assigned arora-sagar Nov 30, 2023

nephio-prow bot added the lgtm label Nov 30, 2023

arora-sagar unassigned josephthaliath and jain-ashish-sam Nov 30, 2023

arora-sagar removed the request for review from henderiw November 30, 2023 11:10

nephio-prow bot merged commit a3dbc1d into nephio-project:main Nov 30, 2023
6 checks passed

henderiw reviewed Nov 30, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAI RAN NF custom controller - initial version #1

OAI RAN NF custom controller - initial version #1

josephthaliath commented Sep 28, 2023

linux-foundation-easycla bot commented Sep 28, 2023 •

edited

Loading

debeaueric left a comment

henderiw commented Sep 29, 2023

iwojdan commented Sep 29, 2023

iwojdan commented Sep 29, 2023

josephthaliath commented Oct 3, 2023 •

edited

Loading

josephthaliath commented Oct 3, 2023

josephthaliath commented Oct 3, 2023

josephthaliath commented Oct 4, 2023

nephio-prow bot commented Oct 11, 2023

arora-sagar Oct 19, 2023 •

edited

Loading

josephthaliath Oct 19, 2023

arora-sagar Oct 27, 2023

josephthaliath Oct 27, 2023

arora-sagar Nov 28, 2023

josephthaliath Nov 28, 2023

arora-sagar commented Nov 30, 2023 •

edited

Loading

henderiw left a comment

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw Nov 30, 2023

henderiw commented Nov 30, 2023


		}

		func (r RANDeploymentReconciler) DeleteAll(log logr.Logger, ctx context.Context, ranDeployment workloadv1alpha1.NFDeployment, nfResource NfResource, configInfo *ConfigInfo) {

OAI RAN NF custom controller - initial version #1

OAI RAN NF custom controller - initial version #1

Conversation

josephthaliath commented Sep 28, 2023

linux-foundation-easycla bot commented Sep 28, 2023 • edited Loading

debeaueric left a comment

Choose a reason for hiding this comment

henderiw commented Sep 29, 2023

iwojdan commented Sep 29, 2023

iwojdan commented Sep 29, 2023

josephthaliath commented Oct 3, 2023 • edited Loading

josephthaliath commented Oct 3, 2023

josephthaliath commented Oct 3, 2023

josephthaliath commented Oct 4, 2023

nephio-prow bot commented Oct 11, 2023

arora-sagar Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arora-sagar commented Nov 30, 2023 • edited Loading

henderiw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henderiw commented Nov 30, 2023

linux-foundation-easycla bot commented Sep 28, 2023 •

edited

Loading

josephthaliath commented Oct 3, 2023 •

edited

Loading

arora-sagar Oct 19, 2023 •

edited

Loading

arora-sagar commented Nov 30, 2023 •

edited

Loading