Fix invalid prerelease characters when parse kernel version #2450

Jexf · 2021-07-22T09:02:39Z

Check kernel version splitted array length and intercept the first 3 elements for parsing kernel version.

Signed-off-by: Zhengdong Wu zhengdong.wu@transwarp.io

tnqn

Thanks @Jexf for fixing it. Can you link the PR to its issue by adding the keyword "Fixes #xx" in the PR description? So that it can be more clear what's this for and merging it can close that issue automatically.

tnqn · 2021-07-22T16:39:54Z

pkg/util/runtime/runtime_linux.go

+	}
+	patch := regexp.MustCompilePOSIX(`^[0-9]+`).FindString(verStrs[2])
+	if patch == "" {
+		verStrs[2] = "0"


Is there a case the kernel release doesn't have patch number?

Is there a case the kernel release doesn't have patch number?

Maybe there is no need to check kernel patch number, the kernel patch usually be numbers, and it will lose prerelease value, I will remove it.

pkg/util/runtime/runtime_linux.go

Jexf · 2021-07-23T03:25:42Z

/test-all
/test-ipv6-all
/test-ipv6-only-all

Jexf · 2021-07-23T03:56:40Z

/test-all
/test-ipv6-all
/test-ipv6-only-all

codecov-commenter · 2021-07-23T05:35:21Z

Codecov Report

Merging #2450 (294a628) into main (9adf9c9) will increase coverage by 5.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2450      +/-   ##
==========================================
+ Coverage   59.79%   64.88%   +5.08%     
==========================================
  Files         284      284              
  Lines       22171    25419    +3248     
==========================================
+ Hits        13258    16493    +3235     
+ Misses       7490     7374     -116     
- Partials     1423     1552     +129

Flag	Coverage Δ
e2e-tests	`56.00% <42.85%> (?)`
kind-e2e-tests	`46.84% <0.00%> (-0.21%)`	⬇️
unit-tests	`42.02% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/util/runtime/runtime_linux.go	`76.47% <100.00%> (+76.47%)`	⬆️
pkg/controller/egress/ipallocator/allocator.go	`67.82% <0.00%> (-15.16%)`	⬇️
pkg/controller/networkpolicy/endpoint_querier.go	`77.64% <0.00%> (-13.79%)`	⬇️
pkg/apis/controlplane/v1beta1/conversion.go	`72.44% <0.00%> (-11.89%)`	⬇️
pkg/legacyapis/core/v1alpha2/register.go	`69.23% <0.00%> (-10.77%)`	⬇️
pkg/controller/egress/controller.go	`76.76% <0.00%> (-10.44%)`	⬇️
pkg/apis/stats/register.go	`71.42% <0.00%> (-10.39%)`	⬇️
pkg/legacyapis/stats/register.go	`71.42% <0.00%> (-10.39%)`	⬇️
pkg/ovs/openflow/ofctrl_meter.go	`33.84% <0.00%> (-10.16%)`	⬇️
pkg/legacyapis/security/v1alpha1/register.go	`73.33% <0.00%> (-10.00%)`	⬇️
... and 269 more

tnqn

I have a question about why there is a special handling for "5.0" case but not for "5".

pkg/util/runtime/runtime_linux.go

tnqn

LGTM
@antoninbas since you added the function, could you please take a look?

pkg/util/runtime/runtime_linux.go

antoninbas

LGTM, but do you think we could have a quick unit test for this function now that we have added some logic? Should be very quick to build a table-driven test.

Jexf · 2021-07-26T03:12:11Z

Thanks for reminding, I have added unit test, please review again : ) @antoninbas @tnqn

antoninbas

LGTM

antoninbas

I actually had a couple of comments

I noticed that you didn't sign the commit, so the DCO check is failing. https://github.com/antrea-io/antrea/blob/main/CONTRIBUTING.md#sign-off-your-work

antoninbas · 2021-07-26T17:17:58Z

pkg/util/runtime/runtime_linux_test.go

+		if tc.expectedError != nil {
+			assert.Equal(t, tc.expectedError, err)
+		} else {
+			assert.Nil(t, err)


should probably be require.Nil to bypass the next check in case of an error

Thanks for reminding, already updated.

antoninbas · 2021-07-26T17:18:12Z

pkg/util/runtime/runtime_linux_test.go

+		},
+	}
+	for _, tc := range testcases {
+		expectedKernelVersion, err := parseKernelVersionStr(tc.kernelString)


s/expectedKernelVersion/parsedKernelVersion

Thanks for reminding, already updated.

tnqn

LGTM, thanks!

tnqn · 2021-07-27T02:38:43Z

@Jexf DCO check failed:

Commit sha: 6e34ef4, Author: Wu zhengdong, Committer: Wu zhengdong; Expected "Wu zhengdong zhengdong.wu@transwarp.io", but got "Zhengdong Wu zhengdong.wu@transwarp.io".

The signed-off author needs to be exactly same as the author information of the commit.

tnqn · 2021-07-27T02:43:07Z

/test-all

Jexf · 2021-07-27T02:43:51Z

@Jexf DCO check failed:

Commit sha: 6e34ef4, Author: Wu zhengdong, Committer: Wu zhengdong; Expected "Wu zhengdong zhengdong.wu@transwarp.io", but got "Zhengdong Wu zhengdong.wu@transwarp.io".

The signed-off author needs to be exactly same as the author information of the commit.

Has been updated : )

Check kernel version splitted array length and intercept the first 3 elements for parsing kernel version. Signed-off-by: Wu zhengdong <zhengdong.wu@transwarp.io>

tnqn · 2021-07-27T03:47:40Z

/test-all

Jexf · 2021-07-27T09:21:51Z

/test-all

antoninbas · 2021-07-27T17:03:09Z

Unrelated Kind test failure

Jexf changed the title ~~Add compatible adaptation to parse kernel version (#2449)~~ Add compatible adaptation to parse kernel version Jul 22, 2021

tnqn reviewed Jul 22, 2021

View reviewed changes

tnqn requested a review from antoninbas July 22, 2021 16:51

Jexf changed the title ~~Add compatible adaptation to parse kernel version~~ Fix invalid prerelease characters when parse kernel version #2449 Jul 23, 2021

Jexf changed the title ~~Fix invalid prerelease characters when parse kernel version #2449~~ Fix invalid prerelease characters when parse kernel version Jul 23, 2021

tnqn reviewed Jul 23, 2021

View reviewed changes

pkg/util/runtime/runtime_linux.go Show resolved Hide resolved

pkg/util/runtime/runtime_linux.go Show resolved Hide resolved

tnqn reviewed Jul 23, 2021

View reviewed changes

pkg/util/runtime/runtime_linux.go Show resolved Hide resolved

antoninbas reviewed Jul 23, 2021

View reviewed changes

Jexf closed this Jul 26, 2021

Jexf reopened this Jul 26, 2021

antoninbas previously approved these changes Jul 26, 2021

View reviewed changes

antoninbas reviewed Jul 26, 2021

View reviewed changes

Jexf dismissed antoninbas’s stale review via f0695e1 July 27, 2021 01:39

tnqn previously approved these changes Jul 27, 2021

View reviewed changes

Jexf dismissed tnqn’s stale review via fd3d70d July 27, 2021 02:40

tnqn previously approved these changes Jul 27, 2021

View reviewed changes

Fix invalid prerelease characters when parsing kernel version (#2449)

294a628

Check kernel version splitted array length and intercept the first 3 elements for parsing kernel version. Signed-off-by: Wu zhengdong <zhengdong.wu@transwarp.io>

Jexf dismissed tnqn’s stale review via 294a628 July 27, 2021 03:08

tnqn approved these changes Jul 27, 2021

View reviewed changes

antoninbas merged commit 52402d3 into antrea-io:main Jul 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix invalid prerelease characters when parse kernel version #2450

Fix invalid prerelease characters when parse kernel version #2450

Jexf commented Jul 22, 2021 •

edited

tnqn left a comment

tnqn Jul 22, 2021

Jexf Jul 23, 2021

Jexf commented Jul 23, 2021

Jexf commented Jul 23, 2021

codecov-commenter commented Jul 23, 2021 •

edited

tnqn left a comment

tnqn left a comment

antoninbas left a comment

Jexf commented Jul 26, 2021

antoninbas left a comment

antoninbas left a comment

antoninbas Jul 26, 2021

Jexf Jul 27, 2021

antoninbas Jul 26, 2021

Jexf Jul 27, 2021

tnqn left a comment

tnqn commented Jul 27, 2021

tnqn commented Jul 27, 2021

Jexf commented Jul 27, 2021

tnqn commented Jul 27, 2021

Jexf commented Jul 27, 2021

antoninbas commented Jul 27, 2021

Fix invalid prerelease characters when parse kernel version #2450

Fix invalid prerelease characters when parse kernel version #2450

Conversation

Jexf commented Jul 22, 2021 • edited

tnqn left a comment

Choose a reason for hiding this comment

tnqn Jul 22, 2021

Choose a reason for hiding this comment

Jexf Jul 23, 2021

Choose a reason for hiding this comment

Jexf commented Jul 23, 2021

Jexf commented Jul 23, 2021

codecov-commenter commented Jul 23, 2021 • edited

Codecov Report

tnqn left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Jexf commented Jul 26, 2021

antoninbas left a comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

antoninbas Jul 26, 2021

Choose a reason for hiding this comment

Jexf Jul 27, 2021

Choose a reason for hiding this comment

antoninbas Jul 26, 2021

Choose a reason for hiding this comment

Jexf Jul 27, 2021

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Jul 27, 2021

tnqn commented Jul 27, 2021

Jexf commented Jul 27, 2021

tnqn commented Jul 27, 2021

Jexf commented Jul 27, 2021

antoninbas commented Jul 27, 2021

Jexf commented Jul 22, 2021 •

edited

codecov-commenter commented Jul 23, 2021 •

edited