Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Antrea architecture doc #1
Add Antrea architecture doc #1
Changes from all commits
05e9cb3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we call this a client ? because we say NetworkPolicy controller is in Antrea Controller and it makes sense there... need to differentiate between the two to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it is indeed a K8s controller and called NetworkPolicyController in code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets call it Antrea NetworkPolicy Controller?.. because technically it is not syncing the networkingv1.NetworkPolicy objects but Antrea NetworkPolicy Object disseminated by antrea-controller. do you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From this perspective, the k8s DeploymentController is also not syncing the Deployment itself but the pods/replica rets managed by it. Maybe it's fine as long as people can get a brief idea about what's the controller for from the name?
What I'm worrying more is the name of NodeController, it's a little hard to get what's it without reading the documentation. I'd propose to rename the package to noderoute and the controller NodeRouteController, what do you think? (mapping to k8s nodeipam and nodelifecycle controllers). Of course, this is out of this PR's scope, just want to discuss here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the current description is fine. Like @tnqn said, we explained what the controllers are doing.
I support to rename NodeController to NoteRouteController, but let us change the code first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just feel like the network policy object and network policy crd for controller are different .. the latter is more specific to antrea controller.. but if you guys believe it sounds clear to the readers.. then no need to change
NodeRouteController does convey the correct message