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

KIALI-652 Add graph appender to flag undefined services #183

Merged
merged 1 commit into from May 8, 2018

Conversation

@jshaughn
Copy link
Contributor

commented May 2, 2018

No description provided.

@jshaughn jshaughn force-pushed the jshaughn:kiali-652 branch from 8737c55 to efe0ca5 May 2, 2018
rate, hasRate := child.Metadata["rate"]
if hasRate && rate.(float64) == 0 {
// determine if there is a backing service for this node
serviceDetails, err := istioClient.GetServiceDetails(namespaceName, strings.Split(n.Name, ".")[0])

This comment has been minimized.

Copy link
@lucasponce

lucasponce May 4, 2018

Contributor

Jumping here because I'm coding KIALI-636 and I guess we need to unify appenders or change the appenders architecture.

I mean, each separate appender makes a call into the backend, and then try to identify the condition to modify the graph, iterating per service.

But, when we have separated appenders, then each one makes a call, and I think that it is duplicating, just because, appenders are independent.

So, without enter in re-arch, I guess it makes sense to use a single appender, fetch the details for service, and check, circuit breakers, route rules, and other things, as per service we only need to fetch info once.

This comment has been minimized.

Copy link
@jshaughn

jshaughn May 7, 2018

Author Contributor

@lucasponce If the single call can provide the information needed then this filtering could feasibly be combined with other badging, etc. Although, I think this could be useful as the first appender, to filter away dead services before subsequent appenders get called. It should not be too heavy because it will inquire only about services with no traffic, and it would not use GetServiceDetails but instead use directly the call like getServicePods.

@jshaughn

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

This impl did not take into consideration service version, which is not adequate. Now waiting on Kiali-683 for necessary API support to get this right.

@jshaughn jshaughn force-pushed the jshaughn:kiali-652 branch 4 times, most recently from 03cd298 to ee4e951 May 8, 2018
…o service or no pods))
@jshaughn jshaughn force-pushed the jshaughn:kiali-652 branch from ee4e951 to 721a6b0 May 8, 2018
@jshaughn jshaughn removed the do not merge label May 8, 2018
@jshaughn

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

There have been some changes to the original design, see the Jira for the final design choices for the feature. Taking the DoNotMerge label off. @lucasponce It may make sense to later combine this with other appenders, but because it only makes backend calls for services with no traffic, and it also may filter nodes, it may be useful to just leave it as an initial appender.

Copy link
Contributor

left a comment

LGTM

@jshaughn jshaughn merged commit f77d053 into kiali:master May 8, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jshaughn jshaughn deleted the jshaughn:kiali-652 branch Dec 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.