Skip to content

Commit

Permalink
Return the error from ipamd to plugin (aws#688)
Browse files Browse the repository at this point in the history
  • Loading branch information
Claes Mogren committed Dec 15, 2019
1 parent f0e89c2 commit 33342f9
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 11 deletions.
9 changes: 1 addition & 8 deletions ipamd/rpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,7 @@ func (s *server) DelNetwork(ctx context.Context, in *pb.DelNetworkRequest) (*pb.
}
log.Infof("Send DelNetworkReply: IPv4Addr %s, DeviceNumber: %d, err: %v", ip, deviceNumber, err)

// Plugins should generally complete a DEL action without error even if some resources are missing. For example,
// an IPAM plugin should generally release an IP allocation and return success even if the container network
// namespace no longer exists, unless that network namespace is critical for IPAM management
success := true
if err != nil && err != datastore.ErrUnknownPod {
success = false
}
return &pb.DelNetworkReply{Success: success, IPv4Addr: ip, DeviceNumber: int32(deviceNumber)}, nil
return &pb.DelNetworkReply{Success: err == nil, IPv4Addr: ip, DeviceNumber: int32(deviceNumber)}, err
}

// RunRPCHandler handles request from gRPC
Expand Down
19 changes: 16 additions & 3 deletions plugins/routed-eni/cni.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"os"
"runtime"

"github.com/aws/amazon-vpc-cni-k8s/ipamd/datastore"

"github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils"

"golang.org/x/net/context"
Expand Down Expand Up @@ -282,9 +284,17 @@ func del(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap
Reason: "PodDeleted"})

if err != nil {
log.Errorf("Error received from DelNetwork grpc call for pod %s namespace %s sandbox %s: %v",
string(k8sArgs.K8S_POD_NAME), string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_INFRA_CONTAINER_ID), err)
return err
if err == datastore.ErrUnknownPod {
// Plugins should generally complete a DEL action without error even if some resources are missing. For example,
// an IPAM plugin should generally release an IP allocation and return success even if the container network
// namespace no longer exists, unless that network namespace is critical for IPAM management
log.Infof("Pod %s in namespace %s not found", string(k8sArgs.K8S_POD_NAME), string(k8sArgs.K8S_POD_NAMESPACE))
return nil
} else {
log.Errorf("Error received from DelNetwork grpc call for pod %s namespace %s sandbox %s: %v",
string(k8sArgs.K8S_POD_NAME), string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_INFRA_CONTAINER_ID), err)
return err
}
}

if !r.Success {
Expand All @@ -305,6 +315,9 @@ func del(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap
string(k8sArgs.K8S_POD_NAME), string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_INFRA_CONTAINER_ID), err)
return err
}
} else {
log.Warnf("Pod %s in namespace %s did not have a valid IP %s", string(k8sArgs.K8S_POD_NAME),
string(k8sArgs.K8S_POD_NAMESPACE), r.IPv4Addr)
}
return nil
}
Expand Down

0 comments on commit 33342f9

Please sign in to comment.