diff --git a/graph/appender/dead_service.go b/graph/appender/dead_service.go new file mode 100644 index 0000000000..9420a7e3dc --- /dev/null +++ b/graph/appender/dead_service.go @@ -0,0 +1,56 @@ +package appender + +import ( + "strings" + + "github.com/kiali/kiali/graph/tree" + "github.com/kiali/kiali/kubernetes" +) + +// DeadServiceAppender is responsible for removing from the graph any service nodes for which +// the service is undefined (presumabley has been removed from K8S) and for which there is +// no traffic reported. (kiali-621) +type DeadServiceAppender struct{} + +func (a DeadServiceAppender) AppendGraph(trees *[]tree.ServiceNode, namespace string) { + istioClient, err := kubernetes.NewClient() + checkError(err) + + for _, tree := range *trees { + applyDeadServices(&tree, namespace, istioClient) + } +} + +func applyDeadServices(n *tree.ServiceNode, namespace string, istioClient kubernetes.IstioClientInterface) { + // set children to list filtered of dead services + filteredChildren := make([]*tree.ServiceNode, 0) + for _, child := range n.Children { + isDead := false + rate, hasRate := child.Metadata["rate"] + serviceName := strings.Split(child.Name, ".")[0] + if hasRate && rate.(float64) == 0 { + // filter the child if it has no backing service + service, err := istioClient.GetService(namespace, serviceName) + if err != nil || service == nil { + isDead = true + } else { + // flag the service if it has a defined service but no pods running for the service version + servicePods, err := istioClient.GetServicePods(namespace, serviceName, child.Version, "") + if err != nil || servicePods == nil || len(servicePods.Items) == 0 { + child.Metadata["isDead"] = "true" + } + } + } + + if !isDead { + filteredChildren = append(filteredChildren, child) + } + } + + n.Children = filteredChildren + + // recurse on the remaining children + for _, child := range n.Children { + applyDeadServices(child, namespace, istioClient) + } +} diff --git a/graph/appender/dead_service_test.go b/graph/appender/dead_service_test.go new file mode 100644 index 0000000000..853e44d759 --- /dev/null +++ b/graph/appender/dead_service_test.go @@ -0,0 +1,140 @@ +package appender + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kiali/kiali/config" + "github.com/kiali/kiali/graph/tree" + "github.com/kiali/kiali/kubernetes/kubetest" + "github.com/kiali/kiali/prometheus/prometheustest" + "github.com/kiali/kiali/services/business" +) + +func TestDeadService(t *testing.T) { + assert := assert.New(t) + k8s := new(kubetest.K8SClientMock) + prom := new(prometheustest.PromClientMock) + business.SetWithBackends(k8s, prom) + + k8s.On("GetService", mock.AnythingOfType("string"), "testPodsWithTraffic").Return( + &v1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "foo", + }, + }, nil) + k8s.On("GetServicePods", mock.AnythingOfType("string"), "testPodsWithTraffic", "v1", "").Return( + &v1.PodList{ + Items: []v1.Pod{v1.Pod{ + Status: v1.PodStatus{ + Message: "foo", + }}, + }, + }, nil) + + k8s.On("GetService", mock.AnythingOfType("string"), "testPodsNoTraffic").Return( + &v1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "foo", + }, + }, nil) + k8s.On("GetServicePods", mock.AnythingOfType("string"), "testPodsNoTraffic", "v1", "").Return( + &v1.PodList{ + Items: []v1.Pod{v1.Pod{ + Status: v1.PodStatus{ + Message: "foo", + }}, + }, + }, nil) + + k8s.On("GetService", mock.AnythingOfType("string"), "testNoPodsWithTraffic").Return( + &v1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "foo", + }, + }, nil) + k8s.On("GetServicePods", mock.AnythingOfType("string"), "testNoPodsWithTraffic", "v1", "").Return( + &v1.PodList{ + Items: []v1.Pod{}, + }, nil) + + k8s.On("GetService", mock.AnythingOfType("string"), "testNoPodsNoTraffic").Return( + &v1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "foo", + }, + }, nil) + k8s.On("GetServicePods", mock.AnythingOfType("string"), "testNoPodsNoTraffic", "v1", "").Return( + &v1.PodList{ + Items: []v1.Pod{}, + }, nil) + + k8s.On("GetService", mock.AnythingOfType("string"), "testNoServiceWithTraffic").Return((*v1.Service)(nil), nil) + k8s.On("GetService", mock.AnythingOfType("string"), "testNoServiceNoTraffic").Return((*v1.Service)(nil), nil) + + config.Set(config.NewConfig()) + + trees := testTree() + + assert.Equal(1, len(trees)) + assert.Equal(tree.UnknownService, trees[0].Name) + assert.Equal(6, len(trees[0].Children)) + + applyDeadServices(&trees[0], "testNamespace", k8s) + + assert.Equal(1, len(trees)) + assert.Equal(tree.UnknownService, trees[0].Name) + assert.Equal(5, len(trees[0].Children)) + + assert.Equal("testPodsWithTraffic.testNamespace.svc.cluster.local", trees[0].Children[0].Name) + assert.Equal("testPodsNoTraffic.testNamespace.svc.cluster.local", trees[0].Children[1].Name) + assert.Equal("testNoPodsWithTraffic.testNamespace.svc.cluster.local", trees[0].Children[2].Name) + assert.Equal("testNoPodsNoTraffic.testNamespace.svc.cluster.local", trees[0].Children[3].Name) + isDead, ok := trees[0].Children[3].Metadata["isDead"] + assert.Equal(true, ok) + assert.Equal("true", isDead) + assert.Equal("testNoServiceWithTraffic.testNamespace.svc.cluster.local", trees[0].Children[4].Name) +} + +func testTree() []tree.ServiceNode { + trees := make([]tree.ServiceNode, 1) + + trees[0] = tree.NewServiceNode(tree.UnknownService, tree.UnknownVersion) + trees[0].Children = make([]*tree.ServiceNode, 6) + + child0 := tree.NewServiceNode("testPodsWithTraffic.testNamespace.svc.cluster.local", "v1") + child0.Metadata = make(map[string]interface{}) + child0.Metadata["rate"] = 0.8 + trees[0].Children[0] = &child0 + + child1 := tree.NewServiceNode("testPodsNoTraffic.testNamespace.svc.cluster.local", "v1") + child1.Metadata = make(map[string]interface{}) + child1.Metadata["rate"] = 0.0 + trees[0].Children[1] = &child1 + + child2 := tree.NewServiceNode("testNoPodsWithTraffic.testNamespace.svc.cluster.local", "v1") + child2.Metadata = make(map[string]interface{}) + child2.Metadata["rate"] = 0.8 + trees[0].Children[2] = &child2 + + child3 := tree.NewServiceNode("testNoPodsNoTraffic.testNamespace.svc.cluster.local", "v1") + child3.Metadata = make(map[string]interface{}) + child3.Metadata["rate"] = 0.0 + trees[0].Children[3] = &child3 + + child4 := tree.NewServiceNode("testNoServiceWithTraffic.testNamespace.svc.cluster.local", "v1") + child4.Metadata = make(map[string]interface{}) + child4.Metadata["rate"] = 0.8 + trees[0].Children[4] = &child4 + + child5 := tree.NewServiceNode("testNoServiceNoTraffic.testNamespace.svc.cluster.local", "v1") + child5.Metadata = make(map[string]interface{}) + child5.Metadata["rate"] = 0.0 + trees[0].Children[5] = &child5 + + return trees +} diff --git a/graph/cytoscape/cytoscape.go b/graph/cytoscape/cytoscape.go index 2dda7c2da8..df1a99a8aa 100644 --- a/graph/cytoscape/cytoscape.go +++ b/graph/cytoscape/cytoscape.go @@ -39,6 +39,7 @@ type NodeData struct { RateSelfInvoke string `json:"rateSelfInvoke,omitempty"` // rate of selfInvoke HasCircuitBreaker string `json:"hasCB,omitempty"` // true | false | unknown HasRouteRule string `json:"hasRR,omitempty"` // true | false | unknown + IsDead string `json:"isDead,omitempty"` // true (has no pods) | false IsGroup string `json:"isGroup,omitempty"` // set to the grouping type, current values: [ 'version' ] IsRoot string `json:"isRoot,omitempty"` // true | false IsUnused string `json:"isUnused,omitempty"` // true | false @@ -147,6 +148,11 @@ func walk(sn *tree.ServiceNode, nodes *[]*NodeWrapper, edges *[]*EdgeWrapper, pa Version: sn.Version, } + // node may be dead (service defined but no pods running) + if dead, ok := sn.Metadata["isDead"]; ok { + nd.IsDead = dead.(string) + } + // node may be a root if root, ok := sn.Metadata["isRoot"]; ok { nd.IsRoot = root.(string) diff --git a/graph/options/options.go b/graph/options/options.go index dc8dce5032..7d44df1a2a 100644 --- a/graph/options/options.go +++ b/graph/options/options.go @@ -87,6 +87,9 @@ func parseAppenders(params url.Values) []appender.Appender { csl = strings.ToLower(params.Get("appenders")) } + if csl == all || strings.Contains(csl, "dead_service") { + appenders = append(appenders, appender.DeadServiceAppender{}) + } if csl == all || strings.Contains(csl, "unused_service") { appenders = append(appenders, appender.UnusedServiceAppender{}) } diff --git a/handlers/graph.go b/handlers/graph.go index e0548f9d07..72d7cb3738 100644 --- a/handlers/graph.go +++ b/handlers/graph.go @@ -36,6 +36,7 @@ import ( "errors" "fmt" "net/http" + "runtime/debug" "sort" "strings" "time" @@ -429,7 +430,7 @@ func handlePanic(w http.ResponseWriter) { default: message = fmt.Sprintf("%v", r) } - log.Errorf("Error: %v\n", message) + log.Errorf("%s: %s", message, debug.Stack()) RespondWithError(w, http.StatusInternalServerError, message) } } diff --git a/kubernetes/client.go b/kubernetes/client.go index a2149d05b6..75ee8dc0df 100644 --- a/kubernetes/client.go +++ b/kubernetes/client.go @@ -33,8 +33,10 @@ var ( // IstioClientInterface for mocks (only mocked function are necessary here) type IstioClientInterface interface { + GetService(namespace string, serviceName string) (*v1.Service, error) GetServices(namespaceName string) (*ServiceList, error) GetServiceDetails(namespace string, serviceName string) (*ServiceDetails, error) + GetServicePods(namespace string, serviceName string, serviceVersion string, selector string) (*v1.PodList, error) GetIstioDetails(namespace string, serviceName string) (*IstioDetails, error) } diff --git a/kubernetes/kubetest/mock.go b/kubernetes/kubetest/mock.go index 3ba1020417..0ac476075c 100644 --- a/kubernetes/kubetest/mock.go +++ b/kubernetes/kubetest/mock.go @@ -2,6 +2,7 @@ package kubetest import ( "github.com/stretchr/testify/mock" + "k8s.io/api/core/v1" "github.com/kiali/kiali/kubernetes" ) @@ -10,8 +11,13 @@ type K8SClientMock struct { mock.Mock } -func (o *K8SClientMock) GetServices(namespaceName string) (*kubernetes.ServiceList, error) { - args := o.Called(namespaceName) +func (o *K8SClientMock) GetService(namespace string, serviceName string) (*v1.Service, error) { + args := o.Called(namespace, serviceName) + return args.Get(0).(*v1.Service), args.Error(1) +} + +func (o *K8SClientMock) GetServices(namespace string) (*kubernetes.ServiceList, error) { + args := o.Called(namespace) return args.Get(0).(*kubernetes.ServiceList), args.Error(1) } @@ -20,6 +26,11 @@ func (o *K8SClientMock) GetServiceDetails(namespace string, serviceName string) return args.Get(0).(*kubernetes.ServiceDetails), args.Error(1) } +func (o *K8SClientMock) GetServicePods(namespace string, serviceName string, serviceVersion string, selector string) (*v1.PodList, error) { + args := o.Called(namespace, serviceName, serviceVersion, selector) + return args.Get(0).(*v1.PodList), args.Error(1) +} + func (o *K8SClientMock) GetIstioDetails(namespace string, serviceName string) (*kubernetes.IstioDetails, error) { args := o.Called(namespace, serviceName) return args.Get(0).(*kubernetes.IstioDetails), args.Error(1)