Skip to content

Commit

Permalink
KIALI-1309 Handle label misconfiguration on workloads
Browse files Browse the repository at this point in the history 
- also, fix an issue with service vs workload nodes in app graph
  • Loading branch information
@jshaughn
jshaughn committed Aug 9, 2018
1 parent da740f1 commit 1835460
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 29 deletions.
46 changes: 26 additions & 20 deletions graph/cytoscape/cytoscape.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,27 @@ type NodeData struct {
Parent string `json:"parent,omitempty"` // Compound Node parent ID

// App Fields (not required by Cytoscape)
NodeType string `json:"nodeType"`
Namespace string `json:"namespace"`
Workload string `json:"workload,omitempty"`
App string `json:"app,omitempty"`
Version string `json:"version,omitempty"`
Service string `json:"service,omitempty"` // requested service for NodeTypeService
DestServices map[string]bool `json:"destServices,omitempty"` // requested services for [dest] node
Rate string `json:"rate,omitempty"` // edge aggregate
Rate3xx string `json:"rate3XX,omitempty"` // edge aggregate
Rate4xx string `json:"rate4XX,omitempty"` // edge aggregate
Rate5xx string `json:"rate5XX,omitempty"` // edge aggregate
RateOut string `json:"rateOut,omitempty"` // edge aggregate
HasCB bool `json:"hasCB,omitempty"` // true (has circuit breaker) | false
HasMissingSC bool `json:"hasMissingSC,omitempty"` // true (has missing sidecar) | false
HasVS bool `json:"hasVS,omitempty"` // true (has route rule) | false
IsDead bool `json:"isDead,omitempty"` // true (has no pods) | false
IsGroup string `json:"isGroup,omitempty"` // set to the grouping type, current values: [ 'version' ]
IsOutside bool `json:"isOutside,omitempty"` // true | false
IsRoot bool `json:"isRoot,omitempty"` // true | false
IsUnused bool `json:"isUnused,omitempty"` // true | false
NodeType string `json:"nodeType"`
Namespace string `json:"namespace"`
Workload string `json:"workload,omitempty"`
App string `json:"app,omitempty"`
Version string `json:"version,omitempty"`
Service string `json:"service,omitempty"` // requested service for NodeTypeService
DestServices map[string]bool `json:"destServices,omitempty"` // requested services for [dest] node
Rate string `json:"rate,omitempty"` // edge aggregate
Rate3xx string `json:"rate3XX,omitempty"` // edge aggregate
Rate4xx string `json:"rate4XX,omitempty"` // edge aggregate
Rate5xx string `json:"rate5XX,omitempty"` // edge aggregate
RateOut string `json:"rateOut,omitempty"` // edge aggregate
HasCB bool `json:"hasCB,omitempty"` // true (has circuit breaker) | false
HasMissingSC bool `json:"hasMissingSC,omitempty"` // true (has missing sidecar) | false
HasVS bool `json:"hasVS,omitempty"` // true (has route rule) | false
IsDead bool `json:"isDead,omitempty"` // true (has no pods) | false
IsGroup string `json:"isGroup,omitempty"` // set to the grouping type, current values: [ 'version' ]
IsMisconfigured string `json:"isMisconfigured,omitempty"` // set to misconfiguration list, current values: [ 'labels' ]
IsOutside bool `json:"isOutside,omitempty"` // true | false
IsRoot bool `json:"isRoot,omitempty"` // true | false
IsUnused bool `json:"isUnused,omitempty"` // true | false
}

