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

[fix]: delete the NIC regardless of whether the Pod was found or not. #4500

Merged
merged 6 commits into from
Sep 19, 2024
Merged

[fix]: delete the NIC regardless of whether the Pod was found or not. #4500

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4076da6
[fix]: delete the NIC regardless of whether the Pod was found or not.
liyh-yusur Sep 10, 2024
db59083
windows Remove Parameter
liyh-yusur Sep 10, 2024
383cc0c
Merge branch 'kubeovn:master' into master
liyh-yusur Sep 14, 2024
a505c5d
-Using constants
Sep 14, 2024
efe9d4f
Restore changes to Windows
Sep 18, 2024
6a7fd73
Fix: Correct argument count for csh.deleteNic call
Sep 18, 2024
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
101 changes: 57 additions & 44 deletions pkg/daemon/handler.go
View file
Open in desktop
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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

感觉像上面那样用个常量，用 nicType = util.VethType 会更好些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经改成了常量

}
}

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)
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion pkg/daemon/ovs_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (csh cniServerHandler) removeDefaultRoute(netns string, ipv4, ipv6 bool) er
return nil
}

func (csh cniServerHandler) deleteNic(podName, podNamespace, containerID, netns, deviceID, ifName, nicType, _ string) error {
func (csh cniServerHandler) deleteNic(podName, podNamespace, containerID, netns, deviceID, ifName, nicType string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个参数在 windows 系统没有用到不需要修改

Copy link
Contributor Author

Choose a reason for hiding this comment

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

此处已恢复到更改前代码

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"not enough arguments in call to csh.deleteNic", 前面的函数调用需要，这里得去掉最后一个参数了

epName := hns.ConstructEndpointName(containerID, netns, util.HnsNetwork)[:12]
// remove ovs port
output, err := ovs.Exec(ovs.IfExists, "--with-iface", "del-port", "br-int", epName)
Expand Down
Loading