Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
akshayhavile committed Sep 20, 2024
1 parent 0597f20 commit 2d1824f
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 25 deletions.
2 changes: 1 addition & 1 deletion helm/ako/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ data:
useDefaultSecretsOnly: {{ .Values.AKOSettings.useDefaultSecretsOnly | quote }}
enablePrometheus: {{ default "false" .Values.featureGates.EnablePrometheus | quote }}
enableEndpointSlice: {{ default "false" .Values.featureGates.EnableEndpointSlice | quote }}
FQDNReusePolicy: {{ default "InterNamespaceAllowed" .Values.L7Settings.FQDNReusePolicy | quote}}
fqdnReusePolicy: {{ default "InterNamespaceAllowed" .Values.L7Settings.fqdnReusePolicy | quote}}
2 changes: 1 addition & 1 deletion helm/ako/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ spec:
valueFrom:
configMapKeyRef:
name: avi-k8s-config
key: FQDNReusePolicy
key: fqdnReusePolicy
resources:
{{- toYaml .Values.resources | nindent 12 }}
livenessProbe:
Expand Down
2 changes: 1 addition & 1 deletion helm/ako/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ L7Settings:
shardVSSize: "LARGE" # Use this to control the layer 7 VS numbers. This applies to both secure/insecure VSes but does not apply for passthrough. ENUMs: LARGE, MEDIUM, SMALL, DEDICATED
passthroughShardSize: "SMALL" # Control the passthrough virtualservice numbers using this ENUM. ENUMs: LARGE, MEDIUM, SMALL
enableMCI: "false" # Enabling this flag would tell AKO to start processing multi-cluster ingress objects.
FQDNReusePolicy: "InterNamespaceAllowed" # Use this to control whether AKO allows cross-namespace usage of FQDNs. enum Strict|InterNamespaceAllowed
fqdnReusePolicy: "InterNamespaceAllowed" # Use this to control whether AKO allows cross-namespace usage of FQDNs. enum Strict|InterNamespaceAllowed

### This section outlines all the knobs used to control Layer 4 loadbalancing settings in AKO.
L4Settings:
Expand Down
10 changes: 5 additions & 5 deletions internal/lib/control_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ type akoControlConfig struct {
//endpointSlices Enabled
isEndpointSlicesEnabled bool

//FQDNReusePolicy is set to Strict/InterNamespaceAllowed according to whether AKO allows FQDN sharing across namespaces
FQDNReusePolicy string
//fqdnReusePolicy is set to Strict/InterNamespaceAllowed according to whether AKO allows FQDN sharing across namespaces
fqdnReusePolicy string
}

var akoControlConfigInstance *akoControlConfig
Expand Down Expand Up @@ -366,15 +366,15 @@ func (c *akoControlConfig) SetAKOFQDNReusePolicy(FQDNPolicy string) {
if FQDNPolicy == "" || !IsEvhEnabled() {
FQDNPolicy = FQDNReusePolicyOpen
}
c.FQDNReusePolicy = strings.ToLower(FQDNPolicy)
utils.AviLog.Infof("AKO FQDN reuse policy is: %s", c.FQDNReusePolicy)
c.fqdnReusePolicy = strings.ToLower(FQDNPolicy)
utils.AviLog.Infof("AKO FQDN reuse policy is: %s", c.fqdnReusePolicy)
}

// This utility returns FQDN Reuse policy of AKO.
// Strict --> FQDN restrict to one namespace
// InternamespaceAllowed --> FQDN can be spanned across multiple namespaces
func (c *akoControlConfig) GetAKOFQDNReusePolicy() string {
return c.FQDNReusePolicy
return c.fqdnReusePolicy
}

func initControllerVersion() string {
Expand Down
29 changes: 14 additions & 15 deletions internal/objects/uniquehostname.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ import (
"github.com/vmware/load-balancer-and-ingress-services-for-kubernetes/pkg/utils"
)

var uniqueHostListerInstace *UniqueHostNamespaceLister
var uniqueHostListerInstance *UniqueHostNamespaceLister
var uniqueHostListerOnce sync.Once

func SharedUniqueNamespaceLister() *UniqueHostNamespaceLister {
uniqueHostListerOnce.Do(func() {
uniqueHostListerInstace = &UniqueHostNamespaceLister{
uniqueHostListerInstance = &UniqueHostNamespaceLister{
HostnameToRoutesStore: NewObjectMapStore(),
RouteToHostnamesStore: NewObjectMapStore(),
}
})
return uniqueHostListerInstace
return uniqueHostListerInstance
}

type UniqueHostNamespaceLister struct {
HostnameLock sync.RWMutex
// hostname --> Active and InActive Routes
// hostname --> Active Routes
HostnameToRoutesStore *ObjectMapStore
// Route (namespace/name) --> Hostnames (not being used)
RouteToHostnamesStore *ObjectMapStore
Expand Down Expand Up @@ -93,9 +93,9 @@ func (n *UniqueHostNamespaceLister) UpdateHostnameToRoute(hostName string, route
return true, nil, nil
}

// Key: not found in active and inactive list, now compare namespace present in activeRoutes
// Key: not found in active , now compare namespace present in activeRoutes
sameNamespace := false
currentRouteNamespace, _ := ExtractTypeNameNamespace(route.RouteNSRouteName)
currentRouteNamespace, _ := ExtractNameNamespace(route.RouteNSRouteName)
// TODO: Debug statements can be reduced
utils.AviLog.Debugf("Namespace of current route is: %s", currentRouteNamespace)

Expand All @@ -104,7 +104,7 @@ func (n *UniqueHostNamespaceLister) UpdateHostnameToRoute(hostName string, route
// Need to check in case of delete of hostname, are we deleting all these list and entry from internal datastore.
for nsName, creationTime := range existingRoutes.activeRoutes {
utils.AviLog.Debugf("NamespaceName from active route list is: %v and creation timestamp: %v", nsName, creationTime)
existingActiveRoutesNamespace, _ := ExtractTypeNameNamespace(nsName)
existingActiveRoutesNamespace, _ := ExtractNameNamespace(nsName)
utils.AviLog.Debugf("Extracted out namespace is: %s", existingActiveRoutesNamespace)
if currentRouteNamespace == existingActiveRoutesNamespace {
// add it to active list
Expand All @@ -113,7 +113,6 @@ func (n *UniqueHostNamespaceLister) UpdateHostnameToRoute(hostName string, route
}
}
// With new logic, we need to check length of active route list. if it is empty then, we need to take another namespace
// from inactive list and fetch all routes of it with given fqdn and ingest it.

if sameNamespace {
utils.AviLog.Debugf("Namespace is same. Attaching Route %v to active route list", route.RouteNSRouteName)
Expand All @@ -126,15 +125,15 @@ func (n *UniqueHostNamespaceLister) UpdateHostnameToRoute(hostName string, route
return false, nil, nil
}

func ExtractTypeNameNamespace(nsName string) (string, string) {
func ExtractNameNamespace(nsName string) (string, string) {
if nsName != "" {
// changed from Split to SplitN
spliStrings := strings.SplitN(nsName, "/", 3)
if len(spliStrings) == 3 {
return spliStrings[1], spliStrings[2]
splitStrings := strings.SplitN(nsName, "/", 3)
if len(splitStrings) == 3 {
return splitStrings[1], splitStrings[2]
}
if len(spliStrings) == 2 {
return spliStrings[0], spliStrings[1]
if len(splitStrings) == 2 {
return splitStrings[0], splitStrings[1]
}
}
return "", ""
Expand All @@ -154,7 +153,7 @@ func (n *UniqueHostNamespaceLister) DeleteHostnameToRoute(hostName string, route
// Check route is present in active list
if _, flag := existingRoutes.activeRoutes[route.RouteNSRouteName]; flag {
// present
utils.AviLog.Debugf("RouteNS to be delete: %v and Active routes are: %v", route.RouteNSRouteName, existingRoutes.activeRoutes)
utils.AviLog.Debugf("RouteNS to be deleted: %v and Active routes are: %v", route.RouteNSRouteName, existingRoutes.activeRoutes)
delete(existingRoutes.activeRoutes, route.RouteNSRouteName)

utils.AviLog.Debugf("After deleting Active routes are %v", utils.Stringify(existingRoutes.activeRoutes))
Expand Down
4 changes: 2 additions & 2 deletions tests/evhtests/l7_evh_graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2103,7 +2103,7 @@ func TestMultiIngressSameHostDifferentNamespaceForEvh(t *testing.T) {

_, err := KubeClient.NetworkingV1().Ingresses("default").Create(context.TODO(), ingrFake1, metav1.CreateOptions{})
if err != nil {
t.Fatalf("error in adding Ingress: %v", err)
t.Fatalf("Error in adding Ingress: %v", err)
}

// red namespace
Expand All @@ -2117,7 +2117,7 @@ func TestMultiIngressSameHostDifferentNamespaceForEvh(t *testing.T) {

_, err = KubeClient.NetworkingV1().Ingresses("red").Create(context.TODO(), ingrFake2, metav1.CreateOptions{})
if err != nil {
t.Fatalf("error in adding Ingress: %v", err)
t.Fatalf("Error in adding Ingress: %v", err)
}

integrationtest.PollForCompletion(t, modelName, 5)
Expand Down

0 comments on commit 2d1824f

Please sign in to comment.