-
Notifications
You must be signed in to change notification settings - Fork 364
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
Realize Egress for a Pod once its network is created #3360
Conversation
c93a460
to
8697a9d
Compare
@Dyanngg Do you remember why it's entityUpdateChannel, instead of podUpdateChannel? I don't find anywhere entity update is published. Is it reserved for future usage? Anyway I think later we could create an externalEntityUpdateChannel when necessary, instead of mixing the channel of PodUpdate. Please let me know if this change breaks anything. |
/test-all |
8697a9d
to
053a48c
Compare
/test-all |
/test-ipv6-all |
053a48c
to
25d0aee
Compare
/test-ipv6-e2e |
Codecov Report
@@ Coverage Diff @@
## main #3360 +/- ##
==========================================
- Coverage 60.85% 53.61% -7.25%
==========================================
Files 268 240 -28
Lines 26723 34179 +7456
==========================================
+ Hits 16263 18324 +2061
- Misses 8649 14078 +5429
+ Partials 1811 1777 -34
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/test-all |
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.
In commit message:
Previously antrea-controller only included a Pod in an EgressGroup only
Remove one "only".
pkg/util/channel/channel.go
Outdated
type eventHandler func(string) | ||
|
||
type Subscriber interface { | ||
// Subscribe subscribes an eventHandler which will be called when there is an event sent to the channel. |
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.
subscribes -> registers?
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.
Remove "there is"
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.
done
pkg/util/channel/channel.go
Outdated
// Channel is different from the Go channel which dispatches every event to only single consumer regardless of the | ||
// number of consumers. Instead, it dispatches every event to all consumers by calling the eventHandlers they have | ||
// registered. | ||
type Channel struct { |
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 use a different name to distinguish from Golang channel? Like SubscriptionChannel.
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.
does SubscribableChannel make sense to you?
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.
Works for me.
Previously antrea-controller included a Pod in an EgressGroup only when its IP has been presented in K8s API. If a Pod tries to access external right after it's up, Node IP will be used as the SNAT IP even when an Egress applying to it has been created because its Pod IP may haven't been reported to K8s API or antrea-controller may haven't included the Pod in the EgressGroup. This patch fixes it by making CNIServer notify EgressController that it has processed CNI ADD request of a Pod, then EgressController can reconcile the corresponding Egress immediately, instead of waiting for the Pod to be reported to K8s API. As NetworkPolicyController relies on that event as well, we introduce a channel implementation which supports multiple subscribers. Fixes antrea-io#3361 Signed-off-by: Quan Tian <[email protected]>
/test-all |
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.
LGTM
Previously antrea-controller included a Pod in an EgressGroup only when its IP has been presented in K8s API. If a Pod tries to access external right after it's up, Node IP will be used as the SNAT IP even when an Egress applying to it has been created because its Pod IP may haven't been reported to K8s API or antrea-controller may haven't included the Pod in the EgressGroup. This patch fixes it by making CNIServer notify EgressController that it has processed CNI ADD request of a Pod, then EgressController can reconcile the corresponding Egress immediately, instead of waiting for the Pod to be reported to K8s API. As NetworkPolicyController relies on that event as well, we introduce a channel implementation which supports multiple subscribers. Fixes antrea-io#3361 Signed-off-by: Quan Tian <[email protected]>
Previously antrea-controller included a Pod in an EgressGroup only when
its IP has been presented in K8s API. If a Pod tries to access external
right after it's up, Node IP will be used as the SNAT IP even when an
Egress applying to it has been created because its Pod IP may haven't
been reported to K8s API or antrea-controller may haven't included the
Pod in the EgressGroup.
This patch fixes it by making CNIServer notify EgressController that it
has processed CNI ADD request of a Pod, then EgressController can
reconcile the corresponding Egress immediately, instead of waiting for
the Pod to be reported to K8s API. As NetworkPolicyController relies on
that event as well, we introduce a channel implementation which supports
multiple subscribers.
Fixes #3361
Signed-off-by: Quan Tian [email protected]