fix: skip unallocated pods during IPAM initialization#6327
Conversation
Summary of ChangesHello @oilbeater, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the IPAM initialization process to prevent unnecessary error logging during the controller's initial startup. By introducing a preliminary check, the system now intelligently bypasses pods that lack IP allocation annotations, ensuring that Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Pull Request Test Coverage Report for Build 22335642797Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Code Review
The pull request successfully addresses the issue of spurious error logs during InitIPAM() by skipping pods that have not yet been allocated an IP by Kube-OVN. This is a sound approach because InitIPAM() is intended to restore existing state, and pods without allocation annotations have no state to restore at this stage. The implementation is clean, follows existing patterns in the codebase, and includes comprehensive unit tests for the new helper function.
…us errors During controller startup, InitIPAM() iterates over all pods and calls getPodKubeovnNets(), which checks namespace network annotations. However, namespace annotations are not yet set at this point because the namespace worker starts after InitIPAM(). This causes spurious error logs like "namespace kube-system network annotations is empty" for pods that have not been allocated an IP by kube-ovn (e.g., coredns on first startup). Skip pods without any allocated annotation before calling getPodKubeovnNets(), since they have no IP to restore into IPAM anyway. Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
7403638 to
467049c
Compare
Summary
InitIPAM()iterates all pods and callsgetPodKubeovnNets(), which checks namespace network annotations. However, namespace annotations are not yet set at this point because the namespace worker starts afterInitIPAM(). This causes spurious error logs for pods not yet managed by kube-ovn (e.g., coredns on first startup):*.kubernetes.io/allocated=trueannotation before callinggetPodKubeovnNets(), since they have no IP to restore into IPAM anyway. The inner loop atinit.go:450already checksallocated == "true"per provider, so these pods would be no-ops even if they passed.Test plan
hasAllocatedAnnotation()covering nil/empty annotations, default/custom providers, false values, and multi-provider scenariosmake lintpasses with 0 issuesgo test ./pkg/controller/...passes🤖 Generated with Claude Code