Simplify isVarnishIngSvc().

... which determines if a Service defines the admin interfaces for
a Varnish Service that implements Ingress. We don't have to query
the Endpoints (which often don't exist yet when the Service is new),
because the service port definitions are in the Spec.

This means that the function doesn't have to return a possible
error, which simplifies the calling code. In particular,
filterVarnishIngSvcs(), which filters a slice of Services for those
for which isVarnishIngSvc() is true, does not have to pass along
the error return value. That in turn simplifies more calling code.

pkg/controller/endpoints.go

@@ -28,10 +28,7 @@
 
 package controller
 
-import (
-	"fmt"
-	api_v1 "k8s.io/api/core/v1"
-)
+import api_v1 "k8s.io/api/core/v1"
 
 func (worker *NamespaceWorker) syncEndp(key string) error {
 	worker.log.Infof("Syncing Endpoints: %s/%s", worker.namespace, key)
@@ -44,14 +41,7 @@ func (worker *NamespaceWorker) syncEndp(key string) error {
 		return nil
 	}
 
-	if eps, err := worker.getServiceEndpoints(svc); err != nil {
-		return err
-	} else if eps == nil {
-		return fmt.Errorf("could not find endpoints for service: %s/%s",
-			svc.Namespace, svc.Name)
-	} else if isIngress, err := worker.isVarnishIngSvc(svc, eps); err != nil {
-		return err
-	} else if isIngress {
+	if worker.isVarnishIngSvc(svc) {
 		worker.log.Infof("Endpoints changed for Varnish Ingress "+
 			"service %s/%s, enqueuing service sync", svc.Namespace,
 			svc.Name)

pkg/controller/ingress.go

@@ -60,23 +60,16 @@ const (
 
 func (worker *NamespaceWorker) filterVarnishIngSvcs(
 	svcs []*api_v1.Service,
-) ([]*api_v1.Service, error) {
+) []*api_v1.Service {
 	n := 0
 	for _, svc := range svcs {
-		if eps, err := worker.getServiceEndpoints(svc); err != nil {
-			return nil, err
-		} else if eps == nil {
-			return nil, fmt.Errorf("could not find endpoints for "+
-				"service: %s/%s", svc.Namespace, svc.Name)
-		} else if isIngress, err := worker.isVarnishIngSvc(svc, eps); err != nil {
-			return nil, err
-		} else if isIngress {
+		if worker.isVarnishIngSvc(svc) {
 			svcs[n] = svc
 			n++
 		}
 	}
 	svcs = svcs[:n]
-	return svcs, nil
+	return svcs
 }
 
 func (worker *NamespaceWorker) getVarnishSvcForIng(
@@ -87,19 +80,7 @@ func (worker *NamespaceWorker) getVarnishSvcForIng(
 		return nil, err
 	}
 	worker.log.Debugf("Cluster Services: %+v", svcs)
-	// n := 0
-	// for _, svc := range svcs {
-	// 	if isIngress, err := worker.isVarnishIngSvc(svc); err != nil {
-	// 		return nil, err
-	// 	} else if isIngress {
-	// 		svcs[n] = svc
-	// 		n++
-	// 	}
-	// }
-	// svcs = svcs[:n]
-	if svcs, err = worker.filterVarnishIngSvcs(svcs); err != nil {
-		return nil, err
-	}
+	svcs = worker.filterVarnishIngSvcs(svcs)
 	worker.log.Debugf("Cluster Varnish Services for Ingress: %+v", svcs)
 
 	if varnishSvc, exists := ing.Annotations[varnishSvcKey]; exists {
@@ -134,19 +115,7 @@ func (worker *NamespaceWorker) getVarnishSvcForIng(
 		return nil, err
 	}
 	worker.log.Debugf("Namespace Services: %+v", svcs)
-	// n = 0
-	// for _, svc := range svcs {
-	// 	if isIngress, err := worker.isVarnishIngSvc(svc); err != nil {
-	// 		return nil, err
-	// 	} else if isIngress {
-	// 		svcs[n] = svc
-	// 		n++
-	// 	}
-	// }
-	// svcs = svcs[:n]
-	if svcs, err = worker.filterVarnishIngSvcs(svcs); err != nil {
-		return nil, err
-	}
+	svcs = worker.filterVarnishIngSvcs(svcs)
 	worker.log.Debugf("Namespace Varnish Services for Ingress: %+v", svcs)
 	if len(svcs) == 1 {
 		worker.log.Debugf("Exactly one Varnish Ingress Service "+
@@ -167,16 +136,13 @@ func (worker *NamespaceWorker) getIngsForVarnishSvc(
 	if err != nil {
 		return nil, err
 	}
-	if allVarnishSvcs, err = worker.filterVarnishIngSvcs(allVarnishSvcs); err != nil {
-		return nil, err
-	}
+	allVarnishSvcs = worker.filterVarnishIngSvcs(allVarnishSvcs)
 	nsVarnishSvcs, err := worker.svc.List(varnishIngressSelector)
 	if err != nil {
 		return nil, err
 	}
-	if nsVarnishSvcs, err = worker.filterVarnishIngSvcs(nsVarnishSvcs); err != nil {
-		return nil, err
-	}
+	nsVarnishSvcs = worker.filterVarnishIngSvcs(nsVarnishSvcs)
+
 	ings4Svc := make([]*extensions.Ingress, 0)
 	for _, ing := range ings {
 		if !worker.isVarnishIngress(ing) {

pkg/controller/service.go

@@ -51,31 +51,19 @@ const (
 // can implement Ingress, for which this controller is responsible.
 // Currently the app label must point to a hardwired name, and a
 // hardwired admin port name must be defined for one of the Endpoints.
-func (worker *NamespaceWorker) isVarnishIngSvc(
-	svc *api_v1.Service,
-	endps *api_v1.Endpoints,
-) (bool, error) {
+func (worker *NamespaceWorker) isVarnishIngSvc(svc *api_v1.Service) bool {
 	app, exists := svc.Labels[labelKey]
 	if !exists || app != labelVal {
-		return false, nil
-	}
-	if endps == nil {
-		panic("isVarnishIngSvc(svc, endps == nil")
+		return false
 	}
-	for _, subset := range endps.Subsets {
-		worker.log.Debugf("Service %s/%s Endpoint subset: %+v",
-			svc.Namespace, svc.Name, subset)
-		worker.log.Debugf("Service %s/%s Endpoint Ports: %+v",
-			svc.Namespace, svc.Name, subset.Ports)
-		for _, port := range subset.Ports {
-			worker.log.Debugf("Service %s/%s port: %+v",
-				svc.Namespace, svc.Name, port)
-			if port.Name == admPortName {
-				return true, nil
-			}
+	for _, port := range svc.Spec.Ports {
+		worker.log.Debugf("Service %s/%s port: %+v", svc.Namespace,
+			svc.Name, port)
+		if port.Name == admPortName {
+			return true
 		}
 	}
-	return false, nil
+	return false
 }
 
 func (worker *NamespaceWorker) getIngsForSvc(
@@ -195,14 +183,8 @@ func (worker *NamespaceWorker) syncSvc(key string) error {
 	if err != nil {
 		return err
 	}
-	if eps, err := worker.getServiceEndpoints(svc); err != nil {
-		return err
-	} else if eps == nil {
-		return fmt.Errorf("could not find endpoints for service: %s/%s",
-			svc.Namespace, svc.Name)
-	} else if isIngress, err := worker.isVarnishIngSvc(svc, eps); err != nil {
-		return err
-	} else if !isIngress {
+
+	if !worker.isVarnishIngSvc(svc) {
 		return worker.enqueueIngressForService(svc)
 	}
 
@@ -399,14 +381,7 @@ func (worker *NamespaceWorker) deleteSvc(obj interface{}) error {
 		}
 	}
 
-	if endps, err := worker.getServiceEndpoints(svc); err != nil {
-		return err
-	} else if endps == nil {
-		worker.log.Warnf("Service %s: Endpoints already deleted", nsKey)
-		return nil
-	} else if isIngress, err := worker.isVarnishIngSvc(svc, endps); err != nil {
-		return err
-	} else if !isIngress {
+	if !worker.isVarnishIngSvc(svc) {
 		return worker.enqueueIngressForService(svc)
 	}