Add tests for network aliases

As part of this, we need two new functions, for retrieving all
aliases for a network and removing all aliases for a network,
both required to test.

Also, rework handling for some things the tests discovered were
broken (notably conflicts between container name and existing
aliases).

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This commit is contained in:
Matthew Heon
2020-10-29 14:27:36 -04:00
committed by Matthew Heon
parent 63efde15f1
commit 844d540d04
5 changed files with 534 additions and 10 deletions

View File

@ -1021,7 +1021,7 @@ func (s *BoltState) GetNetworkAliases(ctr *Container, network string) ([]string,
inNetwork := ctrNetworkBkt.Get([]byte(network))
if inNetwork == nil {
return errors.Wrapf(define.ErrNoAliases, "container %s is not part of network %s, no aliases found")
return errors.Wrapf(define.ErrNoAliases, "container %s is not part of network %s, no aliases found", ctr.ID(), network)
}
ctrAliasesBkt := dbCtr.Bucket(aliasesBkt)
@ -1103,12 +1103,14 @@ func (s *BoltState) GetAllNetworkAliases(ctr *Container) (map[string][]string, e
}
netAliases := []string{}
aliases[string(network)] = netAliases
return netAliasesBkt.ForEach(func(alias, v []byte) error {
_ = netAliasesBkt.ForEach(func(alias, v []byte) error {
netAliases = append(netAliases, string(alias))
return nil
})
aliases[string(network)] = netAliases
return nil
})
})
if err != nil {
@ -1314,6 +1316,119 @@ func (s *BoltState) RemoveNetworkAliases(ctr *Container, network string) error {
})
}
// Get all network aliases for a single CNI network. Returns a map of alias to
// container ID.
func (s *BoltState) GetAllAliasesForNetwork(network string) (map[string]string, error) {
if !s.valid {
return nil, define.ErrDBClosed
}
if network == "" {
return nil, errors.Wrapf(define.ErrInvalidArg, "network name must not be empty")
}
db, err := s.getDBCon()
if err != nil {
return nil, err
}
defer s.deferredCloseDBCon(db)
aliases := make(map[string]string)
err = db.View(func(tx *bolt.Tx) error {
aliasBucket, err := getAliasesBucket(tx)
if err != nil {
return err
}
dbAlias := aliasBucket.Bucket([]byte(network))
if dbAlias == nil {
// We can't tell if the network exists, or doesn't exist
// So... Assume it exists, but has no aliases.
return nil
}
return dbAlias.ForEach(func(alias, ctrId []byte) error {
aliases[string(alias)] = string(ctrId)
return nil
})
})
if err != nil {
return nil, err
}
return aliases, nil
}
// RemoveAllAliasesForNetwork removes all the aliases in a given CNI network, as
// part of that network being removed.
func (s *BoltState) RemoveAllAliasesForNetwork(network string) error {
if !s.valid {
return define.ErrDBClosed
}
if network == "" {
return errors.Wrapf(define.ErrInvalidArg, "network names must not be empty")
}
db, err := s.getDBCon()
if err != nil {
return err
}
defer s.deferredCloseDBCon(db)
return db.Update(func(tx *bolt.Tx) error {
allCtrsBucket, err := getAllCtrsBucket(tx)
if err != nil {
return err
}
ctrBucket, err := getCtrBucket(tx)
if err != nil {
return err
}
allAliasesBucket, err := getAliasesBucket(tx)
if err != nil {
return err
}
checkAliasesBucketExists := allAliasesBucket.Bucket([]byte(network))
if checkAliasesBucketExists != nil {
if err := allAliasesBucket.DeleteBucket([]byte(network)); err != nil {
return errors.Wrapf(err, "error removing network %s aliases bucket from DB", network)
}
}
// Iterate through all containers and remove their aliases
// bucket for the network.
return allCtrsBucket.ForEach(func(ctrID, ctrName []byte) error {
dbCtr := ctrBucket.Bucket(ctrID)
if dbCtr == nil {
// DB State is inconsistent... but we can't do
// anything about it.
// Log and move on.
logrus.Errorf("Container %s listed in all containers, but has no bucket!", string(ctrID))
return nil
}
dbCtrAliases := dbCtr.Bucket(aliasesBkt)
if dbCtrAliases == nil {
// Container has no aliases, this is OK.
return nil
}
ctrNetAliases := dbCtrAliases.Bucket([]byte(network))
if ctrNetAliases != nil {
if err := dbCtrAliases.DeleteBucket([]byte(network)); err != nil {
return errors.Wrapf(err, "error removing bucket for network aliases for network %s from container %s", network, string(ctrID))
}
}
return nil
})
})
}
// GetContainerConfig returns a container config from the database by full ID
func (s *BoltState) GetContainerConfig(id string) (*ContainerConfig, error) {
if len(id) == 0 {

View File

@ -634,6 +634,13 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
return errors.Wrapf(err, "name \"%s\" is in use", ctr.Name())
}
// Check that we don't have any empty network names
for _, net := range ctr.config.Networks {
if net == "" {
return errors.Wrapf(define.ErrInvalidArg, "network names cannot be an empty string")
}
}
// If we have network aliases, check if they are already in use.
for net, aliases := range ctr.config.NetworkAliases {
// Aliases cannot conflict with container names.
@ -682,6 +689,51 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
return errors.Wrapf(err, "error adding container %s to all containers bucket in DB", ctr.ID())
}
// Check aliases for all networks, remove conflicts with the
// container name.
for _, net := range ctr.config.Networks {
netAliasesBkt := allAliasesBkt.Bucket([]byte(net))
if netAliasesBkt == nil {
continue
}
otherCtrID := netAliasesBkt.Get(ctrName)
if otherCtrID == nil {
continue
}
if err := netAliasesBkt.Delete(ctrName); err != nil {
return errors.Wrapf(err, "error removing container %s name from network aliases for network %s", ctr.ID(), net)
}
// We now need to remove from the other container.
// To do this, we work through the container bucket,
// then its aliases bucket, then its aliases for this
// specific network, then we remove the alias.
// Just slightly ridiculous. Just slightly.
otherCtr := ctrBucket.Bucket(otherCtrID)
if otherCtr == nil {
// The state is inconsistent, but we can't do
// much...
logrus.Errorf("Container %s referred to by network alias but not present in state", string(otherCtrID))
continue
}
otherCtrAliases := otherCtr.Bucket(aliasesBkt)
if otherCtrAliases == nil {
logrus.Errorf("Container %s is missing aliases but but has an alias", string(otherCtrID))
continue
}
otherCtrNetworkAliases := otherCtrAliases.Bucket([]byte(net))
if otherCtrNetworkAliases == nil {
logrus.Errorf("Container %s is missing network aliases bucket for network %s but has alias in that network", string(otherCtrID), net)
}
if otherCtrNetworkAliases.Get(ctrName) != nil {
if err := otherCtrNetworkAliases.Delete(ctrName); err != nil {
return errors.Wrapf(err, "error removing container %s name from network %s aliases of container %s", ctr.Name(), net, string(otherCtrID))
}
}
}
for net, aliases := range ctr.config.NetworkAliases {
netAliasesBkt, err := allAliasesBkt.CreateBucketIfNotExists([]byte(net))
if err != nil {
@ -689,13 +741,7 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
}
for _, alias := range aliases {
if err := netAliasesBkt.Put([]byte(alias), ctrID); err != nil {
return errors.Wrapf(err, "error adding container %s network aliasa %q to network %q", ctr.ID(), alias, net)
}
}
// If the container's name is present in the aliases - remove it.
if namePresent := netAliasesBkt.Get(ctrName); namePresent != nil {
if err := netAliasesBkt.Delete(ctrName); err != nil {
return errors.Wrapf(err, "error cleaning container name %q from network %q aliases", ctr.Name(), net)
return errors.Wrapf(err, "error adding container %s network alias %q to network %q", ctr.ID(), alias, net)
}
}
}

View File

@ -285,6 +285,13 @@ func (s *InMemoryState) AddContainer(ctr *Container) error {
return err
}
// Check networks
for _, net := range ctr.config.Networks {
if net == "" {
return errors.Wrapf(define.ErrInvalidArg, "network names cannot be empty")
}
}
// Check network aliases
for network, aliases := range ctr.config.NetworkAliases {
inNet := false
@ -301,6 +308,10 @@ func (s *InMemoryState) AddContainer(ctr *Container) error {
allNetAliases, ok := s.networkAliases[network]
if ok {
for _, alias := range aliases {
// Check if alias is a name
if _, err := s.nameIndex.Get(alias); err == nil {
return define.ErrInvalidArg
}
if _, ok := allNetAliases[alias]; ok {
return define.ErrAliasExists
}
@ -364,6 +375,34 @@ func (s *InMemoryState) AddContainer(ctr *Container) error {
s.addCtrToVolDependsMap(ctr.ID(), vol.Name)
}
for _, network := range ctr.config.Networks {
allNetAliases, ok := s.networkAliases[network]
if !ok {
continue
}
otherCtrID, ok := allNetAliases[ctr.Name()]
if !ok {
continue
}
delete(allNetAliases, ctr.Name())
otherCtrAliases, ok := s.ctrNetworkAliases[otherCtrID]
if !ok {
continue
}
otherCtrNetAliases, ok := otherCtrAliases[network]
if !ok {
continue
}
newAliases := []string{}
for _, alias := range otherCtrNetAliases {
if alias != ctr.Name() {
newAliases = append(newAliases, alias)
}
}
otherCtrAliases[network] = newAliases
}
// Add network aliases
for network, aliases := range ctr.config.NetworkAliases {
allNetAliases, ok := s.networkAliases[network]
@ -696,6 +735,41 @@ func (s *InMemoryState) RemoveNetworkAliases(ctr *Container, network string) err
return nil
}
// GetAllAliasesForNetwork gets all the aliases for a single network.
func (s *InMemoryState) GetAllAliasesForNetwork(network string) (map[string]string, error) {
if network == "" {
return nil, errors.Wrapf(define.ErrInvalidArg, "network names must not be empty")
}
allAliases, ok := s.networkAliases[network]
if !ok {
// Can't tell if the network exists.
// Assume it does.
return map[string]string{}, nil
}
return allAliases, nil
}
// RemoveAllAliasesForNetwork removes all the aliases for a given network.
func (s *InMemoryState) RemoveAllAliasesForNetwork(network string) error {
if network == "" {
return errors.Wrapf(define.ErrInvalidArg, "network names must not be empty")
}
if _, ok := s.networkAliases[network]; ok {
delete(s.networkAliases, network)
}
for _, ctrAliases := range s.ctrNetworkAliases {
if _, ok := ctrAliases[network]; ok {
delete(ctrAliases, network)
}
}
return nil
}
// GetContainerConfig returns a container config from the database by full ID
func (s *InMemoryState) GetContainerConfig(id string) (*ContainerConfig, error) {
ctr, err := s.LookupContainer(id)
@ -1340,6 +1414,13 @@ func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error {
return err
}
// Check networks
for _, net := range ctr.config.Networks {
if net == "" {
return errors.Wrapf(define.ErrInvalidArg, "network names cannot be empty")
}
}
// Check network aliases
for network, aliases := range ctr.config.NetworkAliases {
inNet := false
@ -1356,6 +1437,10 @@ func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error {
allNetAliases, ok := s.networkAliases[network]
if ok {
for _, alias := range aliases {
// Check if alias is a name
if _, err := s.nameIndex.Get(alias); err == nil {
return define.ErrInvalidArg
}
if _, ok := allNetAliases[alias]; ok {
return define.ErrAliasExists
}
@ -1440,6 +1525,34 @@ func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error {
s.addCtrToVolDependsMap(ctr.ID(), vol.Name)
}
for _, network := range ctr.config.Networks {
allNetAliases, ok := s.networkAliases[network]
if !ok {
continue
}
otherCtrID, ok := allNetAliases[ctr.Name()]
if !ok {
continue
}
delete(allNetAliases, ctr.Name())
otherCtrAliases, ok := s.ctrNetworkAliases[otherCtrID]
if !ok {
continue
}
otherCtrNetAliases, ok := otherCtrAliases[network]
if !ok {
continue
}
newAliases := []string{}
for _, alias := range otherCtrNetAliases {
if alias != ctr.Name() {
newAliases = append(newAliases, alias)
}
}
otherCtrAliases[network] = newAliases
}
// Add network aliases
for network, aliases := range ctr.config.NetworkAliases {
allNetAliases, ok := s.networkAliases[network]

View File

@ -106,6 +106,12 @@ type State interface {
SetNetworkAliases(ctr *Container, network string, aliases []string) error
// Remove network aliases for the given container in the given network.
RemoveNetworkAliases(ctr *Container, network string) error
// GetAllAliasesForNetwork returns all the aliases for a given
// network. Returns a map of alias to container ID.
GetAllAliasesForNetwork(network string) (map[string]string, error)
// RemoveAllAliasesForNetwork removes all the aliases for a given
// network.
RemoveAllAliasesForNetwork(network string) error
// Return a container config from the database by full ID
GetContainerConfig(id string) (*ContainerConfig, error)

View File

@ -1319,6 +1319,250 @@ func TestCannotUsePodAsDependency(t *testing.T) {
})
}
func TestAddContainerEmptyNetworkNameErrors(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr, err := getTestCtr1(manager)
assert.NoError(t, err)
testCtr.config.Networks = []string{""}
err = state.AddContainer(testCtr)
assert.Error(t, err)
})
}
func TestAddContainerNetworkAliasesButNoMatchingNetwork(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr, err := getTestCtr1(manager)
assert.NoError(t, err)
testCtr.config.Networks = []string{"test1"}
testCtr.config.NetworkAliases = make(map[string][]string)
testCtr.config.NetworkAliases["test2"] = []string{"alias1"}
err = state.AddContainer(testCtr)
assert.Error(t, err)
})
}
func TestAddContainerNetworkAliasConflictWithName(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr1, err := getTestCtr1(manager)
assert.NoError(t, err)
netName := "testnet"
testCtr1.config.Networks = []string{netName}
testCtr1.config.NetworkAliases = make(map[string][]string)
testCtr1.config.NetworkAliases[netName] = []string{"alias1"}
testCtr2, err := getTestCtr2(manager)
assert.NoError(t, err)
testCtr2.config.Networks = []string{netName}
testCtr2.config.NetworkAliases = make(map[string][]string)
testCtr2.config.NetworkAliases[netName] = []string{testCtr1.Name()}
err = state.AddContainer(testCtr1)
assert.NoError(t, err)
err = state.AddContainer(testCtr2)
assert.Error(t, err)
})
}
func TestAddContainerNetworkAliasConflictWithAlias(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr1, err := getTestCtr1(manager)
assert.NoError(t, err)
netName := "testnet"
aliasName := "alias1"
testCtr1.config.Networks = []string{netName}
testCtr1.config.NetworkAliases = make(map[string][]string)
testCtr1.config.NetworkAliases[netName] = []string{aliasName}
testCtr2, err := getTestCtr2(manager)
assert.NoError(t, err)
testCtr2.config.Networks = []string{netName}
testCtr2.config.NetworkAliases = make(map[string][]string)
testCtr2.config.NetworkAliases[netName] = []string{aliasName}
err = state.AddContainer(testCtr1)
assert.NoError(t, err)
err = state.AddContainer(testCtr2)
assert.Error(t, err)
})
}
func TestAddContainerNetworkAliasConflictWithAliasButDifferentNets(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr1, err := getTestCtr1(manager)
assert.NoError(t, err)
netName := "testnet"
aliasName := "alias1"
testCtr1.config.Networks = []string{netName}
testCtr1.config.NetworkAliases = make(map[string][]string)
testCtr1.config.NetworkAliases[netName] = []string{aliasName}
testCtr2, err := getTestCtr2(manager)
assert.NoError(t, err)
netName2 := "testnet2"
testCtr2.config.Networks = []string{netName2}
testCtr2.config.NetworkAliases = make(map[string][]string)
testCtr2.config.NetworkAliases[netName2] = []string{aliasName}
err = state.AddContainer(testCtr1)
assert.NoError(t, err)
err = state.AddContainer(testCtr2)
assert.NoError(t, err)
})
}
func TestAddContainerNameConflictsWithAliasRemovesAlias(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr1, err := getTestCtr1(manager)
assert.NoError(t, err)
testCtr2, err := getTestCtr2(manager)
assert.NoError(t, err)
netName := "testnet"
aliasName := testCtr2.Name()
testCtr1.config.Networks = []string{netName}
testCtr1.config.NetworkAliases = make(map[string][]string)
testCtr1.config.NetworkAliases[netName] = []string{aliasName}
testCtr2.config.Networks = []string{netName}
err = state.AddContainer(testCtr1)
assert.NoError(t, err)
err = state.AddContainer(testCtr2)
assert.NoError(t, err)
aliases, err := state.GetNetworkAliases(testCtr1, netName)
assert.NoError(t, err)
assert.Equal(t, 0, len(aliases))
})
}
func TestNetworkAliasAddAndRemoveSingleContainer(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr, err := getTestCtr1(manager)
assert.NoError(t, err)
netName := "testnet"
testCtr.config.Networks = []string{netName}
testCtr.config.NetworkAliases = make(map[string][]string)
testCtr.config.NetworkAliases[netName] = []string{"alias1"}
startAliases, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 0, len(startAliases))
err = state.AddContainer(testCtr)
assert.NoError(t, err)
oneAlias, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 1, len(oneAlias))
assert.Equal(t, testCtr.ID(), oneAlias["alias1"])
allAliases, err := state.GetAllNetworkAliases(testCtr)
assert.NoError(t, err)
assert.Equal(t, 1, len(allAliases))
netAliases, ok := allAliases[netName]
assert.True(t, ok)
assert.Equal(t, 1, len(netAliases))
assert.Equal(t, "alias1", netAliases[0])
ctrNetAliases, err := state.GetNetworkAliases(testCtr, netName)
assert.NoError(t, err)
assert.Equal(t, 1, len(ctrNetAliases))
assert.Equal(t, "alias1", ctrNetAliases[0])
err = state.RemoveContainer(testCtr)
assert.NoError(t, err)
noAliases, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 0, len(noAliases))
})
}
func TestNetworkAliasAddAndRemoveTwoContainers(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr1, err := getTestCtr1(manager)
assert.NoError(t, err)
netName := "testnet"
testCtr1.config.Networks = []string{netName}
testCtr1.config.NetworkAliases = make(map[string][]string)
testCtr1.config.NetworkAliases[netName] = []string{"alias1"}
testCtr2, err := getTestCtr2(manager)
assert.NoError(t, err)
testCtr2.config.Networks = []string{netName}
testCtr2.config.NetworkAliases = make(map[string][]string)
testCtr2.config.NetworkAliases[netName] = []string{"alias2"}
startAliases, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 0, len(startAliases))
err = state.AddContainer(testCtr1)
assert.NoError(t, err)
oneAlias, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 1, len(oneAlias))
assert.Equal(t, testCtr1.ID(), oneAlias["alias1"])
err = state.AddContainer(testCtr2)
assert.NoError(t, err)
twoAliases, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 2, len(twoAliases))
assert.Equal(t, testCtr1.ID(), twoAliases["alias1"])
assert.Equal(t, testCtr2.ID(), twoAliases["alias2"])
allAliases, err := state.GetAllNetworkAliases(testCtr1)
assert.NoError(t, err)
assert.Equal(t, 1, len(allAliases))
netAliases, ok := allAliases[netName]
assert.True(t, ok)
assert.Equal(t, 1, len(netAliases))
assert.Equal(t, "alias1", netAliases[0])
ctrNetAliases, err := state.GetNetworkAliases(testCtr1, netName)
assert.NoError(t, err)
assert.Equal(t, 1, len(ctrNetAliases))
assert.Equal(t, "alias1", ctrNetAliases[0])
err = state.RemoveContainer(testCtr2)
assert.NoError(t, err)
oneAlias, err = state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 1, len(oneAlias))
assert.Equal(t, testCtr1.ID(), oneAlias["alias1"])
err = state.RemoveContainer(testCtr1)
assert.NoError(t, err)
noAliases, err := state.GetAllAliasesForNetwork(netName)
assert.NoError(t, err)
assert.Equal(t, 0, len(noAliases))
})
}
func TestCannotUseBadIDAsDependency(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) {
testCtr, err := getTestCtr1(manager)