Skip to content

Commit

Permalink
Missing name in service port (#15)
Browse files Browse the repository at this point in the history
* Handle missing name on service port

* Undo un-needed change
  • Loading branch information
aattuluri committed Sep 25, 2019
1 parent 6a08f21 commit c31f71a
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 35 deletions.
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,5 @@ gen-yaml:
kustomize build ./install/admiralremote/base/ > ./out/yaml/remotecluster.yaml
kustomize build ./install/sample/base/ > ./out/yaml/sample.yaml
cp ./install/sample/sample_dep.yaml ./out/yaml/sample_dep.yaml
cp ./test/scripts/cluster-secret.sh ./out/scripts/cluster-secret.sh
cp ./install/scripts/cluster-secret.sh ./out/scripts/cluster-secret.sh
cp ./install/scripts/cluster-secret.sh ./out/scripts/cluster-secret.sh
39 changes: 5 additions & 34 deletions admiral/pkg/clusters/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ package clusters

import (
"context"
"github.com/gogo/protobuf/proto"
"github.com/admiral/admiral/pkg/apis/admiral/v1"
"github.com/admiral/admiral/pkg/controller/admiral"
"github.com/admiral/admiral/pkg/controller/common"
"github.com/admiral/admiral/pkg/controller/istio"
"github.com/admiral/admiral/pkg/controller/secret"
"github.com/admiral/admiral/pkg/controller/util"
"github.com/gogo/protobuf/proto"
"k8s.io/client-go/rest"
"strconv"
"strings"
"time"

Expand All @@ -20,8 +19,8 @@ import (
"sync"

istioModel "istio.io/istio/pilot/pkg/model"
k8sV1 "k8s.io/api/core/v1"
k8sAppsV1 "k8s.io/api/apps/v1"
k8sV1 "k8s.io/api/core/v1"
k8s "k8s.io/client-go/kubernetes"

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


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

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

func getMeshPorts(clusterName string, destService *k8sV1.Service,
destDeployment *k8sAppsV1.Deployment) map[string]uint32 {
var ports = make(map[string]uint32)
var meshPorts = destDeployment.Spec.Template.Annotations[common.SidecarEnabledPorts]
if len(meshPorts) == 0 {
log.Infof(LogFormat, "GetMeshPorts", "service", destService.Name, clusterName, "No mesh ports present, defaulting to first port")
//TODO default to 8090 instead if the first port?
ports[destService.Spec.Ports[0].Name] = uint32(destService.Spec.Ports[0].Port)
return ports
}

meshPortsSplit := strings.Split(meshPorts, ",")
var meshPortMap = make(map[uint32]uint32)
for _, meshPort := range meshPortsSplit {
port, err := strconv.ParseUint(meshPort, 10, 32)
if err != nil {
continue
}
meshPortMap[uint32(port)] = uint32(port)
}
for _, servicePort := range destService.Spec.Ports {
if _, ok := meshPortMap[uint32(servicePort.Port)]; ok {
log.Debugf(LogFormat, "GetMeshPorts", servicePort.Port, destService.Name, clusterName, "Adding mesh port")
ports[common.Http] = uint32(servicePort.Port)
}
}
return ports
}

func getDependentClusters(dependents *common.Map, identityClusterCache *common.MapOfMaps, sourceServices map[string]*k8sV1.Service) map[string]string {
var dependentClusters = make(map[string]string)
//TODO optimize this map construction
Expand Down
45 changes: 45 additions & 0 deletions admiral/pkg/clusters/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package clusters

import (
"github.com/admiral/admiral/pkg/controller/common"
"istio.io/istio/pkg/log"
"strconv"
"strings"

k8sV1 "k8s.io/api/core/v1"
k8sAppsV1 "k8s.io/api/apps/v1"
)

func GetMeshPorts(clusterName string, destService *k8sV1.Service,
destDeployment *k8sAppsV1.Deployment) map[string]uint32 {
var ports = make(map[string]uint32)
var meshPorts = destDeployment.Spec.Template.Annotations[common.SidecarEnabledPorts]
if len(meshPorts) == 0 {
log.Infof(LogFormat, "GetMeshPorts", "service", destService.Name, clusterName, "No mesh ports present, defaulting to first port")
if destService.Spec.Ports != nil && len(destService.Spec.Ports) > 0 {
var name = destService.Spec.Ports[0].Name
if len(name) == 0 {
name = common.Http
}
ports[name] = uint32(destService.Spec.Ports[0].Port)
}
return ports
}

meshPortsSplit := strings.Split(meshPorts, ",")
var meshPortMap = make(map[uint32]uint32)
for _, meshPort := range meshPortsSplit {
port, err := strconv.ParseUint(meshPort, 10, 32)
if err != nil {
continue
}
meshPortMap[uint32(port)] = uint32(port)
}
for _, servicePort := range destService.Spec.Ports {
if _, ok := meshPortMap[uint32(servicePort.Port)]; ok {
log.Debugf(LogFormat, "GetMeshPorts", servicePort.Port, destService.Name, clusterName, "Adding mesh port")
ports[common.Http] = uint32(servicePort.Port)
}
}
return ports
}
90 changes: 90 additions & 0 deletions admiral/pkg/clusters/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package clusters

import (
"github.com/admiral/admiral/pkg/controller/common"
k8sAppsV1 "k8s.io/api/apps/v1"
v12 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
"reflect"
"strconv"
"testing"

k8sV1 "k8s.io/api/core/v1"
)

func TestGetMeshPorts(t *testing.T) {

annotatedPort := 8090
defaultServicePort := uint32(8080)

defaultK8sSvcPortNoName := k8sV1.ServicePort{Port: int32(defaultServicePort)}
defaultK8sSvcPort := k8sV1.ServicePort{Name: "default", Port: int32(defaultServicePort)}
meshK8sSvcPort := k8sV1.ServicePort{Name: "mesh", Port: int32(annotatedPort)}

serviceMeshPorts := []k8sV1.ServicePort{defaultK8sSvcPort, meshK8sSvcPort}

serviceMeshPortsOnlyDefault := []k8sV1.ServicePort{defaultK8sSvcPortNoName}

service := k8sV1.Service{
ObjectMeta: v1.ObjectMeta{Name: "server", Labels:map[string]string{"asset": "Intuit.platform.mesh.server"}},
Spec: k8sV1.ServiceSpec{Ports: serviceMeshPorts},
}
deployment := k8sAppsV1.Deployment{
Spec: k8sAppsV1.DeploymentSpec{Template:v12.PodTemplateSpec{
ObjectMeta: v1.ObjectMeta{Annotations:map[string]string{common.SidecarEnabledPorts: strconv.Itoa(annotatedPort)}},
}}}

ports := map[string]uint32{"http": uint32(annotatedPort)}

portsFromDefaultSvcPort := map[string]uint32{"http": defaultServicePort}

emptyPorts := map[string]uint32{}

testCases := []struct {
name string
clusterName string
service k8sV1.Service
deployment k8sAppsV1.Deployment
expected map[string]uint32
}{
{
name: "should return a port based on annotation",
service: service,
deployment: deployment,
expected: ports,
},
{
name: "should return a default port",
service: k8sV1.Service{
ObjectMeta: v1.ObjectMeta{Name: "server", Labels:map[string]string{"asset": "Intuit.platform.mesh.server"}},
Spec: k8sV1.ServiceSpec{Ports: serviceMeshPortsOnlyDefault},
},
deployment: k8sAppsV1.Deployment{
Spec: k8sAppsV1.DeploymentSpec{Template:v12.PodTemplateSpec{
ObjectMeta: v1.ObjectMeta{Annotations:map[string]string{}},
}}},
expected: portsFromDefaultSvcPort,
},
{
name: "should return empty ports",
service: k8sV1.Service{
ObjectMeta: v1.ObjectMeta{Name: "server", Labels:map[string]string{"asset": "Intuit.platform.mesh.server"}},
Spec: k8sV1.ServiceSpec{Ports: nil},
},
deployment: k8sAppsV1.Deployment{
Spec: k8sAppsV1.DeploymentSpec{Template:v12.PodTemplateSpec{
ObjectMeta: v1.ObjectMeta{Annotations:map[string]string{}},
}}},
expected: emptyPorts,
},
}

for _, c := range testCases {
t.Run(c.name, func(t *testing.T) {
meshPorts := GetMeshPorts(c.clusterName, &c.service, &c.deployment)
if !reflect.DeepEqual(meshPorts, c.expected) {
t.Errorf("Wanted meshPorts: %v, got: %v", c.expected, meshPorts)
}
})
}
}

0 comments on commit c31f71a

Please sign in to comment.