diff options
author | Matthew Heon <matthew.heon@pm.me> | 2020-10-29 14:27:36 -0400 |
---|---|---|
committer | Matthew Heon <mheon@redhat.com> | 2020-11-03 16:26:42 -0500 |
commit | 844d540d042fe7477e62f8cf28e524fcd756f6c8 (patch) | |
tree | 42ff7cfba8d1216e56e7791560c21230333411d1 | |
parent | 63efde15f14b18f5af385e89936b4ab0868cb357 (diff) | |
download | podman-844d540d042fe7477e62f8cf28e524fcd756f6c8.tar.gz podman-844d540d042fe7477e62f8cf28e524fcd756f6c8.tar.bz2 podman-844d540d042fe7477e62f8cf28e524fcd756f6c8.zip |
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>
-rw-r--r-- | libpod/boltdb_state.go | 121 | ||||
-rw-r--r-- | libpod/boltdb_state_internal.go | 60 | ||||
-rw-r--r-- | libpod/in_memory_state.go | 113 | ||||
-rw-r--r-- | libpod/state.go | 6 | ||||
-rw-r--r-- | libpod/state_test.go | 244 |
5 files changed, 534 insertions, 10 deletions
diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 4a94d9090..0b9b353c7 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -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 { diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 1ecf99661..d5c755a52 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -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) } } } diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 3b88c9664..ba4c70c6b 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -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] diff --git a/libpod/state.go b/libpod/state.go index 6b178ec58..183f773b5 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -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) diff --git a/libpod/state_test.go b/libpod/state_test.go index 373feb6e0..cf41270bf 100644 --- a/libpod/state_test.go +++ b/libpod/state_test.go @@ -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) |