Browse code

oc: update route warnings for oc status

kargakis authored on 2016/03/07 23:17:41
Showing 6 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,33 @@
0
+apiVersion: v1
1
+items:
2
+- apiVersion: v1
3
+  kind: Service
4
+  metadata:
5
+    labels:
6
+      template: application-template-stibuild
7
+    name: frontend
8
+  spec:
9
+    clusterIP: 172.30.204.42
10
+    portalIP: 172.30.204.42
11
+    ports:
12
+    - name: web
13
+      port: 5432
14
+      protocol: TCP
15
+      targetPort: 8080
16
+    selector:
17
+      name: frontend
18
+    sessionAffinity: None
19
+    type: ClusterIP
20
+- apiVersion: v1
21
+  kind: Route
22
+  metadata:
23
+    name: route-edge
24
+  spec:
25
+    host: www.example.com
26
+    port:
27
+      targetPort: "http"
28
+    to:
29
+      kind: Service
30
+      name: frontend
31
+kind: List
32
+metadata: {}
0 33
new file mode 100644
... ...
@@ -0,0 +1,33 @@
0
+apiVersion: v1
1
+items:
2
+- apiVersion: v1
3
+  kind: Service
4
+  metadata:
5
+    labels:
6
+      template: application-template-stibuild
7
+    name: frontend
8
+  spec:
9
+    clusterIP: 172.30.204.42
10
+    portalIP: 172.30.204.42
11
+    ports:
12
+    - name: web
13
+      port: 5432
14
+      protocol: TCP
15
+      targetPort: 8080
16
+    selector:
17
+      name: frontend
18
+    sessionAffinity: None
19
+    type: ClusterIP
20
+- apiVersion: v1
21
+  kind: Route
22
+  metadata:
23
+    name: route-edge
24
+  spec:
25
+    host: www.example.com
26
+    port:
27
+      targetPort: 8081
28
+    to:
29
+      kind: Service
30
+      name: frontend
31
+kind: List
32
+metadata: {}
... ...
@@ -343,7 +343,7 @@ func getMarkerScanners(logsCommandName, securityPolicyCommandFormat, setProbeCom
343 343
 		func(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
344 344
 			return deployanalysis.FindDeploymentConfigReadinessWarnings(g, f, setProbeCommandName)
345 345
 		},
346
-		routeanalysis.FindMissingPortMapping,
346
+		routeanalysis.FindPortMappingIssues,
347 347
 		routeanalysis.FindMissingTLSTerminationType,
348 348
 		routeanalysis.FindPathBasedPassthroughRoutes,
349 349
 		routeanalysis.FindRouteAdmissionFailures,
... ...
@@ -2,6 +2,7 @@ package generator
2 2
 
3 3
 import (
4 4
 	"fmt"
5
+	"strconv"
5 6
 
6 7
 	kapi "k8s.io/kubernetes/pkg/api"
7 8
 	"k8s.io/kubernetes/pkg/kubectl"
... ...
@@ -77,8 +78,14 @@ func (RouteGenerator) Generate(genericParams map[string]interface{}) (runtime.Ob
77 77
 
78 78
 	portString := params["target-port"]
79 79
 	if len(portString) > 0 {
80
+		var targetPort intstr.IntOrString
81
+		if port, err := strconv.Atoi(portString); err == nil {
82
+			targetPort = intstr.FromInt(port)
83
+		} else {
84
+			targetPort = intstr.FromString(portString)
85
+		}
80 86
 		route.Spec.Port = &api.RoutePort{
81
-			TargetPort: intstr.FromString(portString),
87
+			TargetPort: targetPort,
82 88
 		}
83 89
 	}
84 90
 
... ...
@@ -2,6 +2,7 @@ package analysis
2 2
 
3 3
 import (
4 4
 	"fmt"
5
+	"strconv"
5 6
 
6 7
 	"github.com/gonum/graph"
7 8
 
... ...
@@ -18,6 +19,10 @@ const (
18 18
 	// MissingRoutePortWarning is returned when a route has no route port specified
19 19
 	// and the service it routes to has multiple ports.
20 20
 	MissingRoutePortWarning = "MissingRoutePort"
21
+	// WrongRoutePortWarning is returned when a route has a route port specified
22
+	// but the service it points to has no such port (either as a named port or as
23
+	// a target port).
24
+	WrongRoutePortWarning = "WrongRoutePort"
21 25
 	// MissingServiceWarning is returned when there is no service for the specific route.
22 26
 	MissingServiceWarning = "MissingService"
23 27
 	// MissingTLSTerminationTypeErr is returned when a route with a tls config doesn't
... ...
@@ -31,49 +36,105 @@ const (
31 31
 	RouteNotAdmittedTypeErr = "RouteNotAdmitted"
32 32
 )
33 33
 
34
-// FindMissingPortMapping checks all routes and reports those that don't specify a port while
35
-// the service they are routing to, has multiple ports. Also if a service for a route doesn't
36
-// exist, will be reported.
37
-func FindMissingPortMapping(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
34
+// FindPortMappingIssues checks all routes and reports any issues related to their ports.
35
+// Also non-existent services for routes are reported here.
36
+func FindPortMappingIssues(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
38 37
 	markers := []osgraph.Marker{}
39 38
 
40
-route:
41 39
 	for _, uncastRouteNode := range g.NodesByKind(routegraph.RouteNodeKind) {
42
-		for _, uncastServiceNode := range g.SuccessorNodesByEdgeKind(uncastRouteNode, routeedges.ExposedThroughRouteEdgeKind) {
43
-			routeNode := uncastRouteNode.(*routegraph.RouteNode)
44
-			svcNode := uncastServiceNode.(*kubegraph.ServiceNode)
40
+		routeNode := uncastRouteNode.(*routegraph.RouteNode)
41
+		marker := routePortMarker(g, f, routeNode)
42
+		if marker != nil {
43
+			markers = append(markers, *marker)
44
+		}
45
+	}
45 46
 
46
-			if !svcNode.Found() {
47
-				markers = append(markers, osgraph.Marker{
48
-					Node:         routeNode,
49
-					RelatedNodes: []graph.Node{svcNode},
47
+	return markers
48
+}
50 49
 
51
-					Severity: osgraph.WarningSeverity,
52
-					Key:      MissingServiceWarning,
53
-					Message: fmt.Sprintf("%s is supposed to route traffic to %s but %s doesn't exist.",
54
-						f.ResourceName(routeNode), f.ResourceName(svcNode), f.ResourceName(svcNode)),
55
-				})
50
+func routePortMarker(g osgraph.Graph, f osgraph.Namer, routeNode *routegraph.RouteNode) *osgraph.Marker {
51
+	for _, uncastServiceNode := range g.SuccessorNodesByEdgeKind(routeNode, routeedges.ExposedThroughRouteEdgeKind) {
52
+		svcNode := uncastServiceNode.(*kubegraph.ServiceNode)
53
+
54
+		if !svcNode.Found() {
55
+			return &osgraph.Marker{
56
+				Node:         routeNode,
57
+				RelatedNodes: []graph.Node{svcNode},
58
+
59
+				Severity: osgraph.WarningSeverity,
60
+				Key:      MissingServiceWarning,
61
+				Message: fmt.Sprintf("%s is supposed to route traffic to %s but %s doesn't exist.",
62
+					f.ResourceName(routeNode), f.ResourceName(svcNode), f.ResourceName(svcNode)),
63
+				// TODO: Suggest 'oc create service' once that's a thing.
64
+				// See https://github.com/kubernetes/kubernetes/pull/19509
65
+			}
66
+		}
56 67
 
57
-				continue route
68
+		if len(svcNode.Spec.Ports) > 1 && (routeNode.Spec.Port == nil || len(routeNode.Spec.Port.TargetPort.String()) == 0) {
69
+			return &osgraph.Marker{
70
+				Node:         routeNode,
71
+				RelatedNodes: []graph.Node{svcNode},
72
+
73
+				Severity: osgraph.WarningSeverity,
74
+				Key:      MissingRoutePortWarning,
75
+				Message: fmt.Sprintf("%s doesn't have a port specified and is routing traffic to %s which uses multiple ports.",
76
+					f.ResourceName(routeNode), f.ResourceName(svcNode)),
58 77
 			}
78
+		}
59 79
 
60
-			if len(svcNode.Spec.Ports) > 1 && (routeNode.Spec.Port == nil || len(routeNode.Spec.Port.TargetPort.String()) == 0) {
61
-				markers = append(markers, osgraph.Marker{
62
-					Node:         routeNode,
63
-					RelatedNodes: []graph.Node{svcNode},
80
+		if routeNode.Spec.Port == nil {
81
+			// If no port is specified, we don't need to analyze any further.
82
+			return nil
83
+		}
64 84
 
65
-					Severity: osgraph.WarningSeverity,
66
-					Key:      MissingRoutePortWarning,
67
-					Message: fmt.Sprintf("%s doesn't have a port specified and is routing traffic to %s which uses multiple ports.",
68
-						f.ResourceName(routeNode), f.ResourceName(svcNode)),
69
-				})
85
+		routePortString := routeNode.Spec.Port.TargetPort.String()
86
+		if routePort, err := strconv.Atoi(routePortString); err == nil {
87
+			for _, port := range svcNode.Spec.Ports {
88
+				if port.TargetPort.IntValue() == routePort {
89
+					return nil
90
+				}
91
+			}
92
+
93
+			// route has a numeric port, service has no port with that number as a targetPort.
94
+			marker := &osgraph.Marker{
95
+				Node:         routeNode,
96
+				RelatedNodes: []graph.Node{svcNode},
70 97
 
71
-				continue route
98
+				Severity: osgraph.WarningSeverity,
99
+				Key:      WrongRoutePortWarning,
100
+				Message: fmt.Sprintf("%s has a port specified (%d) but %s has no such targetPort.",
101
+					f.ResourceName(routeNode), routePort, f.ResourceName(svcNode)),
72 102
 			}
103
+			if len(svcNode.Spec.Ports) == 1 {
104
+				marker.Suggestion = osgraph.Suggestion(fmt.Sprintf("oc patch %s -p '{\"spec\":{\"port\":{\"targetPort\": %d}}}'", f.ResourceName(routeNode), svcNode.Spec.Ports[0].TargetPort.IntValue()))
105
+			}
106
+
107
+			return marker
73 108
 		}
74
-	}
75 109
 
76
-	return markers
110
+		for _, port := range svcNode.Spec.Ports {
111
+			if port.Name == routePortString {
112
+				return nil
113
+			}
114
+		}
115
+
116
+		// route has a named port, service has no port with that name.
117
+		marker := &osgraph.Marker{
118
+			Node:         routeNode,
119
+			RelatedNodes: []graph.Node{svcNode},
120
+
121
+			Severity: osgraph.WarningSeverity,
122
+			Key:      WrongRoutePortWarning,
123
+			Message: fmt.Sprintf("%s has a named port specified (%q) but %s has no such named port.",
124
+				f.ResourceName(routeNode), routePortString, f.ResourceName(svcNode)),
125
+		}
126
+		if len(svcNode.Spec.Ports) == 1 {
127
+			marker.Suggestion = osgraph.Suggestion(fmt.Sprintf("oc patch %s -p '{\"spec\":{\"port\":{\"targetPort\": %d}}}'", f.ResourceName(routeNode), svcNode.Spec.Ports[0].TargetPort.IntValue()))
128
+		}
129
+
130
+		return marker
131
+	}
132
+	return nil
77 133
 }
78 134
 
79 135
 func FindMissingTLSTerminationType(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
... ...
@@ -8,7 +8,7 @@ import (
8 8
 	routeedges "github.com/openshift/origin/pkg/route/graph"
9 9
 )
10 10
 
11
-func TestMissingPortMapping(t *testing.T) {
11
+func TestPortMappingIssues(t *testing.T) {
12 12
 	// Multiple service ports - no route port specified
13 13
 	g, _, err := osgraphtest.BuildGraph("../../../api/graph/test/missing-route-port.yaml")
14 14
 	if err != nil {
... ...
@@ -16,7 +16,7 @@ func TestMissingPortMapping(t *testing.T) {
16 16
 	}
17 17
 	routeedges.AddAllRouteEdges(g)
18 18
 
19
-	markers := FindMissingPortMapping(g, osgraph.DefaultNamer)
19
+	markers := FindPortMappingIssues(g, osgraph.DefaultNamer)
20 20
 	if expected, got := 1, len(markers); expected != got {
21 21
 		t.Fatalf("expected %d markers, got %d", expected, got)
22 22
 	}
... ...
@@ -31,13 +31,43 @@ func TestMissingPortMapping(t *testing.T) {
31 31
 	}
32 32
 	routeedges.AddAllRouteEdges(g)
33 33
 
34
-	markers = FindMissingPortMapping(g, osgraph.DefaultNamer)
34
+	markers = FindPortMappingIssues(g, osgraph.DefaultNamer)
35 35
 	if expected, got := 1, len(markers); expected != got {
36 36
 		t.Fatalf("expected %d markers, got %d", expected, got)
37 37
 	}
38 38
 	if expected, got := MissingServiceWarning, markers[0].Key; expected != got {
39 39
 		t.Fatalf("expected %s marker key, got %s", expected, got)
40 40
 	}
41
+
42
+	// Wrong named route port
43
+	g, _, err = osgraphtest.BuildGraph("../../../api/graph/test/wrong-numeric-port.yaml")
44
+	if err != nil {
45
+		t.Fatalf("unexpected error: %v", err)
46
+	}
47
+	routeedges.AddAllRouteEdges(g)
48
+
49
+	markers = FindPortMappingIssues(g, osgraph.DefaultNamer)
50
+	if expected, got := 1, len(markers); expected != got {
51
+		t.Fatalf("expected %d markers, got %d", expected, got)
52
+	}
53
+	if expected, got := WrongRoutePortWarning, markers[0].Key; expected != got {
54
+		t.Fatalf("expected %s marker key, got %s", expected, got)
55
+	}
56
+
57
+	// Wrong numeric route port
58
+	g, _, err = osgraphtest.BuildGraph("../../../api/graph/test/wrong-named-port.yaml")
59
+	if err != nil {
60
+		t.Fatalf("unexpected error: %v", err)
61
+	}
62
+	routeedges.AddAllRouteEdges(g)
63
+
64
+	markers = FindPortMappingIssues(g, osgraph.DefaultNamer)
65
+	if expected, got := 1, len(markers); expected != got {
66
+		t.Fatalf("expected %d markers, got %d", expected, got)
67
+	}
68
+	if expected, got := WrongRoutePortWarning, markers[0].Key; expected != got {
69
+		t.Fatalf("expected %s marker key, got %s", expected, got)
70
+	}
41 71
 }
42 72
 
43 73
 func TestPathBasedPassthroughRoutes(t *testing.T) {