host affinity backwards compatiblity (#241)

* host affinity backward compatibility

lib/backend/compat/compat.go

@@ -15,6 +15,8 @@
 package compat
 
 import (
+	goerrors "errors"
+
 	"github.com/projectcalico/libcalico-go/lib/backend/api"
 	"github.com/projectcalico/libcalico-go/lib/backend/model"
 	"github.com/projectcalico/libcalico-go/lib/errors"
@@ -58,6 +60,16 @@ func (c *ModelAdaptor) Create(d *model.KVPair) (*model.KVPair, error) {
 		}
 		d.Revision = p.Revision
 		return d, nil
+	case model.BlockKey:
+		if err = validateBlockValue(d); err != nil {
+			return nil, err
+		}
+		b, err := c.client.Create(d)
+		if err != nil {
+			return nil, err
+		}
+		d.Revision = b.Revision
+		return d, nil
 	default:
 		return c.client.Create(d)
 	}
@@ -90,6 +102,16 @@ func (c *ModelAdaptor) Update(d *model.KVPair) (*model.KVPair, error) {
 		}
 		d.Revision = p.Revision
 		return d, nil
+	case model.BlockKey:
+		if err = validateBlockValue(d); err != nil {
+			return nil, err
+		}
+		b, err := c.client.Update(d)
+		if err != nil {
+			return nil, err
+		}
+		d.Revision = b.Revision
+		return d, nil
 	default:
 		return c.client.Update(d)
 	}
@@ -122,6 +144,16 @@ func (c *ModelAdaptor) Apply(d *model.KVPair) (*model.KVPair, error) {
 		}
 		d.Revision = p.Revision
 		return d, nil
+	case model.BlockKey:
+		if err = validateBlockValue(d); err != nil {
+			return nil, err
+		}
+		b, err := c.client.Apply(d)
+		if err != nil {
+			return nil, err
+		}
+		d.Revision = b.Revision
+		return d, nil
 	default:
 		return c.client.Apply(d)
 	}
@@ -152,6 +184,8 @@ func (c *ModelAdaptor) Get(k model.Key) (*model.KVPair, error) {
 		return c.getProfile(k)
 	case model.NodeKey:
 		return c.getNode(kt)
+	case model.BlockKey:
+		return c.getBlock(k)
 	default:
 		return c.client.Get(k)
 	}
