Skip to content

Commit

Permalink
[fix]: delete the NIC regardless of whether the Pod was found or not.
Browse files Browse the repository at this point in the history
Signed-off-by: liyh <[email protected]>
  • Loading branch information
liyh-yusur committed Sep 10, 2024
1 parent 1cf7e50 commit 9d1fa1d
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 53 deletions.
101 changes: 57 additions & 44 deletions pkg/daemon/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,10 @@ func (csh cniServerHandler) handleDel(req *restful.Request, resp *restful.Respon
return
}

// Try to get the Pod, but if it fails due to not being found, log a warning and continue.
pod, err := csh.Controller.podsLister.Pods(podRequest.PodNamespace).Get(podRequest.PodName)
if err != nil {
if k8serrors.IsNotFound(err) {
resp.WriteHeader(http.StatusNoContent)
return
}

errMsg := fmt.Errorf("parse del request failed %w", err)
if err != nil && !k8serrors.IsNotFound(err) {
errMsg := fmt.Errorf("failed to retrieve Pod %s/%s: %w", podRequest.PodNamespace, podRequest.PodName, err)
klog.Error(errMsg)
if err := resp.WriteHeaderAndEntity(http.StatusBadRequest, request.CniResponse{Err: errMsg.Error()}); err != nil {
klog.Errorf("failed to write response, %v", err)
Expand All @@ -456,56 +452,73 @@ func (csh cniServerHandler) handleDel(req *restful.Request, resp *restful.Respon
return
}

if pod.Annotations != nil && (util.IsOvnProvider(podRequest.Provider) || podRequest.CniType == util.CniTypeName) {
subnet := pod.Annotations[fmt.Sprintf(util.LogicalSwitchAnnotationTemplate, podRequest.Provider)]
if subnet != "" {
ip := pod.Annotations[fmt.Sprintf(util.IPAddressAnnotationTemplate, podRequest.Provider)]
if err = csh.Controller.removeEgressConfig(subnet, ip); err != nil {
errMsg := fmt.Errorf("failed to remove egress configuration: %w", err)
klog.Error(errMsg)
if err = resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: errMsg.Error()}); err != nil {
klog.Errorf("failed to write response, %v", err)
var nicType string
var vmName string

// If the Pod was found, process its annotations and labels.
if err == nil {
if pod.Annotations != nil && (util.IsOvnProvider(podRequest.Provider) || podRequest.CniType == util.CniTypeName) {
subnet := pod.Annotations[fmt.Sprintf(util.LogicalSwitchAnnotationTemplate, podRequest.Provider)]
if subnet != "" {
ip := pod.Annotations[fmt.Sprintf(util.IPAddressAnnotationTemplate, podRequest.Provider)]
if err = csh.Controller.removeEgressConfig(subnet, ip); err != nil {
errMsg := fmt.Errorf("failed to remove egress configuration: %w", err)
klog.Error(errMsg)
if err = resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: errMsg.Error()}); err != nil {
klog.Errorf("failed to write response, %v", err)
}
return
}
return
}
}

// For Support kubevirt hotplug dpdk nic, forbidden set the volume name
if podRequest.VhostUserSocketConsumption == util.ConsumptionKubevirt {
podRequest.VhostUserSocketVolumeName = util.VhostUserSocketVolumeName
}
switch {
case podRequest.DeviceID != "":
nicType = util.OffloadType
case podRequest.VhostUserSocketVolumeName != "":
nicType = util.DpdkType
if err = removeShortSharedDir(pod, podRequest.VhostUserSocketVolumeName, podRequest.VhostUserSocketConsumption); err != nil {
klog.Error(err.Error())
if err = resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: err.Error()}); err != nil {
klog.Errorf("failed to write response: %v", err)
}
return
}
default:
nicType = pod.Annotations[fmt.Sprintf(util.PodNicAnnotationTemplate, podRequest.Provider)]
}

var nicType string
vmName = pod.Annotations[fmt.Sprintf(util.VMAnnotationTemplate, podRequest.Provider)]
if vmName != "" {
podRequest.PodName = vmName
}
}
} else {
// If the Pod is not found, assign a default value.
klog.Warningf("Pod %s not found, proceeding with NIC deletion using ContainerID and NetNs", podRequest.PodName)
switch {
case podRequest.DeviceID != "":
nicType = util.OffloadType
case podRequest.VhostUserSocketVolumeName != "":
nicType = util.DpdkType
if err = removeShortSharedDir(pod, podRequest.VhostUserSocketVolumeName, podRequest.VhostUserSocketConsumption); err != nil {
klog.Error(err.Error())
if err = resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: err.Error()}); err != nil {
klog.Errorf("failed to write response: %v", err)
}
return
}
default:
nicType = pod.Annotations[fmt.Sprintf(util.PodNicAnnotationTemplate, podRequest.Provider)]
}
vmName := pod.Annotations[fmt.Sprintf(util.VMAnnotationTemplate, podRequest.Provider)]
if vmName != "" {
podRequest.PodName = vmName
nicType = "veth-pair"
}
}

