-
Notifications
You must be signed in to change notification settings - Fork 376
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
Promote ServiceExternalIP to beta #6903
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,6 +120,7 @@ const ( | |
SecondaryNetwork featuregate.Feature = "SecondaryNetwork" | ||
|
||
// alpha: v1.5 | ||
// beta: v2.3 | ||
// Enable controlling Services with ExternalIP. | ||
ServiceExternalIP featuregate.Feature = "ServiceExternalIP" | ||
|
||
|
@@ -209,7 +210,7 @@ var ( | |
Multicast: {Default: true, PreRelease: featuregate.Beta}, | ||
Multicluster: {Default: false, PreRelease: featuregate.Alpha}, | ||
SecondaryNetwork: {Default: false, PreRelease: featuregate.Alpha}, | ||
ServiceExternalIP: {Default: false, PreRelease: featuregate.Alpha}, | ||
ServiceExternalIP: {Default: true, PreRelease: featuregate.Beta}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to update the value of ServiceExternalIP feature in the antrea-agent.conf and antrea-controller.conf, and regenerate manitests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unit test is failed. |
||
TrafficControl: {Default: false, PreRelease: featuregate.Alpha}, | ||
IPsecCertAuth: {Default: false, PreRelease: featuregate.Alpha}, | ||
ExternalNode: {Default: false, PreRelease: featuregate.Alpha}, | ||
|
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.
The phrasing is a bit strange. I would expect users to be able to keep using MetalLB with Antrea without having to make any specific configuration change, even after we graduate the feature to Beta.
My understanding is that despite the above warning about "conflicts with MetalLB", as long as the Services are not annotated with
service.antrea.io/external-ip-pool
, Antrea will not try to allocate LB IPs for the Services and there will be no conflict. However, the "check" kind of happens late IMO, which means we are doing work for nothing and some logs may create confusion for users:antrea/pkg/controller/serviceexternalip/controller.go
Lines 501 to 502 in afb9dfc
I'd like to hear @tnqn's opinion on this. Do we feel like the annotation is enough to "gate" that feature once the FeatureGate itself is promoted to Beta (keep in mind that the long term goal after promoting a feature to Beta should to go to GA, at which point the FeatureGate stops being available altogether in theory). Or do we need a boolean configuration parameter, as a way to explicitly enable / disable the feature (as we have done in other places). Typically we do not need the boolean when the feature is API-driven, but this case is slightly different.