migrate auth_test to common

[chaochn47]

Related to #13637

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1202764322183583

[chaochn47]

@serathius Can I get some early feebacks if the common test framework auth implementation makes sense to you? Thanks!

[codecov-commenter]

Codecov Report

Merging #14041 (d5f2696) into main (9cc2f64) will decrease coverage by 1.15%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #14041      +/-   ##
==========================================
- Coverage   75.80%   74.65%   -1.16%     
==========================================
  Files         457      457              
  Lines       37421    37391      -30     
==========================================
- Hits        28367    27914     -453     
- Misses       7294     7747     +453     
+ Partials     1760     1730      -30     

Flag	Coverage Δ
all	74.65% <ø> (-1.16%)	:arrow_down:

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

Impacted Files	Coverage Δ
etcdctl/ctlv3/command/txn_command.go	6.14% <0.00%> (-57.02%)	:arrow_down:
etcdctl/ctlv3/command/defrag_command.go	26.92% <0.00%> (-50.00%)	:arrow_down:
server/proxy/grpcproxy/auth.go	44.44% <0.00%> (-44.45%)	:arrow_down:
etcdctl/ctlv3/command/printer_simple.go	19.13% <0.00%> (-33.96%)	:arrow_down:
server/etcdserver/api/v3rpc/auth.go	55.43% <0.00%> (-26.09%)	:arrow_down:
etcdctl/ctlv3/command/ep_command.go	26.11% <0.00%> (-25.56%)	:arrow_down:
server/etcdserver/apply/apply_auth.go	71.00% <0.00%> (-17.00%)	:arrow_down:
etcdctl/ctlv3/command/auth_command.go	54.23% <0.00%> (-16.95%)	:arrow_down:
etcdctl/ctlv3/command/role_command.go	61.29% <0.00%> (-9.68%)	:arrow_down:
server/etcdserver/apply/apply.go	73.39% <0.00%> (-9.18%)	:arrow_down:
... and 30 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ahrtr]

cc @mitake

[chaochn47]

The failed test is unrelated to the PR.

--- FAIL: TestDowngradeUpgradeClusterOf3 (41.84s)

{
	"level": "warn",
	"ts": "2022-08-04T07:33:45.478Z",
	"caller": "embed/serve.go:120",
	"msg": "stopping grpc server due to error",
	"error": "accept tcp 127.0.0.1:20000: use of closed network connection"
}

ref. https://github.com/etcd-io/etcd/runs/7667198807?check_suite_focus=true

The symptom looks like similar to https://github.com/etcd-io/etcd/issues/14275.

However, I think this type of flake is inevitable without embed server restart retry given we run multiple tests in parallel. Kernel and tests races on TCP connection closing, opening.

See this test causes 41.84s while waiting for client connecting to the dead server. Retry server should prevent this type of flake. Do you have any thoughts on this? @serathius

[ahrtr]

Thanks @chaochn47 for the PR. More than 2K line of code change is too big, could you try to break down this PR into small ones?

[ahrtr]

Sorry for the late response. Are you still working on this PR?

[chaochn47]

Thanks @ahrtr for the reminder

Yeah, I am working on the second common test framework changes mentioned above. https://github.com/etcd-io/etcd/pull/14331 but a little confused how to address the last comment raised by Merek. Will keep the PR updated.

[chaochn47]

The latest code is on top of https://github.com/etcd-io/etcd/pull/14626, once it is merged, rebase will make the code diff smaller.

[chaochn47]

Impacted by a flaky test case

=== FAIL: integration TestPeriodicCheckDetectsCorruption
    corrupt_test.go:100: 
        	Error Trace:	corrupt_test.go:100
        	Error:      	Not equal: 
        	            	expected: []*etcdserverpb.AlarmMember{(*etcdserverpb.AlarmMember)(0xc00bb8f9b0)}
        	            	actual  : []*etcdserverpb.AlarmMember{(*etcdserverpb.AlarmMember)(0xc00bb8f950)}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -2,3 +2,3 @@
        	            	  (*etcdserverpb.AlarmMember)({
        	            	-  MemberID: (uint64) 12970263741109204454,
        	            	+  MemberID: (uint64) 11662817823488512165,
        	            	   Alarm: (etcdserverpb.AlarmType) 2,
        	Test:       	TestPeriodicCheckDetectsCorruption

[fuweid]

Impacted by a flaky test case

=== FAIL: integration TestPeriodicCheckDetectsCorruption

    corrupt_test.go:100: 

        	Error Trace:	corrupt_test.go:100

        	Error:      	Not equal: 

        	            	expected: []*etcdserverpb.AlarmMember{(*etcdserverpb.AlarmMember)(0xc00bb8f9b0)}

        	            	actual  : []*etcdserverpb.AlarmMember{(*etcdserverpb.AlarmMember)(0xc00bb8f950)}

        	            	

        	            	Diff:

        	            	--- Expected

        	            	+++ Actual

        	            	@@ -2,3 +2,3 @@

        	            	  (*etcdserverpb.AlarmMember)({

        	            	-  MemberID: (uint64) 12970263741109204454,

        	            	+  MemberID: (uint64) 11662817823488512165,

        	            	   Alarm: (etcdserverpb.AlarmType) 2,

        	Test:       	TestPeriodicCheckDetectsCorruption

Found that root cause and will file pr later.

[ahrtr]

This PR is too big. So it is almost impossible to carefully check each line of code change. But overall looks clear and good to me.

Please try to use github.com/stretchr/testify to simplify the code.

Suggestion for similar PR in future: Please try to migrate the cases step by step.

I am OK if other maintainers agree to merge this PR. So defer to @serathius and @spzala to merge or comment this PR.

[ahrtr]

Another point is that please try to reuse the common functions in auth_util.go. If the existing functions do not meet some requirements, then please try to enhance the common functions, so that we can simplify the test cases as much as possible.

[chaochn47]

Thanks for the review, I will split those test cases into multiple incremental PRs.

Each PR contains 2 test cases.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.