Save the Pod namespace/name in a vcl.Address when available.

This makes the naming for Varnish and IngressBackend instances more
legible, and more readily associated with the Pods on which they run,
than by using only network addresses.

First use the names in the controller logs.

pkg/controller/service.go

@@ -172,6 +172,10 @@ func epAddrs2VCLAddrs(
 			IP:   epAddr.IP,
 			Port: admPort,
 		}
+		if epAddr.TargetRef != nil {
+			vclAddr.PodNamespace = epAddr.TargetRef.Namespace
+			vclAddr.PodName = epAddr.TargetRef.Name
+		}
 		vclAddrs = append(vclAddrs, vclAddr)
 	}
 	return vclAddrs

pkg/controller/utils.go

@@ -105,6 +105,12 @@ func endpsTargetPort2Addrs(
 						IP:   address.IP,
 						Port: port.Port,
 					}
+					if address.TargetRef != nil {
+						addr.PodNamespace = address.
+							TargetRef.Namespace
+						addr.PodName = address.
+							TargetRef.Name
+					}
 					addrs = append(addrs, addr)
 				}
 				return addrs, update.MakeSuccess("")

pkg/varnish/monitor.go

@@ -72,12 +72,11 @@ func (vc *Controller) checkInst(svc string, inst *varnishInst) bool {
 	metrics := getInstanceMetrics(inst.addr)
 	metrics.monitorChecks.Inc()
 
-	vc.log.Infof("Monitoring Varnish instance %s (Service %s)", inst.addr,
-		svc)
+	vc.log.Infof("Monitoring Varnish instance %s (Service %s)", inst, svc)
 
 	if inst.admSecret == nil {
 		vc.warnEvt(svc, noAdmSecret,
-			"No admin secret known for endpoint %s", inst.addr)
+			"No admin secret known for endpoint %s", inst)
 		return false
 	}
 	inst.admMtx.Lock()
@@ -91,57 +90,55 @@ func (vc *Controller) checkInst(svc string, inst *varnishInst) bool {
 	if err != nil {
 		metrics.connectFails.Inc()
 		vc.errorEvt(svc, connectErr, "Error connecting to %s: %v",
-			inst.addr, err)
+			inst, err)
 		return false
 	}
 	defer adm.Close()
 	inst.Banner = adm.Banner
-	vc.log.Debugf("Connected to Varnish instance %s, banner: %s", inst.addr,
+	vc.log.Debugf("Connected to Varnish instance %s, banner: %s", inst,
 		adm.Banner)
 
 	pong, err := adm.Ping()
 	if err != nil {
 		metrics.pingFails.Inc()
-		vc.errorEvt(svc, pingErr, "Error pinging at %s: %v", inst.addr,
-			err)
+		vc.errorEvt(svc, pingErr, "Error pinging at %s: %v", inst, err)
 		return false
 	}
 	metrics.pings.Inc()
-	vc.log.Debugf("Succesfully pinged instance %s: %+v", inst.addr, pong)
+	vc.log.Debugf("Succesfully pinged instance %s: %+v", inst, pong)
 
 	state, err := adm.Status()
 	if err != nil {
 		vc.errorEvt(svc, statusErr, "Error getting status at %s: %v",
-			inst.addr, err)
+			inst, err)
 		return false
 	}
 	if state == admin.Running {
 		metrics.childRunning.Inc()
-		vc.log.Debugf("Status at %s: %s", inst.addr, state)
+		vc.log.Debugf("Status at %s: %s", inst, state)
 	} else {
 		metrics.childNotRunning.Inc()
-		vc.warnEvt(svc, statusNotRun, "Status at %s: %s", inst.addr,
-			state)
+		vc.warnEvt(svc, statusNotRun, "Status at %s: %s", inst, state)
 	}
 
 	panic, err := adm.GetPanic()
 	if err != nil {
 		vc.errorEvt(svc, panicErr, "Error getting panic at %s: %v",
-			inst.addr, err)
+			inst, err)
 		return false
 	}
 	if panic == "" {
-		vc.log.Debugf("No panic at %s", inst.addr)
+		vc.log.Debugf("No panic at %s", inst)
 	} else {
 		metrics.panics.Inc()
-		vc.errorEvt(svc, panic, "Panic at %s: %s", inst.addr, panic)
+		vc.errorEvt(svc, panic, "Panic at %s: %s", inst, panic)
 		// XXX clear the panic? Should be configurable
 	}
 
 	vcls, err := adm.VCLList()
 	if err != nil {
 		vc.errorEvt(svc, vclListErr,
-			"Error getting VCL list at %s: %v", inst.addr, err)
+			"Error getting VCL list at %s: %v", inst, err)
 		return false
 	}
 	for _, vcl := range vcls {
@@ -150,12 +147,11 @@ func (vc *Controller) checkInst(svc string, inst *varnishInst) bool {
 			if err = adm.VCLDiscard(vcl.Name); err != nil {
 				vc.errorEvt(svc, discardErr,
 					"Error discarding VCL %s at %s: "+
-						"%v", vcl.Name, inst.addr, err)
+						"%v", vcl.Name, inst, err)
 				return false
 			}
 			metrics.vclDiscards.Inc()
-			vc.log.Infof("Discarded VCL %s at %s", vcl.Name,
-				inst.addr)
+			vc.log.Infof("Discarded VCL %s at %s", vcl.Name, inst)
 		}
 	}
 	return true

