-
Notifications
You must be signed in to change notification settings - Fork 136
Update kube-ovn to latest version #922
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
Conversation
WalkthroughThe updates focus on the Kube-OVN Helm chart and its deployment templates. Network configuration values are now sourced from an external ConfigMap instead of static Helm values. Logging verbosity is made configurable via a new argument. Image and chart versions are incremented, and capability and module management logic in container specs and Dockerfile is revised. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmChart
participant CozyConfigMap
participant KubeOVNController
participant CNIServer
User->>HelmChart: Install/upgrade Kube-OVN chart
HelmChart->>CozyConfigMap: lookup("cozystack", "cozy-system", keys)
CozyConfigMap-->>HelmChart: Return network config values
HelmChart->>KubeOVNController: Deploy with dynamic network args, optional --v
HelmChart->>CNIServer: Deploy with dynamic network args, optional --v
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/system/kubeovn/patches/cozyconfig.diff (1)
1-104: 💡 Verification agent🧩 Analysis chain
Verify the ConfigMap exists in target environments
The PR switches to dynamic retrieval of network configuration from a ConfigMap named "cozystack" in the "cozy-system" namespace. This approach is cleaner but requires this ConfigMap to exist.
🏁 Script executed:
#!/bin/bash # Check if the cozystack ConfigMap exists in the cozy-system namespace kubectl get configmap cozystack -n cozy-system -o yaml 2>/dev/null if [ $? -eq 0 ]; then echo "ConfigMap 'cozystack' exists in 'cozy-system' namespace." # Check if the required keys exist in the ConfigMap kubectl get configmap cozystack -n cozy-system -o jsonpath='{.data}' | grep -E "ipv4-(pod-cidr|pod-gateway|join-cidr|svc-cidr)" if [ $? -eq 0 ]; then echo "Required network configuration keys found in ConfigMap." else echo "WARNING: One or more required network configuration keys missing from ConfigMap." fi else echo "WARNING: ConfigMap 'cozystack' does not exist in 'cozy-system' namespace." fiLength of output: 245
Missing
cozystackConfigMap incozy-systemnamespaceThe dynamic lookup of
cozystackis going to fail at runtime because the ConfigMap does not exist. You need to ensure it’s created with the following data keys before deploying these changes:Affected templates:
packages/system/kubeovn/charts/kube-ovn/templates/ovncni-ds.yamlpackages/system/kubeovn/charts/kube-ovn/templates/controller-deploy.yamlRequired
cozystackConfigMap data keys:
ipv4-pod-cidripv4-pod-gatewayipv4-join-cidripv4-svc-cidrPlease add this ConfigMap (or provide Helm chart pre-install hooks) so that the
lookupcalls succeed.
🧹 Nitpick comments (1)
packages/system/kubeovn/charts/kube-ovn/templates/ovsovn-ds.yaml (1)
103-103: VerifyNET_RAWcapability and consider tightening privileges
GrantingNET_RAWis reasonable for raw socket operations, but please confirm it’s required by upstream kube-ovn. Also review whetherSYS_NICEandSYS_ADMINare still necessary now that module management is offloaded; you may be able to drop them to adhere to least-privilege principles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Makefile(0 hunks)packages/system/kubeovn/Makefile(0 hunks)packages/system/kubeovn/charts/kube-ovn/Chart.yaml(1 hunks)packages/system/kubeovn/charts/kube-ovn/templates/controller-deploy.yaml(1 hunks)packages/system/kubeovn/charts/kube-ovn/templates/ovncni-ds.yaml(2 hunks)packages/system/kubeovn/charts/kube-ovn/templates/ovsovn-ds.yaml(3 hunks)packages/system/kubeovn/charts/kube-ovn/values.yaml(1 hunks)packages/system/kubeovn/images/kubeovn/Dockerfile(0 hunks)packages/system/kubeovn/patches/cozyconfig.diff(3 hunks)packages/system/kubeovn/values.yaml(0 hunks)
💤 Files with no reviewable changes (4)
- Makefile
- packages/system/kubeovn/values.yaml
- packages/system/kubeovn/images/kubeovn/Dockerfile
- packages/system/kubeovn/Makefile
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test
🔇 Additional comments (13)
packages/system/kubeovn/charts/kube-ovn/values.yaml (1)
13-13: Version updated to match upstream releaseThe tag has been updated from v1.13.10 to v1.13.11, aligning with the PR objective to upgrade kube-ovn and use the official upstream image.
packages/system/kubeovn/charts/kube-ovn/Chart.yaml (1)
18-18: Version bumps to 1.13.11Both chart version and appVersion are updated to 1.13.11 to match the new upstream kube-ovn version.
Also applies to: 24-24
packages/system/kubeovn/charts/kube-ovn/templates/controller-deploy.yaml (1)
86-88: Added configurable log verbosityNew configuration allows setting log verbosity level through
.Values.global.logVerbosity, improving debugging capabilities.packages/system/kubeovn/charts/kube-ovn/templates/ovncni-ds.yaml (2)
38-38: Simplified initContainer commandThe conditional logic for running
iptables -Vhas been removed, making the command run unconditionally which simplifies the initialization process.
92-94: Added configurable log verbositySimilar to the controller deployment, this change adds support for configurable log verbosity level through
.Values.global.logVerbosity.packages/system/kubeovn/patches/cozyconfig.diff (6)
10-10: Switched to dynamic network configurationThe patch now retrieves service cluster IP range from the ConfigMap instead of using static values, allowing for centralized configuration management.
Also applies to: 23-23
24-26: Added log verbosity configurationAdded optional log verbosity flag for improved debugging capabilities.
38-38: Switched controller to use ConfigMap for network configurationThe controller deployment now retrieves pod CIDR and gateway values from the ConfigMap. This is a cleaner approach than the previous static configuration with conditionals based on network stack type.
Also applies to: 57-58
81-82: Switched to ConfigMap for node switch CIDR and service CIDRNode switch CIDR and service CIDR now come from the ConfigMap, completing the transition to dynamic network configuration retrieval.
83-85: Added log verbosity for controllerAdded verbosity flag consistent with the previous changes to the controller deployment template.
97-100: Removed static network configuration valuesStatic network configuration values have been removed from values.yaml since they're now retrieved from the ConfigMap. This eliminates duplicate configuration and potential sources of inconsistency.
packages/system/kubeovn/charts/kube-ovn/templates/ovsovn-ds.yaml (2)
53-59: Confirm DISABLE_MODULES_MANAGEMENT flag mapping and default
Ensure that.Values.DISABLE_MODULES_MANAGEMENTis defined invalues.yaml, correctly defaults tofalse, and that the inverted guard (if not .Values.DISABLE_MODULES_MANAGEMENT) aligns with the intended module-loading behavior.
68-70: Approve mounting host modules in init container
The read-only mount of/lib/modulesinto the init container is necessary for kernel module inspection and symlink setup. This aligns with the module-management logic and matches the hostPath volume defined below.
| image: | ||
| docker buildx build images/kubeovn \ | ||
| --provenance false \ | ||
| --tag $(REGISTRY)/kubeovn:$(call settag,$(KUBEOVN_TAG)) \ | ||
| --tag $(REGISTRY)/kubeovn:$(call settag,$(KUBEOVN_TAG)-$(TAG)) \ | ||
| --cache-from type=registry,ref=$(REGISTRY)/kubeovn:latest \ | ||
| --cache-to type=inline \ | ||
| --metadata-file images/kubeovn.json \ | ||
| --push=$(PUSH) \ | ||
| --label "org.opencontainers.image.source=https://github.com/cozystack/cozystack" \ | ||
| --load=$(LOAD) | ||
| REGISTRY="$(REGISTRY)" \ | ||
| yq -i '.global.registry.address = strenv(REGISTRY)' values.yaml | ||
| REPOSITORY="kubeovn" \ | ||
| yq -i '.global.images.kubeovn.repository = strenv(REPOSITORY)' values.yaml | ||
| TAG="$(call settag,$(KUBEOVN_TAG))@$$(yq e '."containerimage.digest"' images/kubeovn.json -o json -r)" \ | ||
| yq -i '.global.images.kubeovn.tag = strenv(TAG)' values.yaml | ||
| rm -f images/kubeovn.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the image target, I hope some day we'll move all images into Cozystack
| make -C packages/system/cozystack-api image | ||
| make -C packages/system/cozystack-controller image | ||
| make -C packages/system/cilium image | ||
| make -C packages/system/kubeovn image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the image target, I hope some day we'll move all images into Cozystack
This commit bumps kube-ovn to 1.13.11 and does away with patching the code now that the fixes necessary for kube-ovn to work properly in Talos have been released in the upstream. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
1cdeb07 to
3ac00ea
Compare
kvaps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This commit bumps kube-ovn to 1.13.11 and does away with patching the code now that the fixes necessary for kube-ovn to work properly in Talos have been released in the upstream. (cherry picked from commit 557ffa5) Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
This commit bumps kube-ovn to 1.13.11 and does away with patching the code now that the fixes necessary for kube-ovn to work properly in Talos have been released in the upstream. (cherry picked from commit 557ffa5) Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
This commit bumps kube-ovn to 1.13.11 and uses the upstream image instead of building our own, now that the fixes necessary for kube-ovn to work properly in Talos have been released in the upstream.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes