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

Fall back to lenient decoding when strict decoding config fails #6156

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented Mar 27, 2024

When upgrading Antrea, the config is usually updated first and there may be some new fields added to it. If a yet update agent crashes during the process, it will fail to restart due to the unknown new fields because we do strict decoding only.

The patch adds lenient decoding when strict decoding fails to tolerate unknown fields and duplicate fields. Other errors like malformed data and unmatched type are still fatal.


For reference, kube-proxy and kubelet have been doing this since 1.17:
https://github.com/kubernetes/kubernetes/blob/227c2e7c2b2c05a9c8b2885460e28e4da25cf558/cmd/kube-proxy/app/server.go#L491
https://github.com/kubernetes/kubernetes/blob/227c2e7c2b2c05a9c8b2885460e28e4da25cf558/pkg/kubelet/kubeletconfig/util/codec/codec.go#L76

@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label Mar 27, 2024
@tnqn tnqn added this to the Antrea v2.0 release milestone Mar 27, 2024
luolanzone
luolanzone previously approved these changes Mar 27, 2024
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

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

LGTM

@luolanzone
Copy link
Contributor

The test is failed in Kind for UBI / build-and-test-ubi (pull_request).

@tnqn
Copy link
Member Author

tnqn commented Mar 27, 2024

The test is failed in Kind for UBI / build-and-test-ubi (pull_request).

The job tested wrong image, fixing it in #6157

antoninbas
antoninbas previously approved these changes Mar 27, 2024
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -58,6 +59,7 @@ func TestBasic(t *testing.T) {
t.Run("testGratuitousARP", func(t *testing.T) { testGratuitousARP(t, data, data.testNamespace) })
t.Run("testClusterIdentity", func(t *testing.T) { testClusterIdentity(t, data) })
t.Run("testLogRotate", func(t *testing.T) { testLogRotate(t, data) })
t.Run("testForwardCompatibility", func(t *testing.T) { testForwardCompatibility(t, data) })
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would just suggest renaming the test to testConfigForwardCompatibility

Copy link
Member Author

Choose a reason for hiding this comment

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

renamed, thanks

jianjuns
jianjuns previously approved these changes Mar 27, 2024
When upgrading Antrea, the config is usually updated first and there may
be some new fields added to it. If a yet update agent crashes during the
process, it will fail to restart due to the unknown new fields because
we do strict decoding only.

The patch adds lenient decoding when strict decoding fails to tolerate
unknown fields and duplicate fields. Other errors like malformed data
and unmatched type are still fatal.

Signed-off-by: Quan Tian <quan.tian@broadcom.com>
@tnqn
Copy link
Member Author

tnqn commented Mar 28, 2024

/test-all

@tnqn tnqn merged commit a585238 into antrea-io:main Mar 28, 2024
50 of 56 checks passed
@tnqn tnqn deleted the lenient-decoding branch March 28, 2024 13:31
@tnqn tnqn added the kind/bug Categorizes issue or PR as related to a bug. label Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants