Skip to content

Commit

Permalink
Fix local SE generation. Fix related bugs. (#59)
Browse files Browse the repository at this point in the history
Fixes #56
Fixes #53
  • Loading branch information
aattuluri authored Jan 22, 2020
1 parent e676e0e commit 73888f7
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 29 deletions.
3 changes: 2 additions & 1 deletion admiral/cmd/admiral/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"flag"
"fmt"
"github.com/istio-ecosystem/admiral/admiral/pkg/clusters"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
"istio.io/istio/pkg/log"
"os"
"os/signal"
Expand All @@ -24,7 +25,7 @@ func GetRootCmd(args []string) *cobra.Command {

var ()

params := clusters.AdmiralParams{}
params := clusters.AdmiralParams{LabelSet: &common.LabelSet{}}

rootCmd := &cobra.Command{
Use: "Admiral",
Expand Down
10 changes: 5 additions & 5 deletions admiral/pkg/clusters/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,35 +274,35 @@ func (r *RemoteRegistry) createCacheController(clientConfig *rest.Config, cluste
return fmt.Errorf(" Error with Istio controller init: %v", err)
}

log.Infof("starting global traffic policy controller custerID: %v", clusterID)
log.Infof("starting global traffic policy controller clusterID: %v", clusterID)
rc.GlobalTraffic, err = admiral.NewGlobalTrafficController(stop, &GlobalTrafficHandler{RemoteRegistry: r}, clientConfig, resyncPeriod)

if err != nil {
return fmt.Errorf(" Error with GlobalTrafficController controller init: %v", err)
}

log.Infof("starting deployment controller custerID: %v", clusterID)
log.Infof("starting deployment controller clusterID: %v", clusterID)
rc.DeploymentController, err = admiral.NewDeploymentController(stop, &DeploymentHandler{RemoteRegistry: r}, clientConfig, resyncPeriod)

if err != nil {
return fmt.Errorf(" Error with DeploymentController controller init: %v", err)
}

log.Infof("starting pod controller custerID: %v", clusterID)
log.Infof("starting pod controller clusterID: %v", clusterID)
rc.PodController, err = admiral.NewPodController(stop, &PodHandler{RemoteRegistry: r}, clientConfig, resyncPeriod)

if err != nil {
return fmt.Errorf(" Error with PodController controller init: %v", err)
}

log.Infof("starting node controller custerID: %v", clusterID)
log.Infof("starting node controller clusterID: %v", clusterID)
rc.NodeController, err = admiral.NewNodeController(stop, &NodeHandler{RemoteRegistry: r}, clientConfig)

if err != nil {
return fmt.Errorf(" Error with NodeController controller init: %v", err)
}

log.Infof("starting service controller custerID: %v", clusterID)
log.Infof("starting service controller clusterID: %v", clusterID)
rc.ServiceController, err = admiral.NewServiceController(stop, &ServiceHandler{RemoteRegistry: r}, clientConfig, resyncPeriod)

if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions admiral/pkg/clusters/serviceentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func createServiceEntry(identifier string, rc *RemoteController, config AdmiralP
return tmpSe
}

func createServiceEntryForNewServiceOrPod(namespace string, sourceIdentity string, remoteRegistry *RemoteRegistry, syncNamespace string) map[string]*networking.ServiceEntry {
func createServiceEntryForNewServiceOrPod(env string, sourceIdentity string, remoteRegistry *RemoteRegistry) map[string]*networking.ServiceEntry {
//create a service entry, destination rule and virtual service in the local cluster
sourceServices := make(map[string]*k8sV1.Service)

Expand All @@ -116,15 +116,15 @@ func createServiceEntryForNewServiceOrPod(namespace string, sourceIdentity strin

deployment := rc.DeploymentController.Cache.Get(sourceIdentity)

if deployment == nil || deployment.Deployments[namespace] == nil {
if deployment == nil || deployment.Deployments[env] == nil {
continue
}

deploymentInstance := deployment.Deployments[namespace]
deploymentInstance := deployment.Deployments[env]

serviceInstance := getServiceForDeployment(rc, deploymentInstance[0], namespace)
serviceInstance := getServiceForDeployment(rc, deploymentInstance[0], env)

cname = common.GetCname(deploymentInstance[0], "identity", cname)
cname = common.GetCname(deploymentInstance[0], remoteRegistry.config.LabelSet.WorkloadIdentityLabel, cname)

remoteRegistry.AdmiralCache.IdentityClusterCache.Put(sourceIdentity, rc.ClusterID, rc.ClusterID)
remoteRegistry.AdmiralCache.CnameClusterCache.Put(cname, rc.ClusterID, rc.ClusterID)
Expand All @@ -133,7 +133,7 @@ func createServiceEntryForNewServiceOrPod(namespace string, sourceIdentity strin

sourceDeployments[rc.ClusterID] = deploymentInstance[0]

createServiceEntry("identity", rc, remoteRegistry.config, remoteRegistry.AdmiralCache, deploymentInstance[0], serviceEntries)
createServiceEntry(remoteRegistry.config.LabelSet.WorkloadIdentityLabel, rc, remoteRegistry.config, remoteRegistry.AdmiralCache, deploymentInstance[0], serviceEntries)

}

Expand Down
2 changes: 1 addition & 1 deletion admiral/pkg/clusters/serviceentry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestCreateServiceEntryForNewServiceOrPod(t *testing.T) {
})

rr.remoteControllers["test.cluster"] = rc
createServiceEntryForNewServiceOrPod("test", "bar", rr, "sync")
createServiceEntryForNewServiceOrPod("test", "bar", rr)

}

Expand Down
7 changes: 5 additions & 2 deletions admiral/pkg/clusters/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ func (pc *DeploymentHandler) Added(obj *k8sAppsV1.Deployment) {
return
}

createServiceEntryForNewServiceOrPod(obj.Namespace, globalIdentifier, pc.RemoteRegistry, pc.RemoteRegistry.config.SyncNamespace)
env := common.GetEnv(obj)

createServiceEntryForNewServiceOrPod(env, globalIdentifier, pc.RemoteRegistry)
}

func (pc *DeploymentHandler) Deleted(obj *k8sAppsV1.Deployment) {
Expand All @@ -172,7 +174,8 @@ func (pc *PodHandler) Added(obj *k8sV1.Pod) {
return
}

createServiceEntryForNewServiceOrPod(obj.Namespace, globalIdentifier, pc.RemoteRegistry, pc.RemoteRegistry.config.SyncNamespace)
//TODO Skip pod events until GTP is implemented
//createServiceEntryForNewServiceOrPod(obj.Namespace, globalIdentifier, pc.RemoteRegistry)
}

func (pc *PodHandler) Deleted(obj *k8sV1.Pod) {
Expand Down
13 changes: 6 additions & 7 deletions admiral/pkg/controller/admiral/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,16 @@ func (p *deploymentCache) AppendDeploymentToCluster(key string, deployment *k8sA
}
p.cache[v.Identity] = v
}
env := common.GetEnv(deployment)
envDeployments := v.Deployments[env]

//TODO this is assuming globally unquie names name which might not alway be the case. This would need a cluster name too
namespaceDeployments := v.Deployments[deployment.Namespace]

if namespaceDeployments == nil {
namespaceDeployments = make([]*k8sAppsV1.Deployment, 0)
if envDeployments == nil {
envDeployments = make([]*k8sAppsV1.Deployment, 0)
}

namespaceDeployments = append(namespaceDeployments, deployment)
envDeployments = append(envDeployments, deployment)

v.Deployments[deployment.Namespace] = namespaceDeployments
v.Deployments[env] = envDeployments

}

Expand Down
4 changes: 2 additions & 2 deletions admiral/pkg/controller/admiral/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ func TestDeploymentController_Added(t *testing.T) {
}
} else if len(depController.Cache.cache)==0 && c.expectedCacheSize != 0 {
t.Errorf("Unexpectedly empty cache. Length should have been %v but was 0", c.expectedCacheSize)
}else if len(depController.Cache.cache["id"].Deployments) < 1 && len(depController.Cache.cache["id"].Deployments[""]) != c.expectedCacheSize {
}else if len(depController.Cache.cache["id"].Deployments) < 1 && len(depController.Cache.cache["id"].Deployments[common.Default]) != c.expectedCacheSize {
t.Errorf("Deployment controller cache the wrong size. Got %v, expected %v", len(depController.Cache.cache["id"].Deployments[""]), c.expectedCacheSize)
} else if depController.Cache.cache["id"].Deployments[""][0] != &deployment {
} else if depController.Cache.cache["id"].Deployments[common.Default][0] != &deployment {
t.Errorf("Incorrect deployment added to deployment controller cache. Got %v expected %v", depController.Cache.cache["id"].Deployments[""][0], deployment)
}

Expand Down
23 changes: 18 additions & 5 deletions admiral/pkg/controller/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/sirupsen/logrus"
k8sAppsV1 "k8s.io/api/apps/v1"
k8sV1 "k8s.io/api/core/v1"
"strings"
)

const (
Expand All @@ -24,6 +25,7 @@ const (
NodeRegionLabel = "failure-domain.beta.kubernetes.io/region"
SpiffePrefix = "spiffe://"
SidecarEnabledPorts = "traffic.sidecar.istio.io/includeInboundPorts"
Default = "default"
)

func GetPodGlobalIdentifier(pod *k8sV1.Pod) string {
Expand All @@ -49,11 +51,7 @@ func DefaultGlobalIdentifier() string {

// GetCname returns cname in the format <env>.<service identity>.global, Ex: stage.Admiral.services.registry.global
func GetCname(deployment *k8sAppsV1.Deployment, identifier string, nameSuffix string) string {
var environment = deployment.Spec.Template.Labels[Env]
if len(environment) == 0 {
environment = "default"
logrus.Warnf("%v label missing on %v in namespace %v. Using 'default' as the value.", Env, deployment.Name, deployment.Namespace)
}
var environment = GetEnv(deployment)
alias := deployment.Spec.Template.Labels[identifier]
if len(alias) == 0 {
logrus.Warnf("%v label missing on service %v in namespace %v. Falling back to annotation to create cname.", identifier, deployment.Name, deployment.Namespace)
Expand All @@ -66,6 +64,21 @@ func GetCname(deployment *k8sAppsV1.Deployment, identifier string, nameSuffix st
return environment + Sep + alias + Sep + nameSuffix
}

func GetEnv(deployment *k8sAppsV1.Deployment) string {
var environment = deployment.Spec.Template.Labels[Env]
if len(environment) == 0 {
environment = deployment.Spec.Template.Annotations[Env]
}
if len(environment) == 0 {
splitNamespace := strings.Split(deployment.Namespace, Dash)
environment = splitNamespace[len(splitNamespace) - 1]
}
if len(environment) == 0 {
environment = Default
}
return environment
}

// GetSAN returns SAN for a service entry in the format spiffe://<domain>/<identifier>, Ex: spiffe://subdomain.domain.com/Admiral.platform.mesh.server
func GetSAN(domain string, deployment *k8sAppsV1.Deployment, identifier string) string {
identifierVal := deployment.Spec.Template.Labels[identifier]
Expand Down
39 changes: 39 additions & 0 deletions admiral/pkg/controller/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,42 @@ func TestGetPodGlobalIdentifier(t *testing.T) {
})
}
}

func TestGetEnv(t *testing.T) {

testCases := []struct {
name string
deployment k8sAppsV1.Deployment
expected string
}{
{
name: "should return default env",
deployment: k8sAppsV1.Deployment{Spec: k8sAppsV1.DeploymentSpec{Template: v12.PodTemplateSpec{ObjectMeta: v1.ObjectMeta{Labels: map[string]string{}}}}},
expected: Default,
},
{
name: "should return valid env from label",
deployment: k8sAppsV1.Deployment{Spec: k8sAppsV1.DeploymentSpec{Template: v12.PodTemplateSpec{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{}, Labels: map[string]string{"env": "stage"}}}}},
expected: "stage",
},
{
name: "should return valid env from annotation",
deployment: k8sAppsV1.Deployment{Spec: k8sAppsV1.DeploymentSpec{Template: v12.PodTemplateSpec{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{"env": "stage"}}}}},
expected: "stage",
},
{
name: "should return env from namespace suffix",
deployment: k8sAppsV1.Deployment{Spec: k8sAppsV1.DeploymentSpec{Template: v12.PodTemplateSpec{ObjectMeta: v1.ObjectMeta{Labels: map[string]string{}}}}, ObjectMeta: v1.ObjectMeta{Namespace: "uswest2-prd"}},
expected: "prd",
},
}

for _, c := range testCases {
t.Run(c.name, func(t *testing.T) {
env := GetEnv(&c.deployment)
if !(env == c.expected) {
t.Errorf("Wanted Cname: %s, got: %s", c.expected, env)
}
})
}
}

0 comments on commit 73888f7

Please sign in to comment.