provided an ability to create additional endpoints #284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nirvanagit
reviewed
Feb 21, 2023
admiral/pkg/clusters/serviceentry.go
Outdated
| return fmt.Errorf("failed deleting additional endpoints for serviceentry for SE host: '%s' and additional endpoint suffixes %s", destinationHostName, additionalEndpointSuffixes) | ||
| } | ||
|
|
||
| defaultVSName := getIstioResourceName(virtualServiceHostnames[0], "-vs") |
There was a problem hiding this comment.
Is there another way we can identify such virtual services? May be using identity of the asset or something (which is added as a label), without deriving it from the service entry?
nirvanagit
reviewed
Feb 21, 2023
| // The list suffixes defined in admiralparams.AdditionalEndpointSuffixes will used to generate the endpoints | ||
| func createAdditionalEndpoints(ctx context.Context, serviceEntry *networking.ServiceEntry, namespace string, rc *RemoteController) error { | ||
|
|
||
| additionalEndpointSuffixes := common.GetAdditionalEndpointSuffixes() |
There was a problem hiding this comment.
Both create and delete are using common functionality, may be abstract them in a function so both can reuse it.
nirvanagit
reviewed
Feb 23, 2023
nirvanagit
reviewed
Feb 23, 2023
admiral/pkg/clusters/serviceentry.go
Outdated
| } | ||
| } | ||
| if result == nil { | ||
| return nil, fmt.Errorf("no virtualservice found with labels %s and annotation %s=%s", listOptions.LabelSelector, resourceCreatedByAnnotationLabel, resourceCreatedByAnnotationValue) |
There was a problem hiding this comment.
Is this a valid scenario where there are no VS created for dependency proxy in a given cluster?
There was a problem hiding this comment.
@nirvanagit , this is independent of DependencyProxy. This is the case where the services are part of mesh and communication is not being proxied through a proxy mesh service.
nirvanagit
approved these changes
Feb 24, 2023
Signed-off-by: Shriram Sharma <[email protected]>
Signed-off-by: Shriram Sharma <[email protected]>
Signed-off-by: Shriram Sharma <[email protected]>
Signed-off-by: Shriram Sharma <[email protected]>
Signed-off-by: Shriram Sharma <[email protected]>
shriramsharma
force-pushed
the
additional_endpoints
branch
from
5e465d6 to
02ccfcd
Compare
kpharasi
pushed a commit
that referenced
this pull request
Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provides an ability to generate additional endpoints for a service
The additional endpoints will be generated based on the
additional_endpoints_suffixesarray passed as the command line param.Admiral will use the passed suffixes to generate the endpoints by creating a VirtualService, which will route the corresponding ServiceEntry endpoint