Add hostname propagation to ironcore-net NetworkInterfaces#681
Add hostname propagation to ironcore-net NetworkInterfaces#681ushabelgur wants to merge 4 commits into
NetworkInterfaces#681Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an optional HostName field to NetworkInterfaceSpec, sets it from the machine GuestConfig during machine creation, and includes it when applying APInet NetworkInterface; also bumps Go/Kubernetes dependencies and pinned tool versions. ChangesHostname propagation
Sequence DiagramsequenceDiagram
participant Client
participant MachineCreate as Machine Create Handler
participant APISpec as NetworkInterfaceSpec
participant APINETPlugin as APINet Plugin
participant IronNetAPI as IronCore-Net API
Client->>MachineCreate: Create Machine request
MachineCreate->>APISpec: Construct NetworkInterfaceSpec
MachineCreate->>APISpec: Set HostName (from GuestConfig.Hostname)
MachineCreate->>APINETPlugin: Request NIC creation with Spec
APINETPlugin->>APISpec: Read HostName from Spec
APINETPlugin->>IronNetAPI: Apply NetworkInterface (includes Hostname)
IronNetAPI->>APINETPlugin: Return applied NetworkInterface
APINETPlugin->>IronNetAPI: Get applied NetworkInterface
APINETPlugin->>MachineCreate: Return result
MachineCreate->>Client: Respond Machine created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
b53f555 to
e41fa2f
Compare
072447e to
2474f40
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/server/machine_create.go (1)
57-67:⚠️ Potential issue | 🔴 Critical
HostNamefield will be empty string becauseiriMachine.Metadata.Idis not populated in requests.At line 64, the
HostNameis set toiriMachine.Metadata.Id, but the IRI request'sMetadataobject never includes anIdfield—it's only present in the response after being generated by the server. All test cases confirm that incomingMachine.Metadatacontains only labels, not an id.The PR objective states "Pass machineName as hostname," but the IRI
Machinerequest provides no machine name field. The available option in the request is theLabelsmap (which contains a UID label, not a human-readable name). The server's generated internal ID at line 71 comes too late and is not the IRI machine's identifier anyway.To fix this: determine whether to use a label value (if machine name is stored there), or reconsider the approach for obtaining the machine identifier at the point where
NetworkInterfaceSpecis created.
Signed-off-by: ushabelgur <usha.belgur@t-systems.com>
Signed-off-by: ushabelgur <usha.belgur@t-systems.com>
Signed-off-by: ushabelgur <usha.belgur@t-systems.com>
Signed-off-by: ushabelgur <usha.belgur@t-systems.com>
| NetworkId string `json:"networkId"` | ||
| Ips []string `json:"ips"` | ||
| Attributes map[string]string `json:"attributes"` | ||
| HostName string `json:"hostName,omitempty"` |
There was a problem hiding this comment.
@lukasfrank @afritzler should I introduce GuestConfig type for libvirt machine type? or only HostName is fine for now ?
There was a problem hiding this comment.
I think it is a good idea to keep it consistent with the IRI side and introduce the GuestConfig. Though, not on the NetworkInterfaceSpec, but on the MachineSpec, like on the IRI side. Reasoning is, that there can be multiple NICs and with that we would introduce a possibility of a bug with inconsistent hostnames accross multiple NICs. Something that should be one, would be many. The apinet plugin has also access to the machine, so it can take the hostname there from the machine's GuestConfig. @lukasfrank What do you think?

Proposed Changes
NetworkInterfaceswhile creating machineFixes #538
Summary by CodeRabbit
New Features
Chores