Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

De duplication logic to NF Deploy Fn Param Ref #494

Merged
merged 4 commits into from
Jan 31, 2024
Merged

De duplication logic to NF Deploy Fn Param Ref #494

merged 4 commits into from
Jan 31, 2024

Conversation

aakashchan
Copy link
Contributor

These changes will ensure we have nf deploy fn to apply de dupulication logic to NF Deploy Param Ref

  1. Changes to Add Dependency to check if it already exist before adding. Thanks @gvbalaji for the code snippet in chat.
  2. Added test cases to handle dependency, if same file is present multiple times. Its not in our use case, but its better to add that too.
  3. Added changes to pipeline tests to ensure, if I run the NF Deploy Fn multiples after that, it doesn't break the idempotency principle.

Docker image I used for testing - docker.io/lostbrain/nfdeployfn:test-5

@nephio-prow nephio-prow bot requested review from johnbelamaric and s3wong January 25, 2024 06:40
gvbalaji
gvbalaji approved these changes Jan 31, 2024
View reviewed changes
Copy link

@gvbalaji gvbalaji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@arora-sagar
Copy link

I am just testing it.

@electrocucaracha
Copy link
Member

It seems like the Unit test is passing . I'm okay with these changes, but I'll wait until @arora-sagar ends his testing.

@arora-sagar
Copy link

Looks good not, no duplicates. We can merge I suppose.

apiVersion: workload.nephio.org/v1alpha1
kind: NFDeployment
metadata: # kpt-merge: smf-example/smf-example
  name: smf-core
  namespace: oai-core
  annotations:
    internal.kpt.dev/upstream-identifier: 'workload.nephio.org|NFDeployment|smf-example|smf-example'
spec:
  provider: smf.openairinterface.org
  parametersRefs:
  - name: oai-smf-config
    apiVersion: workload.nephio.org/v1alpha1
    kind: NFConfig
  - name: smf-core-upf-edge
    apiVersion: ref.nephio.org/v1alpha1
    kind: Config
  networkInstances:
  - name: vpc-internal
    interfaces:
    - n4
  interfaces:
  - name: n4
    ipv4:
      address: 172.1.0.254/24
      gateway: 172.1.0.1
    vlanID: 4
  capacity:
    maxDownlinkThroughput: "0"
    maxNFConnections: 5
    maxSessions: 500
    maxUplinkThroughput: "0"

@electrocucaracha
Copy link
Member

/lgtm

@nephio-prow nephio-prow bot added the lgtm label Jan 31, 2024
@s3wong s3wong requested a review from gvbalaji January 31, 2024 21:29
@s3wong
Copy link

s3wong commented Jan 31, 2024

/lgtm

@gvbalaji
Copy link

/approve
/lgtm

@s3wong
Copy link

s3wong commented Jan 31, 2024

/approve

@nephio-prow nephio-prow bot added the approved label Jan 31, 2024
@nephio-prow Nephio PROW
Copy link
Contributor

nephio-prow bot commented Jan 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gvbalaji, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nephio-prow nephio-prow bot merged commit e1135be into nephio-project:main Jan 31, 2024
19 checks passed
@gvbalaji
Copy link

gvbalaji commented Feb 1, 2024

thanks @aakashchan for the quick fix.

@radoslawc
Copy link
Contributor

Reverting the change for testing new release img.

radoslawc added a commit that referenced this pull request Feb 2, 2024
@radoslawc
Revert "De duplication logic to NF Deploy Fn Param Ref (#494)"
f5475de 
This reverts commit e1135be.
This was referenced Feb 2, 2024
Closed
Merged
nephio-prow bot pushed a commit that referenced this pull request Feb 2, 2024
@radoslawc
Revert "De duplication logic to NF Deploy Fn Param Ref" (#498)
b33fe26 
Reverts #494
aakashchan added a commit to aakashchan/nephio that referenced this pull request Feb 14, 2024
@aakashchan
De duplication logic to NF Deploy Fn Param Ref (nephio-project#494)
a16a3db 
These changes will ensure we have nf deploy fn to apply de dupulication
logic to NF Deploy Param Ref

1. Changes to Add Dependency to check if it already exist before adding.
Thanks @gvbalaji for the code snippet in chat.
2. Added test cases to handle dependency, if same file is present
multiple times. Its not in our use case, but its better to add that too.
3. Added changes to pipeline tests to ensure, if I run the NF Deploy Fn
multiples after that, it doesn't break the idempotency principle.


Docker image I used for testing -
[docker.io/lostbrain/nfdeployfn:test-5](http://docker.io/lostbrain/nfdeployfn:test-5)
@nephio-prow nephio-prow bot mentioned this pull request Feb 14, 2024
Merged
PrimalPimmy pushed a commit to 5GSEC/nephio that referenced this pull request Aug 2, 2024
@aakashchan @PrimalPimmy
De duplication logic to NF Deploy Fn Param Ref (nephio-project#494)
5b32639 
These changes will ensure we have nf deploy fn to apply de dupulication
logic to NF Deploy Param Ref

1. Changes to Add Dependency to check if it already exist before adding.
Thanks @gvbalaji for the code snippet in chat.
2. Added test cases to handle dependency, if same file is present
multiple times. Its not in our use case, but its better to add that too.
3. Added changes to pipeline tests to ensure, if I run the NF Deploy Fn
multiples after that, it doesn't break the idempotency principle.


Docker image I used for testing -
[docker.io/lostbrain/nfdeployfn:test-5](http://docker.io/lostbrain/nfdeployfn:test-5)
PrimalPimmy pushed a commit to 5GSEC/nephio that referenced this pull request Aug 2, 2024
@radoslawc @PrimalPimmy
Revert "De duplication logic to NF Deploy Fn Param Ref" (nephio-proje…
8832520 
…ct#498)

Reverts nephio-project#494
nephio-prow bot pushed a commit that referenced this pull request Sep 10, 2024
@aakashchan
De duplication logic to NF Deploy Fn Param Ref (#494) (#525)
6917498 
Solves #493 
These changes will ensure we have nf deploy fn to apply de dupulication
logic to NF Deploy Param Ref

1. Changes to Add Dependency to check if it already exist before adding.
Thanks @gvbalaji for the code snippet in chat.
2. Added test cases to handle dependency, if same file is present
multiple times. Its not in our use case, but its better to add that too.
3. Added changes to pipeline tests to ensure, if I run the NF Deploy Fn
multiples after that, it doesn't break the idempotency principle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s3wong s3wong s3wong approved these changes

@henderiw henderiw Awaiting requested review from henderiw

@kispaljr kispaljr Awaiting requested review from kispaljr

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@arora-sagar arora-sagar Awaiting requested review from arora-sagar

@gvbalaji gvbalaji Awaiting requested review from gvbalaji

Assignees

@s3wong s3wong

@electrocucaracha electrocucaracha

@gvbalaji gvbalaji

Labels
approved lgtm
Projects
Nephio R2
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aakashchan @arora-sagar @electrocucaracha @s3wong @gvbalaji @radoslawc