-
Notifications
You must be signed in to change notification settings - Fork 258
MGMT-21485: Enable dpu-host mode that matches DPF requirements #2778
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,8 +135,13 @@ spec: | |
# will rollout control plane pods as well | ||
network_segmentation_enabled_flag= | ||
multi_network_enabled_flag= | ||
if [[ "{{.OVN_NETWORK_SEGMENTATION_ENABLE}}" == "true" ]]; then | ||
if [[ "{{.OVN_MULTI_NETWORK_ENABLE}}" == "true" ]]; then | ||
multi_network_enabled_flag="--enable-multi-network" | ||
fi | ||
if [[ "{{.OVN_NETWORK_SEGMENTATION_ENABLE}}" == "true" ]]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a logic change. Was there a reason behind it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general took it from here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general i leaved as is OVN_NETWORK_SEGMENTATION_ENABLE logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kyrtapz the bug you probably have in mind in the end was for DPU GA (4.21), so the PR is still open and we can get to it once we're done with 4.20: #2739 @tsorya so the difference between the original code and your code is that when OVN_MULTI_NETWORK_ENABLE=true and OVN_NETWORK_SEGMENTATION_ENABLE=false, we don't set any corresponding flags in the old code, while you set the multi network flag. Sorry for not realizing before. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, not really. Look on configmap part i remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the running setup So in case --enable-network-segmentation we will always have enable-multi-network. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for confirming! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kyrtapz so I've just discussed this with @tsorya. The way that configuration parameters are handled in ovnk is not as intuitive as I thought. The key point is that non-default values that are present in the configmap take precedence over everything else (https://github.com/ovn-kubernetes/ovn-kubernetes/blame/7c047281d46a6e5f154a0f5d3feb8866af3619f4/go-controller/pkg/config/config.go#L750), also over input arguments that we pass here in the yaml. So that's why Igal is removing a set of parameters from the configmap and adding them as input args in the yaml. These parameters are disabled by default in upstream ovn-k and simply enabling/disabling them through input arguments while keeping them enabled in the configmap wouldn't have worked. In the particular case above, the logic that sets the multi network and network segmentation flags is being moved from the configmap to the yaml. The fact that the logic from the original yaml is more restrictive than the one from the original configmap didn't really matter in the end, because it was the non-default values in the configmap (multi network enabled, udn network segmentation enabled) that were taking precedence over everything else. |
||
if [[ "{{.OVN_MULTI_NETWORK_ENABLE}}" != "true" ]]; then | ||
multi_network_enabled_flag="--enable-multi-network" | ||
fi | ||
network_segmentation_enabled_flag="--enable-network-segmentation" | ||
fi | ||
|
||
|
@@ -149,6 +154,18 @@ spec: | |
if [[ "{{.OVN_PRE_CONF_UDN_ADDR_ENABLE}}" == "true" ]]; then | ||
preconfigured_udn_addresses_enable_flag="--enable-preconfigured-udn-addresses" | ||
fi | ||
|
||
# Enable multi-network policy if configured (control-plane always full mode) | ||
multi_network_policy_enabled_flag= | ||
if [[ "{{.OVN_MULTI_NETWORK_POLICY_ENABLE}}" == "true" ]]; then | ||
multi_network_policy_enabled_flag="--enable-multi-networkpolicy" | ||
fi | ||
|
||
# Enable admin network policy if configured (control-plane always full mode) | ||
admin_network_policy_enabled_flag= | ||
if [[ "{{.OVN_ADMIN_NETWORK_POLICY_ENABLE}}" == "true" ]]; then | ||
admin_network_policy_enabled_flag="--enable-admin-network-policy" | ||
fi | ||
|
||
if [ "{{.OVN_GATEWAY_MODE}}" == "shared" ]; then | ||
gateway_mode_flags="--gateway-mode shared" | ||
|
@@ -178,7 +195,15 @@ spec: | |
${network_segmentation_enabled_flag} \ | ||
${gateway_mode_flags} \ | ||
${route_advertisements_enable_flag} \ | ||
${preconfigured_udn_addresses_enable_flag} | ||
${preconfigured_udn_addresses_enable_flag} \ | ||
--enable-egress-ip=true \ | ||
--enable-egress-firewall=true \ | ||
--enable-egress-qos=true \ | ||
--enable-egress-service=true \ | ||
--enable-multicast \ | ||
--enable-multi-external-gateway=true \ | ||
${multi_network_policy_enabled_flag} \ | ||
${admin_network_policy_enabled_flag} | ||
volumeMounts: | ||
- mountPath: /run/ovnkube-config/ | ||
name: ovnkube-config | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -538,6 +538,8 @@ spec: | |
value: "{{.OVN_CONTROLLER_INACTIVITY_PROBE}}" | ||
- name: OVN_KUBE_LOG_LEVEL | ||
value: "4" | ||
- name: OVN_NODE_MODE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your commit message states that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one provides OVN_NODE_MODE per daemonset. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general it allows to provide the mode we running with. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
value: "{{.OVN_NODE_MODE}}" | ||
{{ if .NetFlowCollectors }} | ||
- name: NETFLOW_COLLECTORS | ||
value: "{{.NetFlowCollectors}}" | ||
|
Uh oh!
There was an error while loading. Please reload this page.