DevFW-CICD/ingress-nginx-helm
6
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove multiple calls to CreateInstanceGroups by reusing results from a single call

Browse source
This commit is contained in:
nikhiljindal 2017-09-11 20:29:49 -07:00
parent 0f4f5c97d4
commit 937cde666e
10 changed files with 68 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

35
controllers/gce/controller/cluster_manager.go
View file

@ -116,9 +116,10 @@ func (c *ClusterManager) shutdown() error {
// instance groups.
// - backendServicePorts are the ports for which we require BackendServices.
// - namedPorts are the ports which must be opened on instance groups.
// Returns the list of all instance groups corresponding to the given loadbalancers.
// If in performing the checkpoint the cluster manager runs out of quota, a
// googleapi 403 is returned.
func (c *ClusterManager) Checkpoint(lbs []*loadbalancers.L7RuntimeInfo, nodeNames []string, backendServicePorts []backends.ServicePort, namedPorts []backends.ServicePort) error {
func (c *ClusterManager) Checkpoint(lbs []*loadbalancers.L7RuntimeInfo, nodeNames []string, backendServicePorts []backends.ServicePort, namedPorts []backends.ServicePort) ([]*compute.InstanceGroup, error) {
if len(namedPorts) != 0 {
// Add the default backend node port to the list of named ports for instance groups.
namedPorts = append(namedPorts, c.defaultBackendNodePort)
@ -129,19 +130,18 @@ func (c *ClusterManager) Checkpoint(lbs []*loadbalancers.L7RuntimeInfo, nodeName
namedPorts = uniq(namedPorts)
backendServicePorts = uniq(backendServicePorts)
// Create Instance Groups.
_, err := c.CreateInstanceGroups(namedPorts)
igs, err := c.EnsureInstanceGroupsAndPorts(namedPorts)
if err != nil {
return err
return igs, err
}
if err := c.backendPool.Sync(backendServicePorts); err != nil {
return err
if err := c.backendPool.Sync(backendServicePorts, igs); err != nil {
return igs, err
}
if err := c.SyncNodesInInstanceGroups(nodeNames); err != nil {
return err
if err := c.instancePool.Sync(nodeNames); err != nil {
return igs, err
}
if err := c.l7Pool.Sync(lbs); err != nil {
return err
return igs, err
}
// TODO: Manage default backend and its firewall rule in a centralized way.
@ -160,22 +160,22 @@ func (c *ClusterManager) Checkpoint(lbs []*loadbalancers.L7RuntimeInfo, nodeName
np = append(np, p.Port)
}
if err := c.firewallPool.Sync(np, nodeNames); err != nil {
return err
return igs, err
}
return nil
return igs, nil
}
func (c *ClusterManager) CreateInstanceGroups(servicePorts []backends.ServicePort) ([]*compute.InstanceGroup, error) {
func (c *ClusterManager) EnsureInstanceGroupsAndPorts(servicePorts []backends.ServicePort) ([]*compute.InstanceGroup, error) {
var igs []*compute.InstanceGroup
var err error
for _, p := range servicePorts {
// CreateInstanceGroups always returns all the instance groups, so we can return
// EnsureInstanceGroupsAndPorts always returns all the instance groups, so we can return
// the output of any call, no need to append the return from all calls.
// TODO: Ideally, we want to call CreateInstaceGroups only the first time and
// then call AddNamedPort multiple times. Need to update the interface to
// achieve this.
igs, _, err = instances.CreateInstanceGroups(c.instancePool, c.ClusterNamer, p.Port)
igs, _, err = instances.EnsureInstanceGroupsAndPorts(c.instancePool, c.ClusterNamer, p.Port)
if err != nil {
return nil, err
}
@ -183,13 +183,6 @@ func (c *ClusterManager) CreateInstanceGroups(servicePorts []backends.ServicePor
return igs, nil
}
func (c *ClusterManager) SyncNodesInInstanceGroups(nodeNames []string) error {
if err := c.instancePool.Sync(nodeNames); err != nil {
return err
}
return nil
}
// GC garbage collects unused resources.
// - lbNames are the names of L7 loadbalancers we wish to exist. Those not in
// this list are removed from the cloud.

35
controllers/gce/controller/controller.go
View file

@ -36,7 +36,6 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/ingress/controllers/gce/backends"
"k8s.io/ingress/controllers/gce/loadbalancers"
)
@ -288,7 +287,7 @@ func (lbc *LoadBalancerController) sync(key string) (err error) {
allNodePorts := lbc.tr.toNodePorts(&allIngresses)
gceNodePorts := lbc.tr.toNodePorts(&gceIngresses)
lbNames := lbc.ingLister.Store.ListKeys()
lbs, err := lbc.ListGCERuntimeInfo()
lbs, err := lbc.toRuntimeInfo(gceIngresses)
if err != nil {
return err
}
@ -319,10 +318,10 @@ func (lbc *LoadBalancerController) sync(key string) (err error) {
}
glog.V(3).Infof("Finished syncing %v", key)
}()
// Record any errors during sync and throw a single error at the end. This
// allows us to free up associated cloud resources ASAP.
if err := lbc.CloudClusterManager.Checkpoint(lbs, nodeNames, gceNodePorts, allNodePorts); err != nil {
igs, err := lbc.CloudClusterManager.Checkpoint(lbs, nodeNames, gceNodePorts, allNodePorts)
if err != nil {
// TODO: Implement proper backoff for the queue.
eventMsg := "GCE"
if ingExists {
@ -336,19 +335,12 @@ func (lbc *LoadBalancerController) sync(key string) (err error) {
if !ingExists {
return syncError
}
ing := obj.(*extensions.Ingress)
if isGCEMultiClusterIngress(ing) {
ing := *obj.(*extensions.Ingress)
if isGCEMultiClusterIngress(&ing) {
// Add instance group names as annotation on the ingress.
if ing.Annotations == nil {
ing.Annotations = map[string]string{}
}
// Since we just created instance groups in Checkpoint, calling create
// instance groups again should just return names of the existing
// instance groups. It does not matter which nodePort we pass as argument.
igs, err := lbc.CloudClusterManager.CreateInstanceGroups([]backends.ServicePort{allNodePorts[0]})
if err != nil {
return fmt.Errorf("error in creating instance groups: %v", err)
}
err = setInstanceGroupsAnnotation(ing.Annotations, igs)
if err != nil {
return err
@ -366,13 +358,13 @@ func (lbc *LoadBalancerController) sync(key string) (err error) {
return syncError
}
if urlMap, err := lbc.tr.toURLMap(ing); err != nil {
if urlMap, err := lbc.tr.toURLMap(&ing); err != nil {
syncError = fmt.Errorf("%v, convert to url map error %v", syncError, err)
} else if err := l7.UpdateUrlMap(urlMap); err != nil {
lbc.recorder.Eventf(ing, apiv1.EventTypeWarning, "UrlMap", err.Error())
lbc.recorder.Eventf(&ing, apiv1.EventTypeWarning, "UrlMap", err.Error())
syncError = fmt.Errorf("%v, update url map error: %v", syncError, err)
} else if err := lbc.updateIngressStatus(l7, *ing); err != nil {
lbc.recorder.Eventf(ing, apiv1.EventTypeWarning, "Status", err.Error())
} else if err := lbc.updateIngressStatus(l7, ing); err != nil {
lbc.recorder.Eventf(&ing, apiv1.EventTypeWarning, "Status", err.Error())
syncError = fmt.Errorf("%v, update ingress error: %v", syncError, err)
}
return syncError
@ -432,13 +424,8 @@ func (lbc *LoadBalancerController) updateAnnotations(name, namespace string, ann
return nil
}
// ListGCERuntimeInfo lists L7RuntimeInfo as understood by the loadbalancer module.
// It returns runtime info only for gce ingresses and not for multi cluster ingresses.
func (lbc *LoadBalancerController) ListGCERuntimeInfo() (lbs []*loadbalancers.L7RuntimeInfo, err error) {
ingList, err := lbc.ingLister.ListGCEIngresses()
if err != nil {
return lbs, err
}
// toRuntimeInfo returns L7RuntimeInfo for the given ingresses.
func (lbc *LoadBalancerController) toRuntimeInfo(ingList extensions.IngressList) (lbs []*loadbalancers.L7RuntimeInfo, err error) {
for _, ing := range ingList.Items {
k, err := keyFunc(&ing)
if err != nil {

8
controllers/gce/controller/utils.go
View file

@ -521,13 +521,13 @@ func (t *GCETranslator) ingressToNodePorts(ing *extensions.Ingress) []backends.S
for _, rule := range ing.Spec.Rules {
if rule.HTTP == nil {
glog.Errorf("ignoring non http Ingress rule")
return knownPorts
continue
}
for _, path := range rule.HTTP.Paths {
port, err := t.getServiceNodePort(path.Backend, ing.Namespace)
if err != nil {
glog.Infof("%v", err)
return knownPorts
continue
}
knownPorts = append(knownPorts, port)
}
@ -680,7 +680,7 @@ func setInstanceGroupsAnnotation(existing map[string]string, igs []*compute.Inst
Name string
Zone string
}
instanceGroups := []Value{}
var instanceGroups []Value
for _, ig := range igs {
instanceGroups = append(instanceGroups, Value{Name: ig.Name, Zone: ig.Zone})
}
@ -698,7 +698,7 @@ func uniq(nodePorts []backends.ServicePort) []backends.ServicePort {
for _, p := range nodePorts {
portMap[p.Port] = p
}
nodePorts = []backends.ServicePort{}
nodePorts = make([]backends.ServicePort, 0, len(portMap))
for _, sp := range portMap {
nodePorts = append(nodePorts, sp)
}