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

Stop creation of service entires with dummy endpoints #255

Merged
merged 2 commits into from
Sep 2, 2022

Conversation

vrushalijoshi
Copy link
Collaborator

Updates to avoid creation of service entry with dummy endpoints. Include updates to delete existing service entry with only dummy endpoints.

@vrushalijoshi vrushalijoshi force-pushed the fix-se-creation-with-dummy-endpoints branch from 5c054e2 to 9b92b41 Compare August 30, 2022 21:17
@aattuluri aattuluri changed the title Avoid creation of service entry with dummy endpoints Stop creation of service entry with dummy endpoints Aug 30, 2022
@aattuluri aattuluri changed the title Stop creation of service entry with dummy endpoints Stop creation of service entires with dummy endpoints Aug 30, 2022
Copy link
Contributor

@aattuluri aattuluri left a comment

Choose a reason for hiding this comment

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

The functionality looks good. I think it can be simplified.

log.Infof(LogFormat+" SE=%s", op, "ServiceEntry", obj.Name, rc.ClusterID, "New SE", obj.Spec.String())
} else {
//se will be created if endpoints are valid, in case they are not valid se will be created with just valid endpoints
if areEndpointsValid || (!areEndpointsValid && len(validEndpoints) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be simpler if we can filter them inline during validation. Log any filtered endpoints in the method itself.

vjoshi3 added 2 commits August 31, 2022 15:51
Signed-off-by: vjoshi3 <vrushali_joshi@intuit.com>
Signed-off-by: vjoshi3 <vrushali_joshi@intuit.com>
@vrushalijoshi vrushalijoshi force-pushed the fix-se-creation-with-dummy-endpoints branch from b824ccd to a44f07a Compare August 31, 2022 22:51
Copy link
Contributor

@aattuluri aattuluri left a comment

Choose a reason for hiding this comment

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

lgtm

@vrushalijoshi vrushalijoshi merged commit b371d75 into master Sep 2, 2022
@vrushalijoshi vrushalijoshi deleted the fix-se-creation-with-dummy-endpoints branch September 2, 2022 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants