-
Notifications
You must be signed in to change notification settings - Fork 363
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
Tidy up deprecated options #5158
Conversation
19908c1
to
2883328
Compare
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 would suggest to not make the change until 2.0, because currently all options are uncommented in antrea-config even if they are never changed by users explicitly. There is a risk that if users keep their configmap which is created when creating the cluster and deploying antrea, upgrading to a version that removes these options would fail to start, unless they manually update the configmap.
Sure, I can add this to milestone 2.0 when it's ready. |
This PR is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days |
This PR is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days |
b308e93
to
fa9ed9d
Compare
dad8e7d
to
a2284df
Compare
@@ -484,13 +480,6 @@ func (o *Options) setK8sNodeDefaultOptions() { | |||
} | |||
|
|||
if features.DefaultFeatureGate.Enabled(features.Multicluster) { | |||
if o.config.Multicluster.Enable { |
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.
we should have had a log message about this being deprecated, but as you pointed out Multicluster is still Alpha so I am fine with this change.
Following options are removed since they have been deprecated for a long time. ``` enableIPSecTunnel nplPortRange multicastInterfaces multicluster.enable legacyCRDMirroring ``` Signed-off-by: Lan Luo <luola@vmware.com>
a2284df
to
6730aeb
Compare
/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
Rebased it on main and tested with a legacy ConfigMap that contains the removed options, Pods ran well due to lenient decoding. So it should be quite safe to remove them. BTW, |
Following options are removed since they have been deprecated for a long time.
multicluster.enable is deprecated after 1.10 around 5 months ago, but it maybe safe to remove since it's still in alpha stage.
For #5070