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.

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

Remove duplicated IPAM code for NSE applications #1409

Closed
denis-tingaikin opened this issue Jan 10, 2023 · 1 comment
Closed

Remove duplicated IPAM code for NSE applications #1409

denis-tingaikin opened this issue Jan 10, 2023 · 1 comment
Assignees
Labels
good first issue Good for newcomers refactor The issue where we need to refactor something

Comments

@denis-tingaikin
Copy link
Member

denis-tingaikin commented Jan 10, 2023

Description

Since we'd merged #1407 we can remove copy-pasted code for NSE applications.

Defenition of done

  1. Remove duplicated code
    https://github.com/networkservicemesh/cmd-nse-icmp-responder/blob/main/main.go#L409-L415
    https://github.com/networkservicemesh/cmd-nse-icmp-responder-vpp/blob/main/main.go#L328-L334
    https://github.com/networkservicemesh/cmd-nse-remote-vlan/blob/main/main.go#L318-L329 and replace it with groupipam.NewServer()

  2. Check other NSE applications https://github.com/networkservicemesh/?q=-nse&type=all&language=&sort= and make sure that we don't have copy-pasted IPAM logic

  3. Run a few integration tests with changed IPAM logic and make sure that we don't have regression

@denis-tingaikin denis-tingaikin added bug Something isn't working good first issue Good for newcomers refactor The issue where we need to refactor something and removed bug Something isn't working labels Jan 10, 2023
@glazychev-art glazychev-art moved this to Under review in Release v1.8.0 Jan 12, 2023
@bszirtes bszirtes self-assigned this Mar 20, 2024
bszirtes added a commit to Nordix/cmd-nse-icmp-responder-vpp that referenced this issue Mar 21, 2024
bszirtes added a commit to Nordix/cmd-nse-icmp-responder-vpp that referenced this issue Mar 21, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/cmd-nse-icmp-responder-vpp that referenced this issue Mar 22, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/cmd-nse-l7-proxy that referenced this issue Mar 26, 2024
Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/cmd-nse-l7-proxy that referenced this issue Mar 26, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/nsm-cmd-nse-remote-vlan that referenced this issue Mar 26, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/nsm-cmd-nse-remote-vlan that referenced this issue Mar 26, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/nsm-cmd-nse-remote-vlan that referenced this issue Mar 26, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
bszirtes added a commit to Nordix/nsm-cmd-nse-remote-vlan that referenced this issue Mar 26, 2024
NSM issue link: networkservicemesh/sdk#1409

Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
@denis-tingaikin
Copy link
Member Author

@bszirtes Nice work! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers refactor The issue where we need to refactor something
Projects
Status: Done
Development

No branches or pull requests

3 participants