-
Notifications
You must be signed in to change notification settings - Fork 277
NVIDIA-596: Enable dpu healthcheck #2941
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -540,6 +540,9 @@ data: | |||||
| # Ensure allow_icmp_network_policy_flag is always defined | ||||||
| allow_icmp_network_policy_flag= | ||||||
|
|
||||||
| # Ensure dpu_lease_flags is always defined | ||||||
| dpu_lease_flags= | ||||||
|
|
||||||
| if [[ $# -ne 3 ]]; then | ||||||
| echo "Expected three arguments but got $#" | ||||||
| exit 1 | ||||||
|
|
@@ -600,6 +603,15 @@ data: | |||||
| multi_external_gateway_enable_flag="" | ||||||
| fi | ||||||
|
|
||||||
| if [ "${OVN_NODE_MODE}" == "dpu-host" ] || [ "${OVN_NODE_MODE}" == "dpu" ]; then | ||||||
| if [[ -n "${OVNKUBE_NODE_LEASE_RENEW_INTERVAL}" ]]; then | ||||||
|
Contributor
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. Actually, you could remove line 606 since the other two variables will always be unset in non-DPU modes... |
||||||
| dpu_lease_flags="--dpu-node-lease-renew-interval ${OVNKUBE_NODE_LEASE_RENEW_INTERVAL}" | ||||||
| fi | ||||||
| if [[ -n "${OVNKUBE_NODE_LEASE_DURATION}" ]]; then | ||||||
| dpu_lease_flags="$dpu_lease_flags --dpu-node-lease-duration ${OVNKUBE_NODE_LEASE_DURATION}" | ||||||
|
Contributor
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. ok, since I'm already nitpicking on shell script style...
Suggested change
|
||||||
| fi | ||||||
| fi | ||||||
|
|
||||||
| if [ "{{.OVN_GATEWAY_MODE}}" == "shared" ]; then | ||||||
| gateway_mode_flags="--gateway-mode shared --gateway-interface ${gateway_interface}" | ||||||
| elif [ "{{.OVN_GATEWAY_MODE}}" == "local" ]; then | ||||||
|
|
@@ -797,5 +809,6 @@ data: | |||||
| ${ovn_v4_transit_switch_subnet_opt} \ | ||||||
| ${ovn_v6_transit_switch_subnet_opt} \ | ||||||
| ${egress_features_enable_flag} \ | ||||||
| ${multi_external_gateway_enable_flag} | ||||||
| ${multi_external_gateway_enable_flag} \ | ||||||
| ${dpu_lease_flags} | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,3 +12,5 @@ data: | |
| dpu-mode-label: "network.operator.openshift.io/dpu=" | ||
| smart-nic-mode-label: "network.operator.openshift.io/smart-nic=" | ||
| mgmt-port-resource-name: "openshift.io/mgmtvf" | ||
| dpu-node-lease-renew-interval: "10" | ||
| dpu-node-lease-duration: "40" | ||
|
Contributor
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. 10 and 40 what? if you made this a
Contributor
Author
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. Sounds good, will change
Contributor
Author
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. The issue is it is the same name as in ovnk but i can add some suffix maybe |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -65,6 +65,11 @@ const OVN_NODE_SELECTOR_DEFAULT_DPU = "network.operator.openshift.io/dpu=" | |||||||||||||||||||||||||||||||||||||||||
| const OVN_NODE_SELECTOR_DEFAULT_SMART_NIC = "network.operator.openshift.io/smart-nic=" | ||||||||||||||||||||||||||||||||||||||||||
| const OVN_NODE_IDENTITY_CERT_DURATION = "24h" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Default DPU health check lease configuration. | ||||||||||||||||||||||||||||||||||||||||||
| // Setting renew-interval to 0 disables the health check. | ||||||||||||||||||||||||||||||||||||||||||
| const DPU_NODE_LEASE_RENEW_INTERVAL_DEFAULT = 10 | ||||||||||||||||||||||||||||||||||||||||||
| const DPU_NODE_LEASE_DURATION_DEFAULT = 40 | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // gRPC healthcheck port. See: https://github.com/openshift/enhancements/pull/1209 | ||||||||||||||||||||||||||||||||||||||||||
| const OVN_EGRESSIP_HEALTHCHECK_PORT = "9107" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -232,6 +237,8 @@ func renderOVNKubernetes(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bo | |||||||||||||||||||||||||||||||||||||||||
| data.Data["SmartNicModeLabel"] = bootstrapResult.OVN.OVNKubernetesConfig.SmartNicModeLabel | ||||||||||||||||||||||||||||||||||||||||||
| data.Data["SmartNicModeValue"] = bootstrapResult.OVN.OVNKubernetesConfig.SmartNicModeValue | ||||||||||||||||||||||||||||||||||||||||||
| data.Data["MgmtPortResourceName"] = bootstrapResult.OVN.OVNKubernetesConfig.MgmtPortResourceName | ||||||||||||||||||||||||||||||||||||||||||
| data.Data["DpuNodeLeaseRenewInterval"] = strconv.Itoa(bootstrapResult.OVN.OVNKubernetesConfig.DpuNodeLeaseRenewInterval) | ||||||||||||||||||||||||||||||||||||||||||
| data.Data["DpuNodeLeaseDuration"] = strconv.Itoa(bootstrapResult.OVN.OVNKubernetesConfig.DpuNodeLeaseDuration) | ||||||||||||||||||||||||||||||||||||||||||
| data.Data["OVN_CONTROLLER_INACTIVITY_PROBE"] = os.Getenv("OVN_CONTROLLER_INACTIVITY_PROBE") | ||||||||||||||||||||||||||||||||||||||||||
| controller_inactivity_probe := os.Getenv("OVN_CONTROLLER_INACTIVITY_PROBE") | ||||||||||||||||||||||||||||||||||||||||||
| if len(controller_inactivity_probe) == 0 { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1015,10 +1022,12 @@ func findCommonNode(nodeLists ...[]string) (bool, string) { | |||||||||||||||||||||||||||||||||||||||||
| // if it exists, otherwise returns default configuration for OCP clusters using OVN-Kubernetes | ||||||||||||||||||||||||||||||||||||||||||
| func bootstrapOVNConfig(conf *operv1.Network, kubeClient cnoclient.Client, hc *hypershift.HyperShiftConfig, infraStatus *bootstrap.InfraStatus) (*bootstrap.OVNConfigBoostrapResult, error) { | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult := &bootstrap.OVNConfigBoostrapResult{ | ||||||||||||||||||||||||||||||||||||||||||
| DpuHostModeLabel: OVN_NODE_SELECTOR_DEFAULT_DPU_HOST, | ||||||||||||||||||||||||||||||||||||||||||
| DpuModeLabel: OVN_NODE_SELECTOR_DEFAULT_DPU, | ||||||||||||||||||||||||||||||||||||||||||
| SmartNicModeLabel: OVN_NODE_SELECTOR_DEFAULT_SMART_NIC, | ||||||||||||||||||||||||||||||||||||||||||
| MgmtPortResourceName: "", | ||||||||||||||||||||||||||||||||||||||||||
| DpuHostModeLabel: OVN_NODE_SELECTOR_DEFAULT_DPU_HOST, | ||||||||||||||||||||||||||||||||||||||||||
| DpuModeLabel: OVN_NODE_SELECTOR_DEFAULT_DPU, | ||||||||||||||||||||||||||||||||||||||||||
| SmartNicModeLabel: OVN_NODE_SELECTOR_DEFAULT_SMART_NIC, | ||||||||||||||||||||||||||||||||||||||||||
| MgmtPortResourceName: "", | ||||||||||||||||||||||||||||||||||||||||||
| DpuNodeLeaseRenewInterval: DPU_NODE_LEASE_RENEW_INTERVAL_DEFAULT, | ||||||||||||||||||||||||||||||||||||||||||
| DpuNodeLeaseDuration: DPU_NODE_LEASE_DURATION_DEFAULT, | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if conf.Spec.DefaultNetwork.OVNKubernetesConfig.GatewayConfig == nil { | ||||||||||||||||||||||||||||||||||||||||||
| bootstrapOVNGatewayConfig(conf, kubeClient.ClientFor("").CRClient()) | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1064,6 +1073,33 @@ func bootstrapOVNConfig(conf *operv1.Network, kubeClient cnoclient.Client, hc *h | |||||||||||||||||||||||||||||||||||||||||
| if exists { | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.MgmtPortResourceName = mgmtPortresourceName | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if val, exists := cm.Data["dpu-node-lease-renew-interval"]; exists { | ||||||||||||||||||||||||||||||||||||||||||
| parsed, err := strconv.Atoi(val) | ||||||||||||||||||||||||||||||||||||||||||
| if err == nil && parsed >= 0 { | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.DpuNodeLeaseRenewInterval = parsed | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| klog.Warningf("Invalid dpu-node-lease-renew-interval %q, using default %d", val, DPU_NODE_LEASE_RENEW_INTERVAL_DEFAULT) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if val, exists := cm.Data["dpu-node-lease-duration"]; exists { | ||||||||||||||||||||||||||||||||||||||||||
| parsed, err := strconv.Atoi(val) | ||||||||||||||||||||||||||||||||||||||||||
| if err == nil && parsed >= 0 { | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.DpuNodeLeaseDuration = parsed | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| klog.Warningf("Invalid dpu-node-lease-duration %q, using default %d", val, DPU_NODE_LEASE_DURATION_DEFAULT) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Setting either value to 0 disables the DPU health check. | ||||||||||||||||||||||||||||||||||||||||||
| // When both are non-zero, duration must be greater than interval. | ||||||||||||||||||||||||||||||||||||||||||
| if ovnConfigResult.DpuNodeLeaseRenewInterval != 0 && ovnConfigResult.DpuNodeLeaseDuration != 0 && | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.DpuNodeLeaseDuration <= ovnConfigResult.DpuNodeLeaseRenewInterval { | ||||||||||||||||||||||||||||||||||||||||||
| klog.Warningf("dpu-node-lease-duration (%d) must be greater than dpu-node-lease-renew-interval (%d), using defaults", | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.DpuNodeLeaseDuration, ovnConfigResult.DpuNodeLeaseRenewInterval) | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.DpuNodeLeaseRenewInterval = DPU_NODE_LEASE_RENEW_INTERVAL_DEFAULT | ||||||||||||||||||||||||||||||||||||||||||
| ovnConfigResult.DpuNodeLeaseDuration = DPU_NODE_LEASE_DURATION_DEFAULT | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1094
to
+1102
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. Normalize disable semantics when either lease value is Line 1094 says either value disables health check, but current flow can still render flags when renew interval is non-zero and duration is Proposed fix- // Setting either value to 0 disables the DPU health check.
- // When both are non-zero, duration must be greater than interval.
- if ovnConfigResult.DpuNodeLeaseRenewInterval != 0 && ovnConfigResult.DpuNodeLeaseDuration != 0 &&
- ovnConfigResult.DpuNodeLeaseDuration <= ovnConfigResult.DpuNodeLeaseRenewInterval {
+ // Setting either value to 0 disables the DPU health check.
+ if ovnConfigResult.DpuNodeLeaseRenewInterval == 0 || ovnConfigResult.DpuNodeLeaseDuration == 0 {
+ ovnConfigResult.DpuNodeLeaseRenewInterval = 0
+ ovnConfigResult.DpuNodeLeaseDuration = 0
+ } else if ovnConfigResult.DpuNodeLeaseDuration <= ovnConfigResult.DpuNodeLeaseRenewInterval {
+ // When both are non-zero, duration must be greater than interval.
klog.Warningf("dpu-node-lease-duration-in-seconds (%d) must be greater than dpu-node-lease-renew-interval-in-seconds (%d), using defaults",
ovnConfigResult.DpuNodeLeaseDuration, ovnConfigResult.DpuNodeLeaseRenewInterval)
ovnConfigResult.DpuNodeLeaseRenewInterval = DPU_NODE_LEASE_RENEW_INTERVAL_DEFAULT
ovnConfigResult.DpuNodeLeaseDuration = DPU_NODE_LEASE_DURATION_DEFAULT
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // We want to see if there are any nodes that are labeled for specific modes such as Full/SmartNIC/DPU Host/DPU | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
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.
nitpick: be consistent with
[[/]]rather than[/]