err = csh.deleteNic(podRequest.PodName, podRequest.PodNamespace, podRequest.ContainerID, podRequest.NetNs, podRequest.DeviceID, podRequest.IfName, nicType, podRequest.Provider)
if err != nil {
errMsg := fmt.Errorf("del nic failed %w", err)
klog.Error(errMsg)
if err := resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: errMsg.Error()}); err != nil {
klog.Errorf("failed to write response, %v", err)
}
return
}
// For Support kubevirt hotplug dpdk nic, forbidden set the volume name
if podRequest.VhostUserSocketConsumption == util.ConsumptionKubevirt {
podRequest.VhostUserSocketVolumeName = util.VhostUserSocketVolumeName
}

// Proceed to delete the NIC regardless of whether the Pod was found or not.
err = csh.deleteNic(podRequest.PodName, podRequest.PodNamespace, podRequest.ContainerID, podRequest.NetNs, podRequest.DeviceID, podRequest.IfName, nicType)

Check failure on line 514 in pkg/daemon/handler.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

not enough arguments in call to csh.deleteNic
if err != nil {
errMsg := fmt.Errorf("del nic failed %w", err)
klog.Error(errMsg)
if err := resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: errMsg.Error()}); err != nil {
klog.Errorf("failed to write response, %v", err)
}
return
}
resp.WriteHeader(http.StatusNoContent)
}
13 changes: 4 additions & 9 deletions pkg/daemon/ovs_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func (csh cniServerHandler) configureNic(podName, podNamespace, provider, netns,
return finalRoutes, nil
}

func (csh cniServerHandler) releaseVf(podName, podNamespace, podNetns, ifName, nicType, provider, deviceID string) error {
func (csh cniServerHandler) releaseVf(podName, podNamespace, podNetns, ifName, nicType, deviceID string) error {
// Only for SRIOV case, we'd need to move the VF from container namespace back to the host namespace
if !(nicType == util.OffloadType && deviceID != "") {
return nil
Expand Down Expand Up @@ -245,12 +245,7 @@ func (csh cniServerHandler) releaseVf(podName, podNamespace, podNetns, ifName, n
return fmt.Errorf("failed to bring down container interface %s %s: %w", ifName, podDesc, err)
}
// rename VF device back to its original name in the host namespace:
pod, err := csh.Controller.podsLister.Pods(podNamespace).Get(podName)
if err != nil {
klog.Errorf("failed to get pod %s/%s: %v", podName, podNamespace, err)
return err
}
vfName := pod.Annotations[fmt.Sprintf(util.VfNameTemplate, provider)]
vfName := link.Attrs().Alias
if err = netlink.LinkSetName(link, vfName); err != nil {
return fmt.Errorf("failed to rename container interface %s to %s %s: %w",
ifName, vfName, podDesc, err)
Expand All @@ -270,8 +265,8 @@ func (csh cniServerHandler) releaseVf(podName, podNamespace, podNetns, ifName, n
return nil
}

func (csh cniServerHandler) deleteNic(podName, podNamespace, containerID, netns, deviceID, ifName, nicType, provider string) error {
if err := csh.releaseVf(podName, podNamespace, netns, ifName, nicType, provider, deviceID); err != nil {
func (csh cniServerHandler) deleteNic(podName, podNamespace, containerID, netns, deviceID, ifName, nicType string) error {
if err := csh.releaseVf(podName, podNamespace, netns, ifName, nicType, deviceID); err != nil {
return fmt.Errorf("failed to release VF %s assigned to the Pod %s/%s back to the host network namespace: "+
"%w", ifName, podName, podNamespace, err)
}
Expand Down

0 comments on commit 9d1fa1d

Please sign in to comment.