Skip to content

Commit c31f71a

Browse files
aattuluriaattuluri
authored
Missing name in service port (#15)
* Handle missing name on service port * Undo un-needed change
1 parent 6a08f21 commit c31f71a

File tree

4 files changed

+142
-35
lines changed

4 files changed

+142
-35
lines changed

‎Makefile‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,5 @@ gen-yaml:
102102
kustomize build ./install/admiralremote/base/ > ./out/yaml/remotecluster.yaml
103103
kustomize build ./install/sample/base/ > ./out/yaml/sample.yaml
104104
cp ./install/sample/sample_dep.yaml ./out/yaml/sample_dep.yaml
105-
cp ./test/scripts/cluster-secret.sh ./out/scripts/cluster-secret.sh
105+
cp ./install/scripts/cluster-secret.sh ./out/scripts/cluster-secret.sh
106+
cp ./install/scripts/cluster-secret.sh ./out/scripts/cluster-secret.sh

‎admiral/pkg/clusters/registry.go‎

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ package clusters
22

33
import (
44
"context"
5-
"github.com/gogo/protobuf/proto"
65
"github.com/admiral/admiral/pkg/apis/admiral/v1"
76
"github.com/admiral/admiral/pkg/controller/admiral"
87
"github.com/admiral/admiral/pkg/controller/common"
98
"github.com/admiral/admiral/pkg/controller/istio"
109
"github.com/admiral/admiral/pkg/controller/secret"
1110
"github.com/admiral/admiral/pkg/controller/util"
11+
"github.com/gogo/protobuf/proto"
1212
"k8s.io/client-go/rest"
13-
"strconv"
1413
"strings"
1514
"time"
1615

@@ -20,8 +19,8 @@ import (
2019
"sync"
2120

2221
istioModel "istio.io/istio/pilot/pkg/model"
23-
k8sV1 "k8s.io/api/core/v1"
2422
k8sAppsV1 "k8s.io/api/apps/v1"
23+
k8sV1 "k8s.io/api/core/v1"
2524
k8s "k8s.io/client-go/kubernetes"
2625

2726
networking "istio.io/api/networking/v1alpha3"
@@ -476,7 +475,7 @@ func createServiceEntryForNewServiceOrPod(namespace string, sourceIdentity strin
476475
for sourceCluster, serviceInstance := range sourceServices {
477476
localFqdn := serviceInstance.Name + common.Sep + serviceInstance.Namespace + common.DotLocalDomainSuffix
478477
rc := remoteRegistry.remoteControllers[sourceCluster]
479-
var meshPorts = getMeshPorts(sourceCluster, serviceInstance, sourceDeployments[sourceCluster])
478+
var meshPorts = GetMeshPorts(sourceCluster, serviceInstance, sourceDeployments[sourceCluster])
480479
for key, serviceEntry := range serviceEntries {
481480
for _, ep := range serviceEntry.Endpoints {
482481
clusterIngress := rc.ServiceController.Cache.GetLoadBalancer(admiral.IstioIngressServiceName, common.NamespaceIstioSystem)
@@ -511,6 +510,7 @@ func createServiceEntryForNewServiceOrPod(namespace string, sourceIdentity strin
511510
}
512511
}
513512

513+
514514
func getServiceForDeployment(rc *RemoteController, deployment *k8sAppsV1.Deployment, namespace string) *k8sV1.Service {
515515

516516
cachedService := rc.ServiceController.Cache.Get(namespace)
@@ -530,7 +530,7 @@ func getServiceForDeployment(rc *RemoteController, deployment *k8sAppsV1.Deploym
530530
}
531531
//make sure the service matches the deployment Selector and also has a mesh port in the port spec
532532
if match {
533-
ports := getMeshPorts(rc.ClusterID, service, deployment)
533+
ports := GetMeshPorts(rc.ClusterID, service, deployment)
534534
if len(ports) > 0 {
535535
matchedService = service
536536
break
@@ -540,35 +540,6 @@ func getServiceForDeployment(rc *RemoteController, deployment *k8sAppsV1.Deploym
540540
return matchedService
541541
}
542542

543-
func getMeshPorts(clusterName string, destService *k8sV1.Service,
544-
destDeployment *k8sAppsV1.Deployment) map[string]uint32 {
545-
var ports = make(map[string]uint32)
546-
var meshPorts = destDeployment.Spec.Template.Annotations[common.SidecarEnabledPorts]
547-
if len(meshPorts) == 0 {
548-
log.Infof(LogFormat, "GetMeshPorts", "service", destService.Name, clusterName, "No mesh ports present, defaulting to first port")
549-
//TODO default to 8090 instead if the first port?
550-
ports[destService.Spec.Ports[0].Name] = uint32(destService.Spec.Ports[0].Port)
551-
return ports
552-
}
553-
554-
meshPortsSplit := strings.Split(meshPorts, ",")
555-
var meshPortMap = make(map[uint32]uint32)
556-
for _, meshPort := range meshPortsSplit {
557-
port, err := strconv.ParseUint(meshPort, 10, 32)
558-
if err != nil {
559-
continue
560-
}
561-
meshPortMap[uint32(port)] = uint32(port)
562-
}
563-
for _, servicePort := range destService.Spec.Ports {
564-
if _, ok := meshPortMap[uint32(servicePort.Port)]; ok {
565-
log.Debugf(LogFormat, "GetMeshPorts", servicePort.Port, destService.Name, clusterName, "Adding mesh port")
566-
ports[common.Http] = uint32(servicePort.Port)
567-
}
568-
}
569-
return ports
570-
}
571-
572543
func getDependentClusters(dependents *common.Map, identityClusterCache *common.MapOfMaps, sourceServices map[string]*k8sV1.Service) map[string]string {
573544
var dependentClusters = make(map[string]string)
574545
//TODO optimize this map construction

‎admiral/pkg/clusters/util.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package clusters
2+
3+
import (
4+
"github.com/admiral/admiral/pkg/controller/common"
5+
"istio.io/istio/pkg/log"
6+
"strconv"
7+
"strings"
8+
9+
k8sV1 "k8s.io/api/core/v1"
10+
k8sAppsV1 "k8s.io/api/apps/v1"
11+
)
12+
13+
func GetMeshPorts(clusterName string, destService *k8sV1.Service,
14+
destDeployment *k8sAppsV1.Deployment) map[string]uint32 {
15+
var ports = make(map[string]uint32)
16+
var meshPorts = destDeployment.Spec.Template.Annotations[common.SidecarEnabledPorts]
17+
if len(meshPorts) == 0 {
18+
log.Infof(LogFormat, "GetMeshPorts", "service", destService.Name, clusterName, "No mesh ports present, defaulting to first port")
19+
if destService.Spec.Ports != nil && len(destService.Spec.Ports) > 0 {
20+
var name = destService.Spec.Ports[0].Name
21+
if len(name) == 0 {
22+
name = common.Http
23+
}
24+
ports[name] = uint32(destService.Spec.Ports[0].Port)
25+
}
26+
return ports
27+
}
28+
29+
meshPortsSplit := strings.Split(meshPorts, ",")
30+
var meshPortMap = make(map[uint32]uint32)
31+
for _, meshPort := range meshPortsSplit {
32+
port, err := strconv.ParseUint(meshPort, 10, 32)
33+
if err != nil {
34+
continue
35+
}
36+
meshPortMap[uint32(port)] = uint32(port)
37+
}
38+
for _, servicePort := range destService.Spec.Ports {
39+
if _, ok := meshPortMap[uint32(servicePort.Port)]; ok {
40+
log.Debugf(LogFormat, "GetMeshPorts", servicePort.Port, destService.Name, clusterName, "Adding mesh port")
41+
ports[common.Http] = uint32(servicePort.Port)
42+
}
43+
}
44+
return ports
45+
}

‎admiral/pkg/clusters/util_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package clusters
2+
3+
import (
4+
"github.com/admiral/admiral/pkg/controller/common"
5+
k8sAppsV1 "k8s.io/api/apps/v1"
6+
v12 "k8s.io/api/core/v1"
7+
"k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"reflect"
9+
"strconv"
10+
"testing"
11+
12+
k8sV1 "k8s.io/api/core/v1"
13+
)
14+
15+
func TestGetMeshPorts(t *testing.T) {
16+
17+
annotatedPort := 8090
18+
defaultServicePort := uint32(8080)
19+
20+
defaultK8sSvcPortNoName := k8sV1.ServicePort{Port: int32(defaultServicePort)}
21+
defaultK8sSvcPort := k8sV1.ServicePort{Name: "default", Port: int32(defaultServicePort)}
22+
meshK8sSvcPort := k8sV1.ServicePort{Name: "mesh", Port: int32(annotatedPort)}
23+
24+
serviceMeshPorts := []k8sV1.ServicePort{defaultK8sSvcPort, meshK8sSvcPort}
25+
26+
serviceMeshPortsOnlyDefault := []k8sV1.ServicePort{defaultK8sSvcPortNoName}
27+
28+
service := k8sV1.Service{
29+
ObjectMeta: v1.ObjectMeta{Name: "server", Labels:map[string]string{"asset": "Intuit.platform.mesh.server"}},
30+
Spec: k8sV1.ServiceSpec{Ports: serviceMeshPorts},
31+
}
32+
deployment := k8sAppsV1.Deployment{
33+
Spec: k8sAppsV1.DeploymentSpec{Template:v12.PodTemplateSpec{
34+
ObjectMeta: v1.ObjectMeta{Annotations:map[string]string{common.SidecarEnabledPorts: strconv.Itoa(annotatedPort)}},
35+
}}}
36+
37+
ports := map[string]uint32{"http": uint32(annotatedPort)}
38+
39+
portsFromDefaultSvcPort := map[string]uint32{"http": defaultServicePort}
40+
41+
emptyPorts := map[string]uint32{}
42+
43+
testCases := []struct {
44+
name string
45+
clusterName string
46+
service k8sV1.Service
47+
deployment k8sAppsV1.Deployment
48+
expected map[string]uint32
49+
}{
50+
{
51+
name: "should return a port based on annotation",
52+
service: service,
53+
deployment: deployment,
54+
expected: ports,
55+
},
56+
{
57+
name: "should return a default port",
58+
service: k8sV1.Service{
59+
ObjectMeta: v1.ObjectMeta{Name: "server", Labels:map[string]string{"asset": "Intuit.platform.mesh.server"}},
60+
Spec: k8sV1.ServiceSpec{Ports: serviceMeshPortsOnlyDefault},
61+
},
62+
deployment: k8sAppsV1.Deployment{
63+
Spec: k8sAppsV1.DeploymentSpec{Template:v12.PodTemplateSpec{
64+
ObjectMeta: v1.ObjectMeta{Annotations:map[string]string{}},
65+
}}},
66+
expected: portsFromDefaultSvcPort,
67+
},
68+
{
69+
name: "should return empty ports",
70+
service: k8sV1.Service{
71+
ObjectMeta: v1.ObjectMeta{Name: "server", Labels:map[string]string{"asset": "Intuit.platform.mesh.server"}},
72+
Spec: k8sV1.ServiceSpec{Ports: nil},
73+
},
74+
deployment: k8sAppsV1.Deployment{
75+
Spec: k8sAppsV1.DeploymentSpec{Template:v12.PodTemplateSpec{
76+
ObjectMeta: v1.ObjectMeta{Annotations:map[string]string{}},
77+
}}},
78+
expected: emptyPorts,
79+
},
80+
}
81+
82+
for _, c := range testCases {
83+
t.Run(c.name, func(t *testing.T) {
84+
meshPorts := GetMeshPorts(c.clusterName, &c.service, &c.deployment)
85+
if !reflect.DeepEqual(meshPorts, c.expected) {
86+
t.Errorf("Wanted meshPorts: %v, got: %v", c.expected, meshPorts)
87+
}
88+
})
89+
}
90+
}

0 commit comments

Comments
 (0)