Fix free5GC tests and dedup SMF configrefs #48
Conversation
tliron
force-pushed
the
main
branch
2 times, most recently
from
512875c
to
072c270
Compare
tliron
requested review from
arora-sagar
and removed request for
johnbelamaric
README.md
Outdated
|
|
||
| ```sh | ||
| git clone https://github.com/nephio-project/api | ||
| kubectl apply -f api/config/crd/bases/ref.nephio.org_configs.yaml |
There was a problem hiding this comment.
@tliron we can do this instead
kubectl apply -f https://raw.githubusercontent.com/nephio-project/api/main/config/crd/bases/ref.nephio.org_configs.yaml
kubectl apply -f https://raw.githubusercontent.com/nephio-project/api/main/config/crd/bases/workload.nephio.org_nfdeployments.yaml
kubectl apply -f https://github.com/nephio-project/api/blob/main/config/crd/bases/workload.nephio.org_nfconfigs.yaml
There was a problem hiding this comment.
Ah... this is much better. Actually I should remove the CRD dupes from the free5gc repo! And also fix the Makefile to not install these dupes.
test/upf2-n3.yaml
Outdated
There was a problem hiding this comment.
@tliron Don't we need multus definitions for testing the UPF?
There was a problem hiding this comment.
Hm, I didn't change this from the original. Actually I didn't test that UPF works, only that it stands up.
There was a problem hiding this comment.
Then let's try it and then delete this part. I think for the block test we will need it.
test/smf1-config-ref.yaml
Outdated
| spec: | ||
| config: | ||
| apiVersion: workload.nephio.org/v1alpha1 | ||
| kind: NFDeployment | ||
| metadata: | ||
| name: free5gc-upf-1 | ||
| namespace: upf-1 | ||
| namespace: smf-1 |
There was a problem hiding this comment.
Hi in the old version it was "free5gc-upf-1" should we keep UPF here? I think smf config has reference of the upf?
There was a problem hiding this comment.
TBH, the name and namespace are ignored. :) SMF just reads the other properties. However you are right that this is a UPF config, not an SMF config, so I will update.
controllers/nf/amf/resources.go
Outdated
| func createService(amdDeployment *nephiov1alpha1.NFDeployment) *apiv1.Service { | ||
| namespace := amdDeployment.Namespace | ||
| instanceName := amdDeployment.Name |
There was a problem hiding this comment.
| func createService(amdDeployment *nephiov1alpha1.NFDeployment) *apiv1.Service { | |
| namespace := amdDeployment.Namespace | |
| instanceName := amdDeployment.Name | |
| func createService(amfDeployment *nephiov1alpha1.NFDeployment) *apiv1.Service { | |
| namespace := amfDeployment.Namespace | |
| instanceName := amfDeployment.Name |
| @@ -172,7 +172,7 @@ func (r *AMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques | |||
| } | |||
| } | |||
| } else { | |||
| log.Error(err, fmt.Sprintf("Failed to create Deployment %s\n", err.Error())) | |||
| log.Error(err, fmt.Sprintf("Failed to create Deployment %s", err.Error())) | |||
There was a problem hiding this comment.
| log.Error(err, fmt.Sprintf("Failed to create Deployment %s", err.Error())) | |
| log.Error(err, fmt.Sprintf("Failed to create AMF NFDeployment %s", err.Error())) |
There was a problem hiding this comment.
Actually, this log error is about failure to creating a K8s Deployment resource, I will keep the original.
Also: * Update README to install ref.nephio.org Config * Upgrade to Go 1.21.6 * Remove old and unused CRD files * Match AMF code style to that of the other NFs * Small fixes to log and error messages
|
/lgtm |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: s3wong 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:
Approvers can indicate their approval by writing |
Also: