PCP-5152: Multiple node HCP cluster losing interface reference #238
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the PCP-5152 issue regarding multiple node HCP clusters losing interface references by refactoring the LXD initialization and registration process. The changes move the responsibility for LXD host registration from the controller to a DaemonSet running on target cluster nodes, which improves reliability and provides better MAAS integration.
Key changes:
- Removes direct LXD host registration from the controller's
ReconcileLXD()method - Adds new utility functions for safe type conversion and trust password derivation
- Significantly enhances the LXD initializer DaemonSet with proper registration logic, staggered execution, and improved error handling
Reviewed Changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/util/utils.go | Adds safe int64 to int32 conversion utility |
| pkg/util/trust/password.go | Adds deterministic trust password generation |
| pkg/maas/lxd/service.go | Removes setupLXDOnMachine function and simplifies reconciliation |
| pkg/maas/lxd/host_maas_client.go | Adds hostname support for LXD host registration |
| lxd-initializer/lxd-initializer.go | Major refactor with registration logic, staggering, and improved configuration |
| lxd-initializer/lxd-initializer-daemonset.yaml | Updates container image version |
| lxd-initializer/go.mod | Updates maas-client-go dependency |
| go.mod | Updates maas-client-go dependency |
| controllers/templates/lxd_initializer_rbac.yaml | Adds list and watch permissions for nodes |
| controllers/templates/lxd_initializer_ds.yaml | Updates image and adds imagePullPolicy |
| controllers/maasmachine_controller.go | Adds forced VM deletion during host cleanup |
| controllers/lxd_initializer_ds.go | Adds intelligent deployment gating and short-circuit logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // // Determine host short name for unique bridge suffix | ||
| // hostShort := nodeName | ||
| // if hostShort == "" { | ||
| // if osHN, _ := os.Hostname(); osHN != "" { | ||
| // hostShort = osHN | ||
| // } | ||
| // } | ||
| // hostToken := normalizeName(hostShort) |
There was a problem hiding this comment.
Remove commented-out code to improve maintainability. If this logic might be needed later, consider documenting the reasoning in a regular comment instead.
| // // Unique bridge per host to avoid cross-host name collisions | ||
| // uniqueBridge := networkBridge | ||
| // if hostToken != "" { | ||
| // uniqueBridge = fmt.Sprintf("%s-%s", networkBridge, hostToken) | ||
| // } |
There was a problem hiding this comment.
Remove commented-out code to improve maintainability. If this logic might be needed later, consider documenting the reasoning in a regular comment instead.
| // // Keep the container running to maintain the DaemonSet if in daemon mode | ||
| // log.Println("LXD initialization completed successfully") | ||
| // log.Println("Starting periodic trust-password maintainer") | ||
| // go func() { | ||
| // for { | ||
| // if err := setTrustPassword(trustPassword); err != nil { | ||
| // log.Printf("periodic trust password set failed: %v", err) | ||
| // } | ||
| // time.Sleep(15 * time.Minute) | ||
| // } | ||
| // }() |
There was a problem hiding this comment.
Remove commented-out code to improve maintainability. If this periodic trust password functionality might be needed later, consider documenting the reasoning in a regular comment instead.
| // // Keep the container running to maintain the DaemonSet if in daemon mode | |
| // log.Println("LXD initialization completed successfully") | |
| // log.Println("Starting periodic trust-password maintainer") | |
| // go func() { | |
| // for { | |
| // if err := setTrustPassword(trustPassword); err != nil { | |
| // log.Printf("periodic trust password set failed: %v", err) | |
| // } | |
| // time.Sleep(15 * time.Minute) | |
| // } | |
| // }() | |
| // If periodic trust password maintenance is needed in the future, consider running setTrustPassword in a background goroutine here. |
|
Closing this change will go inhttps://github.com//pull/239 |
1 participant
No description provided.