Add network connectivity information to Server status#562
Add network connectivity information to Server status#562
Server status#562Conversation
api/v1alpha1/server_types.go
Outdated
| // +kubebuilder:validation:Enum=Pass;Fail;Unknown | ||
| // +kubebuilder:default=Unknown | ||
| // +optional | ||
| NetworkCheck string `json:"networkCheck,omitempty"` |
There was a problem hiding this comment.
Who exactly should react on changes to that field? Also this looks like something which is better kept in the status.conditions.
There was a problem hiding this comment.
it's like a flag to check after buildup, reconfiguration. It will also be present in greenhouse/prometheus.
I agree that status.conditions is a better place.
There was a problem hiding this comment.
What we are doing on our side is setting lables/annotations indicating that the Server is ready for consumption.
There was a problem hiding this comment.
An idiomatic way of doing things like that would be e.g.:
Your controller which talks to netbox writes:
my-operator.io/network-validation-result: success
Then the ServerReconciler could populate e.g. the conditions slice with a corresponding Condition.
Alternatively we can define a dedicated substruct in the status for that e.g.:
status:
state: Available
conditions:
- type: Ready
# ... other fields ...
# Managed by some external component
networkConnectivity:
status: "Unknown" # "Ready", "Failure", "Checking"
lastUpdateTime: "2025-12-10T12:00:00Z"
message: "Awaiting connectivity check from external service."
The external component should have only PATCH RBAC permissions on the Server resources.
There was a problem hiding this comment.
IMO this would be a good candidate for the Conditions instead.
|
Can you please regenerate the Helm chart files via |
afritzler
changed the title
Server status
api/v1alpha1/server_types.go
Outdated
| // +kubebuilder:validation:Enum=Pass;Fail;Unknown | ||
| // +kubebuilder:default=Unknown | ||
| // +optional | ||
| NetworkCheck string `json:"networkCheck,omitempty"` |
There was a problem hiding this comment.
Could we use here consts, and enums if we remain with the field?
const (
NetworkCheckPass NetworkCheck = "Pass"
NetworkCheckFail NetworkCheck = "Fail"
NetworkCheckUnknown NetworkCheck = "Unknown"
)
// +kubebuilder:validation:Enum=Pass;Fail;Unknown
// +kubebuilder:default=Unknown
// +optional
NetworkCheck NetworkCheck `json:"networkCheck,omitempty"`
api/v1alpha1/server_types.go
Outdated
| // +kubebuilder:validation:Enum=Pass;Fail;Unknown | ||
| // +kubebuilder:default=Unknown | ||
| // +optional | ||
| NetworkCheck string `json:"networkCheck,omitempty"` |
There was a problem hiding this comment.
IMO this would be a good candidate for the Conditions instead.
Introduces a ServerNetworkConfig CRD that holds the expected network interface configuration (populated by an external source such as argora from NetBox) and the result of the check performed by metal-operator. At the Discovery→Available transition, metal-operator compares spec.interfaces (expected switch/port per MAC) against Server.status.networkInterfaces (LLDP data from discovery). The gate is opt-in by presence: no ServerNetworkConfig means no gate.
xkonni
force-pushed
the
feat/networkCheck
branch
from
030267d to
6f023cd
Compare
|
replaced by a new one. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
WalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Reconciler as Server Reconciler
participant Server as Server Resource
participant SNC as ServerNetworkConfig
participant LLDP as Discovered Interfaces<br/>(Server.Status)
Reconciler->>Server: Discover network interfaces via LLDP
activate Server
Server-->>Reconciler: Store discovered interfaces in Status
deactivate Server
Reconciler->>Reconciler: handleDiscoveryState() invoked
Reconciler->>SNC: Lookup ServerNetworkConfig referencing this Server
activate SNC
SNC-->>Reconciler: Return ServerNetworkConfig with expected interfaces
deactivate SNC
Reconciler->>Reconciler: runNetworkCheck(): Compare expected vs discovered
Reconciler->>LLDP: Retrieve discovered LLDP neighbors
activate LLDP
LLDP-->>Reconciler: Return interface list
deactivate LLDP
Reconciler->>Reconciler: switchPortMatch(): Normalize & compare<br/>expected vs actual switch/port
alt Check passes (no mismatches)
Reconciler->>SNC: Update Status (Phase=Passed, Mismatches=[])
Reconciler->>Server: Transition to ServerStateAvailable
else Check fails (mismatches found)
Reconciler->>SNC: Update Status (Phase=Failed, Mismatches=[...])
Reconciler->>Reconciler: Requeue reconciliation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
bot
added
size/XL
documentation
3 participants
NetworkCheck indicates the network configuration validation status.
Possible values: Unknown/Pass/Fail, default Unknown.
Proposed Changes
networkCheckfield to theserverscrdFixes #
Allows an inventory check to compare the network definitions in netbox with the information in the crd and report on the outcome
Summary by CodeRabbit
Release Notes
New Features
Documentation