@@ -163,6 +197,8 @@ func (c *ModelAdaptor) List(l model.ListInterface) ([]*model.KVPair, error) {
 	switch lt := l.(type) {
 	case model.NodeListOptions:
 		return c.listNodes(lt)
+	case model.BlockListOptions:
+		return c.listBlock(lt)
 	default:
 		return c.client.List(l)
 	}
@@ -199,6 +235,23 @@ func (c *ModelAdaptor) getProfile(k model.Key) (*model.KVPair, error) {
 	return &d, nil
 }
 
+// getBlock gets KVPair for Block. It gets the block value first,
+// then checks for `Affinity` field first, then `HostAffinity` as a backup.
+// For more details see: https://github.com/projectcalico/libcalico-go/issues/226
+func (c *ModelAdaptor) getBlock(k model.Key) (*model.KVPair, error) {
+	bk := k.(model.BlockKey)
+
+	v, err := c.client.Get(model.BlockKey{CIDR: bk.CIDR})
+	if err != nil {
+		return nil, err
+	}
+
+	// Make sure Affinity field has a proper value,
+	// and map the value to Affinity if the deprecated HostAffinity field is used
+	// by calling ensureBlockAffinity, and update the KVPair to return.
+	return ensureBlockAffinity(v), nil
+}
+
 // getNode gets the composite node by getting the individual components
 // and joining the results together.
 func (c *ModelAdaptor) getNode(nk model.NodeKey) (*model.KVPair, error) {
@@ -219,6 +272,15 @@ func (c *ModelAdaptor) getNode(nk model.NodeKey) (*model.KVPair, error) {
 	return &model.KVPair{Key: nk, Value: &nv}, nil
 }
 
+// validateBlockValue validates the AllocationBlock fields (specifically Affinity) to
+// make sure the deprecated HostAffinity field is not used.
+func validateBlockValue(kvp *model.KVPair) error {
+	if kvp.Value.(*model.AllocationBlock).HostAffinity != nil {
+		return goerrors.New("AllocationBlock.HostAffinity is deprecated, please use Affinity instead.")
+	}
+	return nil
+}
+
 // listNodes lists the composite node resources by listing the primary node
 // object and then getting the remaining components through additional queries.
 // Note that enumeration of the primary component is horribly inefficient
@@ -251,6 +313,47 @@ func (c *ModelAdaptor) listNodes(l model.NodeListOptions) ([]*model.KVPair, erro
 	return results, nil
 }
 
+// listBlock returns list of KVPairs for Block, includes making sure
+// backwards compatiblity. See getBlock for more details.
+func (c *ModelAdaptor) listBlock(l model.BlockListOptions) ([]*model.KVPair, error) {
+
+	// Get a list of block KVPairs.
+	blockList, err := c.client.List(l)
+	if err != nil {
+		return nil, err
+	}
+
+	// Create an empty slice of KVPair.
+	results := make([]*model.KVPair, len(blockList))
+
+	// Go through the list to make sure Affinity field has a proper value,
+	// and maps the value to Affinity if the deprecated HostAffinity field is used
+	// by calling ensureBlockAffinity, and populate the KVPair slice to return.
+	for i, bkv := range blockList {
+		results[i] = ensureBlockAffinity(bkv)
+	}
+
+	return results, nil
+}
+
+// ensureBlockAffinity ensures Affinity field has a proper value,
+// and maps the value to Affinity if the deprecated HostAffinity field is used.
+func ensureBlockAffinity(kvp *model.KVPair) *model.KVPair {
+	val := kvp.Value.(*model.AllocationBlock)
+
+	// Check for `Affinity` field first (this is to make sure we're
+	// compatible with Python version etcd data-model).
+	if val.Affinity == nil && val.HostAffinity != nil {
+		// Convert HostAffinity=hostname into Affinity=host:hostname format.
+		hostAffinityStr := "host:" + *val.HostAffinity
+		val.Affinity = &hostAffinityStr
+
+		// Set AllocationBlock.HostAffinity to nil so it's never non-nil for the clients.
+		val.HostAffinity = nil
+	}
+	return &model.KVPair{Key: kvp.Key, Value: val}
+}
+
 // Get the node sub components and fill in the details in the supplied node
 // struct.
 func (c *ModelAdaptor) getNodeSubcomponents(nk model.NodeKey, nv *model.Node) error {

lib/backend/model/block.go

@@ -81,11 +81,16 @@ func (options BlockListOptions) KeyFromDefaultPath(path string) Key {
 
 type AllocationBlock struct {
 	CIDR           net.IPNet             `json:"cidr"`
-	HostAffinity   *string               `json:"hostAffinity"`
+	Affinity       *string               `json:"affinity"`
 	StrictAffinity bool                  `json:"strictAffinity"`
 	Allocations    []*int                `json:"allocations"`
 	Unallocated    []int                 `json:"unallocated"`
 	Attributes     []AllocationAttribute `json:"attributes"`
+
+	// HostAffinity is deprecated in favor of Affinity.
+	// This is only to keep compatiblity with existing deployments.
+	// The data format should be `Affinity: host:hostname` (not `hostAffinity: hostname`).
+	HostAffinity *string `json:"hostAffinity,omitempty"`
 }
 
 type AllocationAttribute struct {

lib/client/ipam.go

@@ -423,7 +423,7 @@ func (c ipams) releaseIPsFromBlock(ips []net.IP, blockCIDR net.IPNet) ([]net.IP,
 		// the Value since we have updated the structure pointed to in the
 		// KVPair.
 		var updateErr error
-		if b.empty() && b.HostAffinity == nil {
+		if b.empty() && b.Affinity == nil {
 			log.Debugf("Deleting non-affine block '%s'", b.CIDR.String())
 			updateErr = c.client.backend.Delete(obj)
 		} else {
@@ -486,7 +486,7 @@ func (c ipams) assignFromExistingBlock(
 
 		// Update the block using CAS by passing back the original
 		// KVPair.
-		obj.Value = &b.AllocationBlock
+		obj.Value = b.AllocationBlock
 		_, err = c.client.backend.Update(obj)
 		if err != nil {
 			log.Infof("Failed to update block '%s' - try again", b.CIDR.String())
@@ -760,7 +760,7 @@ func (c ipams) releaseByHandle(handleID string, blockCIDR net.IPNet) error {
 			return nil
 		}
 
-		if block.empty() && block.HostAffinity == nil {
+		if block.empty() && block.Affinity == nil {
 			err = c.client.backend.Delete(&model.KVPair{
 				Key: model.BlockKey{blockCIDR},
 			})

lib/client/ipam_block.go

@@ -77,9 +77,9 @@ func (b *allocationBlock) autoAssign(
 
 	// Determine if we need to check for affinity.
 	checkAffinity := b.StrictAffinity || affinityCheck
-	if checkAffinity && b.HostAffinity != nil && host != *b.HostAffinity {
+	if checkAffinity && b.Affinity != nil && !hostAffinityMatches(host, b.AllocationBlock) {
 		// Affinity check is enabled but the host does not match - error.
-		s := fmt.Sprintf("Block affinity (%s) does not match provided (%s)", *b.HostAffinity, host)
+		s := fmt.Sprintf("Block affinity (%s) does not match provided (%s)", *b.Affinity, host)
 		return nil, errors.New(s)
 	}
 
@@ -103,7 +103,7 @@ func (b *allocationBlock) autoAssign(
 }
 
 func (b *allocationBlock) assign(address cnet.IP, handleID *string, attrs map[string]string, host string) error {
-	if b.StrictAffinity && b.HostAffinity != nil && host != *b.HostAffinity {
+	if b.StrictAffinity && b.Affinity != nil && !hostAffinityMatches(host, b.AllocationBlock) {
 		// Affinity check is enabled but the host does not match - error.
 		return errors.New("Block host affinity does not match")
 	}
@@ -133,6 +133,11 @@ func (b *allocationBlock) assign(address cnet.IP, handleID *string, attrs map[st
 	return nil
 }
 
+// hostAffinityMatches checks if the provided host matches the provided affinity.
+func hostAffinityMatches(host string, block *model.AllocationBlock) bool {
+	return *block.Affinity == "host:"+host
+}
+
 func (b allocationBlock) numFreeAddresses() int {
 	return len(b.Unallocated)
 }

lib/client/ipam_block_reader_writer.go

@@ -137,13 +137,20 @@ func (rw blockReaderWriter) claimBlockAffinity(subnet cnet.IPNet, host string, c
 
 	// Create the new block.
 	block := newBlock(subnet)
-	block.HostAffinity = &host
+
+	// Make sure hostname is not empty.
+	if host == "" {
+		log.Errorf("Hostname can't be empty")
+		return goerrors.New("Hostname must be sepcified to claim block affinity")
+	}
+	affinityKeyStr := "host:" + host
+	block.Affinity = &affinityKeyStr
 	block.StrictAffinity = config.StrictAffinity
 
 	// Create the new block in the datastore.
 	o := model.KVPair{
 		Key:   model.BlockKey{block.CIDR},
-		Value: &block.AllocationBlock,
+		Value: block.AllocationBlock,
 	}
 	_, err = rw.client.backend.Create(&o)
 	if err != nil {
@@ -159,7 +166,7 @@ func (rw blockReaderWriter) claimBlockAffinity(subnet cnet.IPNet, host string, c
 			// Pull out the allocationBlock object.
 			b := allocationBlock{obj.Value.(*model.AllocationBlock)}
 
-			if b.HostAffinity != nil && *b.HostAffinity == host {
+			if b.Affinity != nil && *b.Affinity == affinityKeyStr {
 				// Block has affinity to this host, meaning another
 				// process on this host claimed it.
 				log.Debugf("Block %s already claimed by us.  Success", subnet)
@@ -194,9 +201,15 @@ func (rw blockReaderWriter) releaseBlockAffinity(host string, blockCIDR cnet.IPN
 		}
 		b := allocationBlock{obj.Value.(*model.AllocationBlock)}
 
+		// Make sure hostname is not empty.
+		if host == "" {
+			log.Errorf("Hostname can't be empty")
+			return goerrors.New("Hostname must be sepcified to release block affinity")
+		}
+
 		// Check that the block affinity matches the given affinity.
-		if b.HostAffinity != nil && *b.HostAffinity != host {
-			log.Errorf("Mismatched affinity: %s != %s", *b.HostAffinity, host)
+		if b.Affinity != nil && !hostAffinityMatches(host, b.AllocationBlock) {
+			log.Errorf("Mismatched affinity: %s != %s", *b.Affinity, "host:"+host)
 			return affinityClaimedError{Block: b}
 		}
 
@@ -217,7 +230,7 @@ func (rw blockReaderWriter) releaseBlockAffinity(host string, blockCIDR cnet.IPN
 			// This prevents the host from automatically assigning
 			// from this block unless we're allowed to overflow into
 			// non-affine blocks.
-			b.HostAffinity = nil
+			b.Affinity = nil
 
 			// Pass back the original KVPair with the new
 			// block information so we can do a CAS.

lib/client/ipam_errors.go

@@ -48,5 +48,5 @@ type affinityClaimedError struct {
 }
 
 func (e affinityClaimedError) Error() string {
-	return fmt.Sprintf("%s already claimed by %s", e.Block.CIDR, e.Block.HostAffinity)
+	return fmt.Sprintf("%s already claimed by %s", e.Block.CIDR, e.Block.Affinity)
 }