Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor network commands #4757

Merged
merged 1 commit into from
Dec 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 15 additions & 49 deletions pkg/adapter/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,10 @@ func (r *LocalRuntime) NetworkInspect(cli *cliconfig.NetworkInspectValues) error
rawCNINetworks []map[string]interface{}
)
for _, name := range cli.InputArgs {
b, err := network.ReadRawCNIConfByName(name)
rawList, err := network.InspectNetwork(name)
if err != nil {
return err
}
rawList := make(map[string]interface{})
if err := json.Unmarshal(b, &rawList); err != nil {
return fmt.Errorf("error parsing configuration list: %s", err)
}
rawCNINetworks = append(rawCNINetworks, rawList)
}
out, err := json.MarshalIndent(rawCNINetworks, "", "\t")
Expand All @@ -98,7 +94,20 @@ func (r *LocalRuntime) NetworkRemove(ctx context.Context, cli *cliconfig.Network
if err != nil {
return networkRmSuccesses, networkRmErrors, err
}
if err := r.removeNetwork(ctx, name, containers, cli.Force); err != nil {
// We need to iterate containers looking to see if they belong to the given network
for _, c := range containers {
if util.StringInSlice(name, c.Config().Networks) {
// if user passes force, we nuke containers
if !cli.Force {
// Without the force option, we return an error
return nil, nil, errors.Errorf("%q has associated containers with it. Use -f to forcibly delete containers", name)
}
if err := r.RemoveContainer(ctx, c.Container, true, true); err != nil {
return nil, nil, err
}
}
}
if err := network.RemoveNetwork(name); err != nil {
if lastError != nil {
networkRmErrors[name] = lastError
}
Expand All @@ -110,49 +119,6 @@ func (r *LocalRuntime) NetworkRemove(ctx context.Context, cli *cliconfig.Network
return networkRmSuccesses, networkRmErrors, lastError
}

// removeNetwork removes a single network and its containers given a force bool
func (r *LocalRuntime) removeNetwork(ctx context.Context, name string, containers []*Container, force bool) error {
cniPath, err := network.GetCNIConfigPathByName(name)
if err != nil {
return err
}
// We need to iterate containers looking to see if they belong to the given network
for _, c := range containers {
if util.StringInSlice(name, c.Config().Networks) {
// if user passes force, we nuke containers
if force {
if err := r.RemoveContainer(ctx, c.Container, true, true); err != nil {
return err
}
} else {
// Without the the force option, we return an error
return errors.Errorf("%q has associated containers with it. use -f to forcibly delete containers", name)
}

}
}
// Before we delete the configuration file, we need to make sure we can read and parse
// it to get the network interface name so we can remove that too
interfaceName, err := network.GetInterfaceNameFromConfig(cniPath)
if err != nil {
return errors.Wrapf(err, "failed to find network interface name in %q", cniPath)
}
liveNetworkNames, err := network.GetLiveNetworkNames()
if err != nil {
return errors.Wrapf(err, "failed to get live network names")
}
if util.StringInSlice(interfaceName, liveNetworkNames) {
if err := network.RemoveInterface(interfaceName); err != nil {
return errors.Wrapf(err, "failed to delete the network interface %q", interfaceName)
}
}
// Remove the configuration file
if err := os.Remove(cniPath); err != nil {
return errors.Wrapf(err, "failed to remove network configuration file %q", cniPath)
}
return nil
}

// NetworkCreateBridge creates a CNI network
func (r *LocalRuntime) NetworkCreateBridge(cli *cliconfig.NetworkCreateValues) (string, error) {
isGateway := true
Expand Down
44 changes: 43 additions & 1 deletion pkg/network/network.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package network

import (
"github.com/containers/libpod/pkg/util"
"encoding/json"
"net"
"os"

"github.com/containernetworking/cni/pkg/types"
"github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator"
"github.com/containers/libpod/pkg/util"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -148,3 +150,43 @@ func ValidateUserNetworkIsAvailable(userNet *net.IPNet) error {
}
return nil
}

// RemoveNetwork removes a given network by name. If the network has container associated with it, that
// must be handled outside the context of this.
func RemoveNetwork(name string) error {
cniPath, err := GetCNIConfigPathByName(name)
if err != nil {
return err
}
// Before we delete the configuration file, we need to make sure we can read and parse
// it to get the network interface name so we can remove that too
interfaceName, err := GetInterfaceNameFromConfig(cniPath)
if err != nil {
return errors.Wrapf(err, "failed to find network interface name in %q", cniPath)
}
liveNetworkNames, err := GetLiveNetworkNames()
if err != nil {
return errors.Wrapf(err, "failed to get live network names")
}
if util.StringInSlice(interfaceName, liveNetworkNames) {
if err := RemoveInterface(interfaceName); err != nil {
return errors.Wrapf(err, "failed to delete the network interface %q", interfaceName)
}
}
// Remove the configuration file
if err := os.Remove(cniPath); err != nil {
return errors.Wrapf(err, "failed to remove network configuration file %q", cniPath)
}
return nil
}

// InspectNetwork reads a CNI config and returns its configuration
func InspectNetwork(name string) (map[string]interface{}, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we return anything better than this? I know that CNI doesn't bother exporting most types, but can we potentially provide some sort of struct defining what contents could be?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is really too loosely defined to do much more. I don't like it either.

b, err := ReadRawCNIConfByName(name)
if err != nil {
return nil, err
}
rawList := make(map[string]interface{})
err = json.Unmarshal(b, &rawList)
return rawList, err
}