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

Merge handleAddPod with handleUpdatePod. #2563

Merged
merged 1 commit into from
Apr 3, 2023
Merged

Merge handleAddPod with handleUpdatePod. #2563

merged 1 commit into from
Apr 3, 2023

Conversation

xujunjie-cover
Copy link
Member

ValidatePodCidr and create lsp only in handleAddPod before;
For hotplug and hotunplug nic for a running pod. we also need support when update pod.
so merge handleAddPod with handleUpdatePod to handleAddPodOrUpdatePod.

What type of this PR

Examples of user facing changes:

  • Features

Which issue(s) this PR fixes:

Fixes #2520

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the patch diff.
  • It is difficult to assess performance issues without further context on the codebase and the changes made.
  • Ways to improve could include adding comments to explain complex logic, using more descriptive variable names, and following consistent coding conventions throughout the codebase.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs mentioned in the commit message, but it would be helpful to have a brief overview of any potential issues that were addressed or introduced.
  • The format of the code patch diff looks fine.
  • There is no mention of performance improvements or issues in the commit message. It would be helpful to know if any changes were made to improve performance or if there are any potential performance issues to be aware of.
  • It would be helpful to have more details on how the code could be further improved or optimized.

@@ -1218,6 +1123,27 @@ func needAllocateSubnets(pod *v1.Pod, nets []*kubeovnNet) []*kubeovnNet {
return result
}

func needRouteSubnets(pod *v1.Pod, nets []*kubeovnNet) []*kubeovnNet {
if pod.Status.Phase == v1.PodSucceeded ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use isPodAlive?

if !isOvnSubnet(n.Subnet) {
continue
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

should check if pod.Annotations is nil here

return err
}

// check if allocate subnet is need. also allocte subnet when hotplug nic
Copy link
Collaborator

Choose a reason for hiding this comment

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

allocte -> allocate

ValidatePodCidr and create lsp only in handleAddPod before;
For hotplug and hotunplug nic for a running pod. we also need support when update pod.
so merge handleAddPod with handleUpdatePod to handleAddPodOrUpdatePod.
@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why they were necessary.
  • There are no potential bugs or format errors in the patch diff.
  • It is difficult to assess performance issues without more context about the code being modified.
  • Ways to improve could include adding comments to explain complex logic, refactoring repetitive code, or optimizing algorithms for better efficiency. However, it is difficult to provide specific suggestions without more information about the code being modified.

@oilbeater oilbeater merged commit 2fb1f95 into kubeovn:master Apr 3, 2023
zhangzujian added a commit to zhangzujian/kube-ovn that referenced this pull request Apr 8, 2023
zhangzujian added a commit to zhangzujian/kube-ovn that referenced this pull request Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: Support hotplug nic to pod.
2 participants