Bugfix updating endpoints when an IngressSpec has not changed.

The Ingress namespace/name and UID do not change, so we cannot depend
on those fields alone.

- For each endpoint update, consider an Ingress update.
- For the candidate Ingress, compute the VCL spec.
- Compute a canonical form for VCL specs.
- Compute a hash for a VCL spec.
- The Ingress is updated if the canonical form of its VCL spec is
  not deeply equal to the current VCL spec.
- VCL configs are identified by the hash (in addition to namespace/name
  and UID of the Ingress).

cmd/controller/controller.go

@@ -430,6 +430,13 @@ func (ingc *IngressController) addOrUpdateIng(task Task,
 		return
 	}
 
+	ingc.log.Debugf("Check if Ingress is loaded: key=%s uuid=%s hash=%0x",
+		key, string(ing.UID), vclSpec.Canonical.DeepHash())
+	if ingc.hasIngress(&ing, vclSpec) {
+		ingc.log.Infof("Ingress %s uid=%s hash=%0x already loaded", key,
+			ing.UID, vclSpec.Canonical.DeepHash())
+		return
+	}
 	ingc.log.Debugf("Update Ingress key=%s uuid=%s: %+v", key,
 		string(ing.ObjectMeta.UID), vclSpec)
 	err = ingc.vController.Update(key, string(ing.ObjectMeta.UID), vclSpec)
