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.

Sign up for GitHub

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

Jump to bottom

cni-plugin: Clean up code #26505

Merged
merged 4 commits into from
Jun 27, 2023
Merged

cni-plugin: Clean up code #26505

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd8bf4c
cni: Use more idiomatic error handling
gandro Jun 27, 2023
1bc8c3d
cni: Remove unused variable
gandro Jun 27, 2023
2a9bc7e
cni: Declare variables in a more idiomatic manner
gandro Jun 27, 2023
160d1ef
cni: Remove unused struct field
gandro Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 30 additions & 56 deletions plugins/cilium-cni/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func init() {
}

type CmdState struct {
Endpoint *models.EndpointChangeRequest
IP6 netip.Addr
IP6routes []route.Route
IP4 netip.Addr
Expand Down Expand Up @@ -358,25 +357,13 @@ func setupLogging(n *types.NetConf) error {
}

func cmdAdd(args *skel.CmdArgs) (err error) {
var (
ipConfig *cniTypesV1.IPConfig
routes []*cniTypes.Route
ipam *models.IPAMResponse
n *types.NetConf
c *client.Client
netNs ns.NetNS
conf *models.DaemonConfigurationStatus
)

n, err = types.LoadNetConf(args.StdinData)
n, err := types.LoadNetConf(args.StdinData)
if err != nil {
err = fmt.Errorf("unable to parse CNI configuration \"%s\": %s", args.StdinData, err)
return
return fmt.Errorf("unable to parse CNI configuration \"%s\": %s", args.StdinData, err)
}

if innerErr := setupLogging(n); innerErr != nil {
err = fmt.Errorf("unable to setup logging: %w", innerErr)
return
if err = setupLogging(n); err != nil {
return fmt.Errorf("unable to setup logging: %w", err)
}

logger := log.WithField("eventUUID", uuid.New())
Expand All @@ -397,15 +384,13 @@ func cmdAdd(args *skel.CmdArgs) (err error) {

cniArgs := types.ArgsSpec{}
if err = cniTypes.LoadArgs(args.Args, &cniArgs); err != nil {
err = fmt.Errorf("unable to extract CNI arguments: %s", err)
return
return fmt.Errorf("unable to extract CNI arguments: %s", err)
}
logger.Debugf("CNI Args: %#v", cniArgs)

c, err = client.NewDefaultClientWithTimeout(defaults.ClientConnectTimeout)
c, err := client.NewDefaultClientWithTimeout(defaults.ClientConnectTimeout)
if err != nil {
err = fmt.Errorf("unable to connect to Cilium daemon: %s", client.Hint(err))
return
return fmt.Errorf("unable to connect to Cilium daemon: %s", client.Hint(err))
}

// If CNI ADD gives us a PrevResult, we're a chained plugin and *must* detect a
Expand Down Expand Up @@ -440,26 +425,23 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
}
}

netNs, err = ns.GetNS(args.Netns)
netNs, err := ns.GetNS(args.Netns)
if err != nil {
err = fmt.Errorf("failed to open netns %q: %s", args.Netns, err)
return
return fmt.Errorf("failed to open netns %q: %s", args.Netns, err)
}
defer netNs.Close()

if err = netns.RemoveIfFromNetNSIfExists(netNs, args.IfName); err != nil {
err = fmt.Errorf("failed removing interface %q from namespace %q: %s",
return fmt.Errorf("failed removing interface %q from namespace %q: %s",
args.IfName, args.Netns, err)
return
}

addLabels := models.Labels{}

conf, err = getConfigFromCiliumAgent(c)
conf, err := getConfigFromCiliumAgent(c)
if err != nil {
return
return err
}

var ipam *models.IPAMResponse
var releaseIPsFunc func(context.Context)
if conf.IpamMode == ipamOption.IPAMDelegatedPlugin {
ipam, releaseIPsFunc, err = allocateIPsWithDelegatedPlugin(context.TODO(), conf, n, args.StdinData)
Expand All @@ -475,17 +457,16 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
}()

if err != nil {
return
return err
}

if err = connector.SufficientAddressing(ipam.HostAddressing); err != nil {
err = fmt.Errorf("IP allocation addressing in insufficient: %s", err)
return
return fmt.Errorf("IP allocation addressing in insufficient: %s", err)
}

ep := &models.EndpointChangeRequest{
ContainerID: args.ContainerID,
Labels: addLabels,
Labels: models.Labels{},
State: models.EndpointStateWaitingDashForDashIdentity.Pointer(),
Addressing: &models.AddressPair{},
K8sPodName: string(cniArgs.K8S_POD_NAME),
Expand All @@ -509,8 +490,7 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
int(conf.GROMaxSize), int(conf.GSOMaxSize),
int(conf.GROIPV4MaxSize), int(conf.GSOIPV4MaxSize), ep)
if err != nil {
err = fmt.Errorf("unable to set up veth on host side: %s", err)
return err
return fmt.Errorf("unable to set up veth on host side: %s", err)
}
defer func() {
if err != nil {
Expand All @@ -521,39 +501,38 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
}()

if err = netlink.LinkSetNsFd(peer, int(netNs.Fd())); err != nil {
err = fmt.Errorf("unable to move veth pair '%v' to netns: %s", peer, err)
return
return fmt.Errorf("unable to move veth pair '%v' to netns: %s", peer, err)
}

_, _, err = connector.SetupVethRemoteNs(netNs, tmpIfName, args.IfName)
if err != nil {
err = fmt.Errorf("unable to set up veth on container side: %s", err)
return
return fmt.Errorf("unable to set up veth on container side: %s", err)
}
}

state := CmdState{
Endpoint: ep,
Client: c,
HostAddr: ipam.HostAddressing,
}

res := &cniTypesV1.Result{}

if !ipv6IsEnabled(ipam) && !ipv4IsEnabled(ipam) {
err = fmt.Errorf("IPAM did not provide IPv4 or IPv6 address")
return
return fmt.Errorf("IPAM did not provide IPv4 or IPv6 address")
}

var (
ipConfig *cniTypesV1.IPConfig
routes []*cniTypes.Route
)
if ipv6IsEnabled(ipam) {
ep.Addressing.IPV6 = ipam.Address.IPV6
ep.Addressing.IPV6PoolName = ipam.Address.IPV6PoolName
ep.Addressing.IPV6ExpirationUUID = ipam.IPV6.ExpirationUUID

ipConfig, routes, err = prepareIP(ep.Addressing.IPV6, &state, int(conf.RouteMTU))
if err != nil {
err = fmt.Errorf("unable to prepare IP addressing for '%s': %s", ep.Addressing.IPV6, err)
return
return fmt.Errorf("unable to prepare IP addressing for '%s': %s", ep.Addressing.IPV6, err)
}
res.IPs = append(res.IPs, ipConfig)
res.Routes = append(res.Routes, routes...)
Expand All @@ -566,8 +545,7 @@ func cmdAdd(args *skel.CmdArgs) (err error) {

ipConfig, routes, err = prepareIP(ep.Addressing.IPV4, &state, int(conf.RouteMTU))
if err != nil {
err = fmt.Errorf("unable to prepare IP addressing for '%s': %s", ep.Addressing.IPV4, err)
return
return fmt.Errorf("unable to prepare IP addressing for '%s': %s", ep.Addressing.IPV4, err)
}
res.IPs = append(res.IPs, ipConfig)
res.Routes = append(res.Routes, routes...)
Expand All @@ -577,8 +555,7 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
case ipamOption.IPAMENI, ipamOption.IPAMAzure, ipamOption.IPAMAlibabaCloud:
err = interfaceAdd(ipConfig, ipam.IPV4, conf)
if err != nil {
err = fmt.Errorf("unable to setup interface datapath: %s", err)
return
return fmt.Errorf("unable to setup interface datapath: %s", err)
}
}

Expand All @@ -592,8 +569,7 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
macAddrStr, err = configureIface(ipam, args.IfName, &state)
return err
}); err != nil {
err = fmt.Errorf("unable to configure interfaces in container namespace: %s", err)
return
return fmt.Errorf("unable to configure interfaces in container namespace: %s", err)
}

res.Interfaces = append(res.Interfaces, &cniTypesV1.Interface{
Expand All @@ -612,8 +588,7 @@ func cmdAdd(args *skel.CmdArgs) (err error) {
if err = c.EndpointCreate(ep); err != nil {
logger.WithError(err).WithFields(logrus.Fields{
logfields.ContainerID: ep.ContainerID}).Warn("Unable to create endpoint")
err = fmt.Errorf("unable to create endpoint: %s", err)
return
return fmt.Errorf("unable to create endpoint: %s", err)
}

logger.WithFields(logrus.Fields{
Expand All @@ -631,8 +606,7 @@ func cmdDel(args *skel.CmdArgs) error {
// are guaranteed to be recoverable.
n, err := types.LoadNetConf(args.StdinData)
if err != nil {
err = fmt.Errorf("unable to parse CNI configuration \"%s\": %s", args.StdinData, err)
return err
return fmt.Errorf("unable to parse CNI configuration \"%s\": %s", args.StdinData, err)
}

if err := setupLogging(n); err != nil {
Expand Down