pkg/varnish/varnish.go

@@ -69,15 +69,16 @@ var (
 // AdmError encapsulates an error encountered for an individual
 // Varnish instance, and satisfies the Error interface.
 type AdmError struct {
-	addr string
+	inst string
 	err  error
 }
 
 // Error returns an error meesage for an error encountered at a
-// Varnish instance, identifying the instance by its Endpoint address
-// (internal IP) and admin port.
+// Varnish instance, identifying the instance by its Pod
+// namespace/name if known, and its Endpoint address (internal IP) and
+// admin port.
 func (vadmErr AdmError) Error() string {
-	return fmt.Sprintf("%s: %v", vadmErr.addr, vadmErr.err)
+	return fmt.Sprintf("%s: %v", vadmErr.inst, vadmErr.err)
 }
 
 // AdmErrors is a collection of errors encountered at Varnish
@@ -176,12 +177,21 @@ func (spec vclSpec) configName() string {
 }
 
 type varnishInst struct {
+	ns        string
+	name      string
 	addr      string
 	admSecret *[]byte
 	Banner    string
 	admMtx    *sync.Mutex
 }
 
+func (inst varnishInst) String() string {
+	if inst.ns != "" && inst.name != "" {
+		return fmt.Sprintf("%s/%s(%s)", inst.ns, inst.name, inst.addr)
+	}
+	return inst.addr
+}
+
 type varnishSvc struct {
 	instances []*varnishInst
 	spec      *vclSpec
@@ -242,8 +252,8 @@ func (vc *Controller) Start() {
 func (vc *Controller) updateVarnishInstance(inst *varnishInst, cfgName string,
 	vclSrc string, metrics *instanceMetrics) error {
 
-	vc.log.Infof("Update Varnish instance at %s", inst.addr)
-	vc.log.Tracef("Varnish instance %s: %+v", inst.addr, *inst)
+	vc.log.Infof("Update Varnish instance at %s", inst)
+	vc.log.Tracef("Varnish instance %s: %+v", inst, *inst)
 	if inst.admSecret == nil {
 		return fmt.Errorf("No known admin secret")
 	}
@@ -252,7 +262,7 @@ func (vc *Controller) updateVarnishInstance(inst *varnishInst, cfgName string,
 	vc.wg.Add(1)
 	defer vc.wg.Done()
 
-	vc.log.Tracef("Connect to %s, timeout=%v", inst.addr, admTimeout)
+	vc.log.Tracef("Connect to %s, timeout=%v", inst, admTimeout)
 	timer := prometheus.NewTimer(metrics.connectLatency)
 	adm, err := admin.Dial(inst.addr, *inst.admSecret, admTimeout)
 	timer.ObserveDuration()
@@ -262,15 +272,15 @@ func (vc *Controller) updateVarnishInstance(inst *varnishInst, cfgName string,
 	}
 	defer adm.Close()
 	inst.Banner = adm.Banner
-	vc.log.Infof("Connected to Varnish admin endpoint at %s", inst.addr)
+	vc.log.Infof("Connected to Varnish admin endpoint at %s", inst)
 
 	loaded, labelled, ready := false, false, false
-	vc.log.Tracef("List VCLs at %s", inst.addr)
+	vc.log.Tracef("List VCLs at %s", inst)
 	vcls, err := adm.VCLList()
 	if err != nil {
 		return err
 	}
-	vc.log.Tracef("VCL List at %s: %+v", inst.addr, vcls)
+	vc.log.Tracef("VCL List at %s: %+v", inst, vcls)
 	for _, vcl := range vcls {
 		if vcl.Name == cfgName {
 			loaded = true
@@ -287,15 +297,15 @@ func (vc *Controller) updateVarnishInstance(inst *varnishInst, cfgName string,
 
 	if loaded {
 		vc.log.Infof("Config %s already loaded at instance %s", cfgName,
-			inst.addr)
+			inst)
 	} else {
-		vc.log.Tracef("Load config %s at %s", cfgName, inst.addr)
+		vc.log.Tracef("Load config %s at %s", cfgName, inst)
 		timer = prometheus.NewTimer(metrics.vclLoadLatency)
 		err = adm.VCLInline(cfgName, vclSrc)
 		timer.ObserveDuration()
 		if err != nil {
 			vc.log.Tracef("Error loading config %s at %s: %v",
-				cfgName, inst.addr, err)
+				cfgName, inst, err)
 			metrics.vclLoadErrs.Inc()
 			if resp, ok := err.(admin.UnexpectedResponse); ok {
 				if resp.Response.Code < admin.OK {
@@ -306,35 +316,35 @@ func (vc *Controller) updateVarnishInstance(inst *varnishInst, cfgName string,
 		}
 		metrics.vclLoads.Inc()
 		vc.log.Infof("Loaded config %s at Varnish endpoint %s", cfgName,
-			inst.addr)
+			inst)
 	}
 
 	if labelled {
 		vc.log.Infof("Config %s already labelled as regular at %s",
-			cfgName, inst.addr)
+			cfgName, inst)
 	} else {
 		vc.log.Tracef("Label config %s as %s at %s", cfgName,
-			regularLabel, inst.addr)
+			regularLabel, inst)
 		err = adm.VCLLabel(regularLabel, cfgName)
 		if err != nil {
 			return err
 		}
 		vc.log.Infof("Labeled config %s as %s at Varnish endpoint %s",
-			cfgName, regularLabel, inst.addr)
+			cfgName, regularLabel, inst)
 	}
 
 	if ready {
 		vc.log.Infof("Config %s already labelled as ready at %s",
-			readyCfg, inst.addr)
+			readyCfg, inst)
 	} else {
 		vc.log.Tracef("Label config %s as %s at %s", readyCfg,
-			readinessLabel, inst.addr)
+			readinessLabel, inst)
 		err = adm.VCLLabel(readinessLabel, readyCfg)
 		if err != nil {
 			return err
 		}
 		vc.log.Infof("Labeled config %s as %s at Varnish endpoint %s",
-			readyCfg, readinessLabel, inst.addr)
+			readyCfg, readinessLabel, inst)
 	}
 	return nil
 }
@@ -375,7 +385,7 @@ func (vc *Controller) updateVarnishSvc(name string) error {
 		if e := vc.updateVarnishInstance(inst, cfgName, vclSrc,
 			metrics); e != nil {
 
-			admErr := AdmError{addr: inst.addr, err: e}
+			admErr := AdmError{inst: inst.String(), err: e}
 			errs = append(errs, admErr)
 			metrics.updateErrs.Inc()
 			continue
@@ -396,7 +406,7 @@ func (vc *Controller) setCfgLabel(inst *varnishInst, cfg, lbl string,
 
 	if inst.admSecret == nil {
 		return AdmError{
-			addr: inst.addr,
+			inst: inst.String(),
 			err:  fmt.Errorf("No known admin secret"),
 		}
 	}
@@ -406,32 +416,30 @@ func (vc *Controller) setCfgLabel(inst *varnishInst, cfg, lbl string,
 	vc.wg.Add(1)
 	defer vc.wg.Done()
 
-	vc.log.Tracef("Connect to %s, timeout=%v", inst.addr, admTimeout)
+	vc.log.Tracef("Connect to %s, timeout=%v", inst, admTimeout)
 	timer := prometheus.NewTimer(metrics.connectLatency)
 	adm, err := admin.Dial(inst.addr, *inst.admSecret, admTimeout)
 	timer.ObserveDuration()
 	if err != nil {
 		if mayClose {
-			vc.log.Warnf("Could not connect to %s: %v", inst.addr,
-				err)
+			vc.log.Warnf("Could not connect to %s: %v", inst, err)
 			return nil
 		}
 		metrics.connectFails.Inc()
-		return AdmError{addr: inst.addr, err: err}
+		return AdmError{inst: inst.String(), err: err}
 	}
 	defer adm.Close()
 	inst.Banner = adm.Banner
-	vc.log.Infof("Connected to Varnish admin endpoint at %s", inst.addr)
+	vc.log.Infof("Connected to Varnish admin endpoint at %s", inst)
 
-	vc.log.Tracef("Set config %s to label %s at %s", inst.addr, cfg, lbl)
+	vc.log.Tracef("Set config %s to label %s at %s", inst, cfg, lbl)
 	if err := adm.VCLLabel(lbl, cfg); err != nil {
 		if err == io.EOF {
 			if mayClose {
-				vc.log.Warnf("Connection at EOF at %s",
-					inst.addr)
+				vc.log.Warnf("Connection at EOF at %s", inst)
 				return nil
 			}
-			return AdmError{addr: inst.addr, err: err}
+			return AdmError{inst: inst.String(), err: err}
 		}
 	}
 	return nil
@@ -461,7 +469,7 @@ func (vc *Controller) removeVarnishInstances(insts []*varnishInst) error {
 			if vc.ignorePermNetErr(err) {
 				continue
 			}
-			admErr := AdmError{addr: inst.addr, err: err}
+			admErr := AdmError{inst: inst.String(), err: err}
 			errs = append(errs, admErr)
 			continue
 		}
@@ -485,22 +493,24 @@ func (vc *Controller) updateVarnishSvcAddrs(key string, addrs []vcl.Address,
 		return fmt.Errorf("No known Varnish Service %s", key)
 	}
 
-	updateAddrs := make(map[string]struct{})
+	updateAddrs := make(map[string]vcl.Address)
 	prevAddrs := make(map[string]*varnishInst)
 	for _, addr := range addrs {
-		key := addr.String()
-		updateAddrs[key] = struct{}{}
+		key := addr.Address()
+		updateAddrs[key] = addr
 	}
 	for _, inst := range svc.instances {
 		prevAddrs[inst.addr] = inst
 	}
-	for addr := range updateAddrs {
+	for addr, vclAddr := range updateAddrs {
 		inst, exists := prevAddrs[addr]
 		if exists {
 			keepInsts = append(keepInsts, inst)
 			continue
 		}
 		newInst := &varnishInst{
+			ns:        vclAddr.PodNamespace,
+			name:      vclAddr.PodName,
 			addr:      addr,
 			admSecret: secrPtr,
 			admMtx:    &sync.Mutex{},
@@ -525,7 +535,7 @@ func (vc *Controller) updateVarnishSvcAddrs(key string, addrs []vcl.Address,
 		if err := vc.setCfgLabel(inst, notAvailCfg, readinessLabel,
 			true); err != nil {
 
-			admErr := AdmError{addr: inst.addr, err: err}
+			admErr := AdmError{inst: inst.String(), err: err}
 			errs = append(errs, admErr)
 			continue
 		}
@@ -572,8 +582,10 @@ func (vc *Controller) AddOrUpdateVarnishSvc(
 		var instances []*varnishInst
 		svc = &varnishSvc{}
 		for _, addr := range addrs {
-			admAddr := addr.String()
+			admAddr := addr.Address()
 			instance := &varnishInst{
+				ns:     addr.PodNamespace,
+				name:   addr.PodName,
 				addr:   admAddr,
 				admMtx: &sync.Mutex{},
 			}
@@ -681,7 +693,9 @@ func (vc *Controller) Update(
 		}
 		for i, addr := range addrs {
 			svc.instances[i] = &varnishInst{
-				addr:   addr.String(),
+				ns:     addr.PodNamespace,
+				name:   addr.PodName,
+				addr:   addr.Address(),
 				admMtx: &sync.Mutex{},
 			}
 		}
@@ -737,7 +751,7 @@ func (vc *Controller) SetNotReady(svcKey string) error {
 				continue
 			}
 			admErr := AdmError{
-				addr: inst.addr,
+				inst: inst.String(),
 				err:  err,
 			}
 			errs = append(errs, admErr)
@@ -790,7 +804,7 @@ func (vc *Controller) HasConfig(svcKey string, addrs []vcl.Address,
 	}
 	newAddrs := make(map[string]struct{})
 	for _, addr := range addrs {
-		a := addr.String()
+		a := addr.Address()
 		newAddrs[a] = struct{}{}
 	}
 	curAddrs := make(map[string]struct{})
@@ -872,7 +886,7 @@ func (vc *Controller) UpdateSvcForSecret(
 	svc.secrName = secretKey
 
 	for _, inst := range svc.instances {
-		vc.log.Infof("Setting secret for instance %s", inst.addr)
+		vc.log.Infof("Setting secret for instance %s", inst)
 		inst.admSecret = secret
 	}
 

pkg/varnish/varnish_test.go

@@ -38,11 +38,11 @@ import (
 
 func TestAdmError(t *testing.T) {
 	vadmErr := AdmError{
-		addr: "123.45.67.89:4711",
+		inst: "default/viking-svc-4711(123.45.67.89:4711)",
 		err:  fmt.Errorf("Error message"),
 	}
 	err := vadmErr.Error()
-	want := "123.45.67.89:4711: Error message"
+	want := "default/viking-svc-4711(123.45.67.89:4711): Error message"
 	if err != want {
 		t.Errorf("AdmError.Error() want=%s got=%s", want, err)
 	}
@@ -50,16 +50,17 @@ func TestAdmError(t *testing.T) {
 	vadmErrs := AdmErrors{
 		vadmErr,
 		AdmError{
-			addr: "98.76.54.321:815",
+			inst: "kube-system/viking-svc-815(98.76.54.321:815)",
 			err:  fmt.Errorf("Error 2"),
 		},
 		AdmError{
-			addr: "192.0.2.255:80",
+			inst: "192.0.2.255:80",
 			err:  fmt.Errorf("Error 3"),
 		},
 	}
 	err = vadmErrs.Error()
-	want = "[{123.45.67.89:4711: Error message}{98.76.54.321:815: Error 2}" +
+	want = "[{default/viking-svc-4711(123.45.67.89:4711): Error message}" +
+		"{kube-system/viking-svc-815(98.76.54.321:815): Error 2}" +
 		"{192.0.2.255:80: Error 3}]"
 	if err != want {
 		t.Errorf("AdmErrors.Error() want=%s got=%s", want, err)

pkg/varnish/vcl/shard_test.go

@@ -36,16 +36,31 @@ import (
 var varnishCluster = ShardCluster{
 	Nodes: []Service{
 		{
-			Name:      "varnish-8445d4f7f-z2b9p",
-			Addresses: []Address{{"172.17.0.12", 80}},
+			Name: "varnish-8445d4f7f-z2b9p",
+			Addresses: []Address{{
+				PodNamespace: "default",
+				PodName:      "varnish-8445d4f7f-z2b9p",
+				IP:           "172.17.0.12",
+				Port:         80,
+			}},
 		},
 		{
-			Name:      "varnish-8445d4f7f-k22dn",
-			Addresses: []Address{{"172.17.0.13", 80}},
+			Name: "varnish-8445d4f7f-k22dn",
+			Addresses: []Address{{
+				PodNamespace: "default",
+				PodName:      "varnish-8445d4f7f-k22dn",
+				IP:           "172.17.0.13",
+				Port:         80,
+			}},
 		},
 		{
-			Name:      "varnish-8445d4f7f-ldljf",
-			Addresses: []Address{{"172.17.0.14", 80}},
+			Name: "varnish-8445d4f7f-ldljf",
+			Addresses: []Address{{
+				PodNamespace: "default",
+				PodName:      "varnish-8445d4f7f-ldljf",
+				IP:           "172.17.0.14",
+				Port:         80,
+			}},
 		},
 	},
 	Probe: Probe{

pkg/varnish/vcl/spec.go

@@ -48,17 +48,30 @@ func hashUint16(u16 uint16, hash hash.Hash) {
 // (Endpoint of a Service to which requests are routed) or a Varnish
 // instance (where the port is the admin port).
 type Address struct {
-	IP   string
-	Port int32
+	PodNamespace string
+	PodName      string
+	IP           string
+	Port         int32
 }
 
-func (addr Address) String() string {
+// Address returns the network address represented by addr as
+// "<IP>:<Port>"
+func (addr Address) Address() string {
 	return addr.IP + ":" + strconv.Itoa(int(addr.Port))
 }
 
+func (addr Address) String() string {
+	if addr.PodNamespace != "" && addr.PodName != "" {
+		return addr.PodNamespace + "/" + addr.PodName
+	}
+	return addr.Address()
+}
+
 func (addr Address) hash(hash hash.Hash) {
 	portBytes := make([]byte, 4)
 	binary.BigEndian.PutUint32(portBytes, uint32(addr.Port))
+	hash.Write([]byte(addr.PodNamespace))
+	hash.Write([]byte(addr.PodName))
 	hash.Write([]byte(addr.IP))
 	hash.Write(portBytes)
 }