Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update network concept doc with network peering #256

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

kasabe28
Copy link
Contributor

@kasabe28 kasabe28 commented Mar 7, 2024

Proposed Changes

  • Added network peering support concept doc

Fixes #

@kasabe28 kasabe28 requested a review from a team as a code owner March 7, 2024 11:00
@github-actions github-actions bot added size/S documentation Improvements or additions to documentation labels Mar 7, 2024
`spec.peerings` referencing each other respectively. A `ironcore` `Network` can be peered with
multiple `network`s in any namespcae.

Once specified `apinetlet` `NetworkPeeringController` validates if all the specified `network`s are in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need an own reconciler for the Network peering or wouldn't it be just enough to extend the existing Network reconciler to ensure the peering information in the Network in the apinet group and update the ironcore Status once the peering was established.

Copy link
Contributor Author

@kasabe28 kasabe28 Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Network peering is happening only with Available state networks, so I think we should have different reconciler for network peering. If we do it in existing Network controller we have to wait till both networks come Available and then do peering in same reconcile loop will not be possible.
@balpert89 do you have different thoughts for new controller?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes my suggestion was to separate this in a dedicated controller because of two reasons:

  • what happens when the peering partner network has not set the currently reconciling network in its peering reference? The reconcile request has to be requeued then and can block succeeding the loop for this particular network (and its partner). If the logic for peering is isolated in its own controller, the "normal" reconciliation can happen without changing its logic.
  • as @kasabe28 mentioned, the reconciliation should only happen once the network status is Available which is contradicting to the current network reconciler as this one is actually setting the status to Available.

`spec.peeredIDs` with valid peered network's `providerID`'s.

Once `ironcore-net` `Network` is updated with `spec.peeredIDs`, `metalnetlet` `NetworkController`
updates `metalnet` `Network` `spec.peeredIDs` with corresponding `ironcore-net` `Network` `spec.peeredIDs`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the peering status I opened an issue in the the metalnet project to clarify if we actually can get any status information: ironcore-dev/metalnet#189

@afritzler afritzler merged commit 6a1e4dd into main Mar 20, 2024
8 checks passed
@afritzler afritzler deleted the concept-doc/network-peering branch March 20, 2024 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation size/S
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants