interface dhcp configuration #143
Conversation
dhcp should be an opt-in option because running it can cause delays on pod startup on environments that don't have dhcp enabled. Change-Id: I2905788c261583d86a5701576c2e61a245138c02
There was a problem hiding this comment.
Pull Request Overview
This PR makes DHCP configuration an opt‐in option to avoid startup delays in environments without DHCP enabled. It updates the network driver and DHCP client logic to conditionally request network parameters via DHCP, adjusts validations and tests to enforce mutually exclusive DHCP and manual addresses, and enhances documentation for the new DHCP field.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/driver/dra_hooks.go | Adds conditional DHCP logic with a context-based timeout in the DHCP request and removes redundant DHCP code blocks. |
| pkg/driver/dhcp.go | Updates the getDHCP function to accept a context and pass it to the DHCP request. |
| pkg/apis/validation_test.go | Adds tests covering valid and invalid configurations involving the DHCP option. |
| pkg/apis/validation.go | Updates validation to error when DHCP and addresses are both provided. |
| pkg/apis/types.go | Documents and declares the new DHCP field in the InterfaceConfig type. |
Comments suppressed due to low confidence (1)
pkg/driver/dhcp.go:48
- [nitpick] Consider updating the error message format for consistency by using '%w' for error wrapping and reviewing the use of 'up' in the message to maintain uniformity with similar messages in the code.
return "", nil, fmt.Errorf("fail to obtain DHCP lease on interface %s up: %v", ifName, err)
| // If there is no custom addresses then use the existing ones | ||
| if len(podCfg.Network.Interface.Addresses) == 0 { | ||
| // If DHCP is requested, do a DHCP request to gather the network parameters (IPs and Routes) | ||
| // ... but we DO NOT apply them in the root namespace |
There was a problem hiding this comment.
[nitpick] Consider adding a small inline comment or refactoring the conditional block to clarify that the DHCP configuration is opt-in and bypasses the default address acquisition to improve code readability in the future.
| // ... but we DO NOT apply them in the root namespace | |
| // ... but we DO NOT apply them in the root namespace | |
| // Note: DHCP configuration is opt-in and bypasses the default address acquisition logic below. |
Copilot uses AI. Check for mistakes.
dhcp should be an opt-in option because running it can cause delays on pod startup on environments that don't have dhcp enabled.
Change-Id: I2905788c261583d86a5701576c2e61a245138c02