@@ -457,13 +464,19 @@ func (ingc *IngressController) syncEndp(task Task) {
 	}
 
 	if endpExists {
+		ingc.log.Debug("Checking ingresses for endpoints:", key)
 		ings := ingc.getIngForEndp(obj)
 
+		if len(ings) == 0 {
+			ingc.log.Debug("No ingresses for endpoints:", key)
+			return
+		}
+
+		ingc.log.Debugf("Update ingresses for endpoints %s", key)
 		for _, ing := range ings {
 			if !ingc.isVarnishIngress(&ing) {
-				continue
-			}
-			if !ingc.hasIngress(&ing) {
+				ingc.log.Debugf("Ingress %s/%s: not Varnish",
+					ing.Namespace, ing.Name)
 				continue
 			}
 			ingc.addOrUpdateIng(task, ing)
@@ -517,9 +530,6 @@ func (ingc *IngressController) enqueueIngressForService(svc *api_v1.Service) {
 		if !ingc.isVarnishIngress(&ing) {
 			continue
 		}
-		if !ingc.hasIngress(&ing) {
-			continue
-		}
 		ingc.syncQueue.enqueue(&ing)
 	}
 }
@@ -811,9 +821,11 @@ func (ingc *IngressController) isVarnishIngress(ing *extensions.Ingress) bool {
 	return true
 }
 
-func (ingc *IngressController) hasIngress(ing *extensions.Ingress) bool {
+func (ingc *IngressController) hasIngress(ing *extensions.Ingress,
+	spec vcl.Spec) bool {
+
 	name := ing.ObjectMeta.Namespace + "/" + ing.ObjectMeta.Name
-	return ingc.vController.HasIngress(name)
+	return ingc.vController.HasIngress(name, string(ing.UID), spec)
 }
 
 // isVarnishAdmSvc determines if a Service represents the admin

cmd/varnish/varnish.go

@@ -42,6 +42,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"reflect"
 	"regexp"
 	"strconv"
 	"strings"
@@ -59,6 +60,7 @@ const (
 	readinessLabel = "vk8s_readiness"
 	readyCfg       = "vk8s_ready"
 	notAvailCfg    = "vk8s_notavailable"
+	ingressPrefix  = "vk8s_ing_"
 )
 
 // XXX make admTimeout configurable
@@ -67,11 +69,6 @@ var (
 	admTimeout = time.Second * 10
 )
 
-func vclConfigName(key string, uid string) string {
-	name := "vk8sing_" + key + "_" + uid
-	return nonAlNum.ReplaceAllLiteralString(name, "_")
-}
-
 type VarnishAdmError struct {
 	addr string
 	err  error
@@ -101,6 +98,12 @@ type vclSpec struct {
 	uid  string
 }
 
+func (spec vclSpec) configName() string {
+	name := fmt.Sprintf("%s%s_%s_%0x", ingressPrefix, spec.key, spec.uid,
+		spec.spec.Canonical().DeepHash())
+	return nonAlNum.ReplaceAllLiteralString(name, "_")
+}
+
 type varnishSvc struct {
 	addr   string
 	Banner string
@@ -228,7 +231,7 @@ func (vc *VarnishController) updateVarnishInstances(svcs []*varnishSvc) error {
 	if err != nil {
 		return err
 	}
-	cfgName := vclConfigName(vc.spec.key, vc.spec.uid)
+	cfgName := vc.spec.configName()
 
 	vc.log.Infof("Update Varnish instances: load config %s", cfgName)
 	var errs VarnishAdmErrors
@@ -398,6 +401,7 @@ func (vc *VarnishController) DeleteVarnishSvc(key string) error {
 }
 
 func (vc *VarnishController) Update(key, uid string, spec vcl.Spec) error {
+
 	if vc.spec != nil && vc.spec.key != "" && vc.spec.key != key {
 		return fmt.Errorf("Multiple Ingress definitions currently not "+
 			"supported: current=%s new=%s", vc.spec.key, key)
@@ -456,11 +460,13 @@ func (vc *VarnishController) DeleteIngress(key string) error {
 }
 
 // Currently only one Ingress at a time
-func (vc *VarnishController) HasIngress(key string) bool {
+func (vc *VarnishController) HasIngress(key, uid string, spec vcl.Spec) bool {
 	if vc.spec == nil {
 		return false
 	}
-	return vc.spec.key == key
+	return vc.spec.key == key &&
+		vc.spec.uid == uid &&
+		reflect.DeepEqual(vc.spec.spec.Canonical(), spec.Canonical())
 }
 
 func (vc *VarnishController) SetAdmSecret(secret []byte) {

cmd/varnish/vcl/vcl.go

@@ -29,8 +29,12 @@
 package vcl
 
 import (
+	"encoding/binary"
 	"fmt"
+	"hash"
+	"hash/fnv"
 	"regexp"
+	"sort"
 	"text/template"
 )
 
@@ -39,22 +43,112 @@ type Address struct {
 	Port int32
 }
 
+func (addr Address) hash(hash hash.Hash) {
+	portBytes := make([]byte, 4)
+	binary.BigEndian.PutUint32(portBytes, uint32(addr.Port))
+	hash.Write([]byte(addr.IP))
+	hash.Write(portBytes)
+}
+
+// interface for sorting []Address
+type ByIPPort []Address
+
+func (a ByIPPort) Len() int      { return len(a) }
+func (a ByIPPort) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
+func (a ByIPPort) Less(i, j int) bool {
+	if a[i].IP < a[j].IP {
+		return true
+	}
+	return a[i].Port < a[j].Port
+}
+
 type Service struct {
 	Name      string
 	Addresses []Address
 }
 
+func (svc Service) hash(hash hash.Hash) {
+	hash.Write([]byte(svc.Name))
+	for _, addr := range svc.Addresses {
+		addr.hash(hash)
+	}
+}
+
+// interface for sorting []Service
+type ByName []Service
+
+func (a ByName) Len() int           { return len(a) }
+func (a ByName) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+func (a ByName) Less(i, j int) bool { return a[i].Name < a[j].Name }
+
 type Rule struct {
 	Host    string
 	PathMap map[string]Service
 }
 
+func (rule Rule) hash(hash hash.Hash) {
+	hash.Write([]byte(rule.Host))
+	paths := make([]string, len(rule.PathMap))
+	i := 0
+	for k := range rule.PathMap {
+		paths[i] = k
+		i++
+	}
+	sort.Strings(paths)
+	for _, p := range paths {
+		hash.Write([]byte(p))
+		rule.PathMap[p].hash(hash)
+	}
+}
+
+// interface for sorting []Rule
+type ByHost []Rule
+
+func (a ByHost) Len() int           { return len(a) }
+func (a ByHost) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+func (a ByHost) Less(i, j int) bool { return a[i].Host < a[j].Host }
+
 type Spec struct {
 	DefaultService Service
 	Rules          []Rule
 	AllServices    map[string]Service
 }
 
+func (spec Spec) DeepHash() uint64 {
+	hash := fnv.New64a()
+	spec.DefaultService.hash(hash)
+	for _, rule := range spec.Rules {
+		rule.hash(hash)
+	}
+	for k, v := range spec.AllServices {
+		hash.Write([]byte(k))
+		v.hash(hash)
+	}
+	return hash.Sum64()
+}
+
+func (spec Spec) Canonical() Spec {
+	canon := Spec{
+		DefaultService: Service{Name: spec.DefaultService.Name},
+		Rules:          make([]Rule, len(spec.Rules)),
+		AllServices:    make(map[string]Service, len(spec.AllServices)),
+	}
+	copy(canon.DefaultService.Addresses, spec.DefaultService.Addresses)
+	sort.Stable(ByIPPort(canon.DefaultService.Addresses))
+	copy(canon.Rules, spec.Rules)
+	sort.Stable(ByHost(canon.Rules))
+	for _, rule := range canon.Rules {
+		for _, svc := range rule.PathMap {
+			sort.Stable(ByIPPort(svc.Addresses))
+		}
+	}
+	for name, svcs := range spec.AllServices {
+		canon.AllServices[name] = svcs
+		sort.Stable(ByIPPort(canon.AllServices[name].Addresses))
+	}
+	return canon
+}
+
 var fMap = template.FuncMap{
 	"plusOne":   func(i int) int { return i + 1 },
 	"vclMangle": func(s string) string { return Mangle(s) },

cmd/varnish/vcl/template_test.go → cmd/varnish/vcl/vcl_test.go

@@ -65,6 +65,24 @@ var coffeeSvc = Service{
 	},
 }
 
+var coffeeSvc3 = Service{
+	Name: "coffee-svc",
+	Addresses: []Address{
+		{
+			IP:   "192.0.2.4",
+			Port: 80,
+		},
+		{
+			IP:   "192.0.2.5",
+			Port: 80,
+		},
+		{
+			IP:   "192.0.2.6",
+			Port: 80,
+		},
+	},
+}
+
 var cafeSpec = Spec{
 	DefaultService: Service{},
 	Rules: []Rule{{
@@ -80,6 +98,21 @@ var cafeSpec = Spec{
 	},
 }
 
+var cafeSpec2 = Spec{
+	DefaultService: Service{},
+	Rules: []Rule{{
+		Host: "cafe.example.com",
+		PathMap: map[string]Service{
+			"/tea":    teaSvc,
+			"/coffee": coffeeSvc3,
+		},
+	}},
+	AllServices: map[string]Service{
+		"tea-svc":    teaSvc,
+		"coffee-svc": coffeeSvc3,
+	},
+}
+
 func TestTemplate(t *testing.T) {
 	var buf bytes.Buffer
 	if err := Tmpl.Execute(&buf, cafeSpec); err != nil {
@@ -87,3 +120,13 @@ func TestTemplate(t *testing.T) {
 	}
 	t.Log(string(buf.Bytes()))
 }
+
+func TestDeepHash(t *testing.T) {
+	t.Log("cafeSpec.DeepHash():", cafeSpec.DeepHash())
+	t.Log("cafeSpec2.DeepHash():", cafeSpec2.DeepHash())
+	if cafeSpec.DeepHash() == cafeSpec2.DeepHash() {
+		t.Errorf("Distinct specs with different hashes: spec1=%+v "+
+			"spec2=%+v hash=%0x", cafeSpec, cafeSpec2,
+			cafeSpec.DeepHash())
+	}
+}