type EdgeData struct {
Expand Down Expand Up @@ -189,6 +190,11 @@ func buildConfig(trafficMap graph.TrafficMap, nodes *[]*NodeWrapper, edges *[]*E
nd.HasMissingSC = val.(bool)
}

// check if node is misconfigured
if val, ok := n.Metadata["isMisconfigured"]; ok {
nd.IsMisconfigured = val.(string)
}

// check if node is on another namespace
if val, ok := n.Metadata["isOutside"]; ok {
nd.IsOutside = val.(bool)
Expand Down
10 changes: 5 additions & 5 deletions graph/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,11 @@ func Id(namespace, workload, app, version, service, graphType string) (id, nodeT
}
}

// fall back to service if applicable
if serviceOk {
return fmt.Sprintf("svc_%v_%v", namespace, service), NodeTypeService
// fall back to workload if applicable
if workloadOk {
return fmt.Sprintf("wl_%v_%v", namespace, workload), NodeTypeWorkload
}

// fall back to workload as a last resort in the app graph
return fmt.Sprintf("wl_%v_%v", namespace, workload), NodeTypeWorkload
// fall back to service as a last resort in the app graph
return return fmt.Sprintf("svc_%v_%v", namespace, service), NodeTypeService
}
41 changes: 37 additions & 4 deletions handlers/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ func populateTrafficMap(trafficMap graph.TrafficMap, vector *model.Vector, o opt
code := string(lCode)
csp := string(lCsp)

source, _ := addSourceNode(trafficMap, sourceWlNs, sourceWl, sourceApp, sourceVer, o)
dest, _ := addDestNode(trafficMap, destSvcNs, destWl, destApp, destVer, destSvcName, o)
source, sourceFound := addSourceNode(trafficMap, sourceWlNs, sourceWl, sourceApp, sourceVer, o)
dest, destFound := addDestNode(trafficMap, destSvcNs, destWl, destApp, destVer, destSvcName, o)

addToDestServices(dest.Metadata, destSvcName)

Expand All @@ -258,6 +258,16 @@ func populateTrafficMap(trafficMap graph.TrafficMap, vector *model.Vector, o opt
}

val := float64(s.Value)

// A workload may mistakenly have multiple app and or version label values.
// This is a misconfiguration we need to handle. See Kiali-1309.
if sourceFound {
handleMisconfiguredLabels(source, sourceApp, sourceVer, val, o)
}
if destFound {
handleMisconfiguredLabels(dest, destApp, destVer, val, o)
}

var ck string
switch {
case strings.HasPrefix(string(code), "2"):
Expand Down Expand Up @@ -300,6 +310,29 @@ func addToDestServices(md map[string]interface{}, destService string) {
destServices.(map[string]bool)[destService] = true
}

func handleMisconfiguredLabels(node *graph.Node, app, version string, rate float64, o options.Options) {
isVersionedAppGraph := o.VendorOptions.GraphType == graph.GraphTypeVersionedApp
isWorkloadNode := node.NodeType == graph.NodeTypeWorkload
isVersionedAppNode := node.NodeType == graph.NodeTypeApp && isVersionedAppGraph
if isWorkloadNode || isVersionedAppNode {
labels := []string{}
if node.App != app {
labels = append(labels, "app")
}
if node.Version != version {
labels = append(labels, "version")
}
// prefer the labels of an active time series as often the other labels are inactive
if len(labels) > 0 {
node.Metadata["isMisconfigured"] = fmt.Sprintf("labels=%v", labels)
if rate > 0.0 {
node.App = app
node.Version = version
}
}
}
}

func addSourceNode(trafficMap graph.TrafficMap, namespace, workload, app, version string, o options.Options) (*graph.Node, bool) {
id, nodeType := graph.Id(namespace, workload, app, version, "", o.VendorOptions.GraphType)
node, found := trafficMap[id]
Expand All @@ -308,7 +341,7 @@ func addSourceNode(trafficMap graph.TrafficMap, namespace, workload, app, versio
node = &newNode
trafficMap[id] = node
}
return node, !found
return node, found
}

func addDestNode(trafficMap graph.TrafficMap, namespace, workload, app, version, service string, o options.Options) (*graph.Node, bool) {
Expand All @@ -319,7 +352,7 @@ func addDestNode(trafficMap graph.TrafficMap, namespace, workload, app, version,
node = &newNode
trafficMap[id] = node
}
return node, !found
return node, found
}

// mergeTrafficMaps ensures that we only have unique nodes by removing duplicate
Expand Down

0 comments on commit 1835460

Please sign in to comment.