Add arm64 MicroOVN support#833
Conversation
hmlanigan
left a comment
hmlanigan
left a comment
There was a problem hiding this comment.
How can I verify this change works in the real world, not only in unit tests?
| } | ||
|
|
||
| resource "juju_application" "microovn_arm64" { | ||
| count = length(var.microovn_arm64_machine_ids) > 0 ? 1 : 0 |
There was a problem hiding this comment.
question: why is this count instead of units?
suggestion: 1 : 0 should be 1 : null
There was a problem hiding this comment.
count is a terraform meta-argument, unit is a Juju provider attribute inside a juju_application, and count must be a whole number (0, 1, 2, …). null is not valid for count.
| count = length(var.microovn_arm64_machine_ids) > 0 ? 1 : 0 | ||
| name = "microovn-arm64" | ||
| model_uuid = data.juju_model.machine_model.uuid | ||
| machines = toset(var.microovn_arm64_machine_ids) |
There was a problem hiding this comment.
| machines = toset(var.microovn_arm64_machine_ids) | |
| machines = length(var.microovn_machine_ids) == 0 ? null : toset(var.microovn_machine_ids) |
| model_uuid = data.juju_model.machine_model.uuid | ||
| machines = length(var.microovn_machine_ids) == 0 ? null : toset(var.microovn_machine_ids) | ||
| units = length(var.microovn_machine_ids) == 0 ? 1 : null | ||
| units = ( |
There was a problem hiding this comment.
question: why does this change with the addition of arm64 machines?
| def get_machines_amd64(self) -> list[str]: | ||
| """Get amd64 machine IDs for MicroOVN.""" | ||
| return self.get_machines(DEFAULT_ARCHITECTURE) | ||
|
|
||
| def get_machines_arm64(self) -> list[str]: | ||
| """Get arm64 machine IDs for MicroOVN (e.g. DPU network nodes).""" | ||
| return self.get_machines(ARM64_ARCHITECTURE) | ||
|
|
There was a problem hiding this comment.
question: what about s390x, riskv, and other architectures?
There was a problem hiding this comment.
These get_machines_amd64/arm64 helpers have been removed. Machine selection now goes through get_machines_by_architecture(), which groups by whatever arch clusterd reports (s390x, riscv, etc.)
|
@hmlanigan @gboutry please review, thanks |
gboutry
left a comment
gboutry
left a comment
There was a problem hiding this comment.
This is overall fine to me.
We'll need to sort out the role-distributor situation still.
The part that is more worrisome to me, but not necessarily this PR's concern, is how do we actually manage MicroOVN upgrade over multiple applications?
| } | ||
| } | ||
|
|
||
| resource "juju_integration" "role-distributor-microovn" { |
There was a problem hiding this comment.
WHy are we dropping this integration?
We should have the integration to all microovn apps
3 participants
Summary
Adds support for deploying the
microovn-arm64charm on ARM64 DPU nodes. Sunbeam now detects arm64 nodes and deploys the arm64 MicroOVN application with the correct Juju constraints.Changes
microovn-arm64as a separate Juju application for arm64 nodesarch=arm64when deploying arm64 MicroOVN machinesmicroovnandmicroovn-arm64when both are presentLinks
Jira card: OPEN-4508