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

Start two CNS servers with different handlers #2650

Merged
merged 42 commits into from
Apr 30, 2024
Merged

Conversation

paulyufan2
Copy link
Contributor

@paulyufan2 paulyufan2 commented Mar 19, 2024

Reason for Change:

This PR is to start two CNS servers:
1.Localhost echo server => it will use default URL for CNI func handlers
2.HTTP legacy server => it will use private VM's ip or IP that customer provides for DNC/CNS func handlers

Issue Fixed:

Requirements:

Notes:

@paulyufan2 paulyufan2 requested review from a team as code owners March 19, 2024 21:30
@paulyufan2 paulyufan2 requested a review from csfmomo March 19, 2024 21:30
@paulyufan2
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cns/restserver/api.go Outdated Show resolved Hide resolved
cns/service/main.go Show resolved Hide resolved
cns/restserver/v2/server.go Outdated Show resolved Hide resolved
cns/service.go Outdated Show resolved Hide resolved
cns/service.go Outdated Show resolved Hide resolved
cns/service.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
@paulyufan2
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cns/service/main.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
cns/restserver/restserver.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
cns/service.go Outdated Show resolved Hide resolved
cns/service.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
@paulyufan2
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cns/restserver/api_test.go Outdated Show resolved Hide resolved
cns/restserver/v2/server.go Show resolved Hide resolved
cns/restserver/v2/server.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
@tamilmani1989 tamilmani1989 added the cns Related to CNS. label Apr 12, 2024
@paulyufan2 paulyufan2 dismissed stale reviews from nddq and rbtr via 54d6c61 April 29, 2024 15:55
@paulyufan2 paulyufan2 requested a review from a team as a code owner April 29, 2024 15:55
paulyufan2 and others added 2 commits April 29, 2024 12:01
Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com>
tamilmani1989
tamilmani1989 previously approved these changes Apr 29, 2024
The startService function is a mistake that has been perpetuated through
time and needs to be removed. As a consequence of its existence, it
requires mixing concerns in all tests and stands in the way of
refactoring.

The addition of two type assertion checks caused this function to break,
because the corresponding options were not set in the Service's
configuration. This is a problem in itself--there's no reason the
options should be pulled out of a map[string]interface{}. Given this, we
can just set these two options in startService, committing sins to
offset the ones in service.(*HTTPRestService).Service.Options.
Copy link
Member

@timraymond timraymond left a comment

Choose a reason for hiding this comment

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

Removing PR block

@paulyufan2
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@paulyufan2 paulyufan2 added this pull request to the merge queue Apr 29, 2024
Merged via the queue into master with commit be2dcc0 Apr 30, 2024
24 of 25 checks passed
@paulyufan2 paulyufan2 deleted the startCnsListeners branch April 30, 2024 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cns Related to CNS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants