feat: ingress annotations support enable websocket #1101

dickens7 · 2022-06-24T01:54:16Z

Type of change:

Bugfix
New feature provided
Improve performance
Backport patches

What this PR does / why we need it:

In our daily usage scenarios, we expect to be able to directly open websocket support for routing on ingress

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

tao12345666333

Thanks,

you should add e2e test cases for this one. you can ref to https://github.com/apache/apisix-ingress-controller/blob/master/test/e2e/suite-features/websocket.go
and add test cases to https://github.com/apache/apisix-ingress-controller/tree/master/test/e2e/suite-annotations directory

codecov-commenter · 2022-06-24T04:12:21Z

Codecov Report

Merging #1101 (427089e) into master (0e1f8d4) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1101      +/-   ##
==========================================
+ Coverage   30.38%   30.42%   +0.04%     
==========================================
  Files          81       81              
  Lines        9390     9396       +6     
==========================================
+ Hits         2853     2859       +6     
  Misses       6222     6222              
  Partials      315      315

Impacted Files	Coverage Δ
pkg/kube/translation/ingress.go	`68.66% <100.00%> (+0.54%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 810f1a1...427089e. Read the comment docs.

dickens7 · 2022-06-24T05:47:49Z

Thanks,

you should add e2e test cases for this one. you can ref to https://github.com/apache/apisix-ingress-controller/blob/master/test/e2e/suite-features/websocket.go and add test cases to https://github.com/apache/apisix-ingress-controller/tree/master/test/e2e/suite-annotations directory

ok, i'll take a look

lingsamuel · 2022-06-28T08:27:56Z

ci failed

dickens7 · 2022-06-29T10:07:48Z

Please help, I have no problem running make unit-test locally

go version go1.18.3 linux/amd64

Linux seven 5.10.102.1-microsoft-standard-WSL2+ #1 SMP Mon Jun 13 04:56:19 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

make unit-test
go test -cover -coverprofile=coverage.txt ./...
?       github.com/apache/apisix-ingress-controller     [no test files]
?       github.com/apache/apisix-ingress-controller/cmd [no test files]
ok      github.com/apache/apisix-ingress-controller/cmd/ingress 10.027s coverage: 80.0% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/api     0.037s  coverage: 85.7% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/api/router      0.045s  coverage: 81.8% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/api/validation  0.050s  coverage: 19.3% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/apisix  0.366s  coverage: 49.3% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/apisix/cache    0.010s  coverage: 83.8% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/config  0.013s  coverage: 64.7% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/id      0.014s  coverage: 100.0% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/ingress 0.031s  coverage: 2.1% of statements
?       github.com/apache/apisix-ingress-controller/pkg/ingress/gateway [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/ingress/gateway/translation     0.226s  coverage: 77.3% of statements
?       github.com/apache/apisix-ingress-controller/pkg/ingress/namespace       [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/ingress/utils   0.052s  coverage: 37.9% of statements
?       github.com/apache/apisix-ingress-controller/pkg/kube    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1      [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2      [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta2 [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta3 [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned  [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/fake     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/scheme   [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2  [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2/fake     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta2     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta2/fake        [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta3     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta3/fake        [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions   [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v2 [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v2beta2    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v2beta3    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/internalinterfaces        [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v2    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v2beta2       [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v2beta3       [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/const       [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/kube/translation        1.951s  coverage: 49.7% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/kube/translation/annotations    0.009s  coverage: 38.8% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/log     0.021s  coverage: 88.5% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/metrics 0.028s  coverage: 59.5% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/types   0.008s  coverage: 87.0% of statements
?       github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1 [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/version 0.003s  coverage: 100.0% of statements

tao12345666333 · 2022-06-29T12:35:00Z

Let me take a look

AlinsRan · 2022-06-30T06:52:40Z

Please help, I have no problem running make unit-test locally

go version go1.18.3 linux/amd64

Linux seven 5.10.102.1-microsoft-standard-WSL2+ #1 SMP Mon Jun 13 04:56:19 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

make unit-test
go test -cover -coverprofile=coverage.txt ./...
?       github.com/apache/apisix-ingress-controller     [no test files]
?       github.com/apache/apisix-ingress-controller/cmd [no test files]
ok      github.com/apache/apisix-ingress-controller/cmd/ingress 10.027s coverage: 80.0% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/api     0.037s  coverage: 85.7% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/api/router      0.045s  coverage: 81.8% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/api/validation  0.050s  coverage: 19.3% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/apisix  0.366s  coverage: 49.3% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/apisix/cache    0.010s  coverage: 83.8% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/config  0.013s  coverage: 64.7% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/id      0.014s  coverage: 100.0% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/ingress 0.031s  coverage: 2.1% of statements
?       github.com/apache/apisix-ingress-controller/pkg/ingress/gateway [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/ingress/gateway/translation     0.226s  coverage: 77.3% of statements
?       github.com/apache/apisix-ingress-controller/pkg/ingress/namespace       [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/ingress/utils   0.052s  coverage: 37.9% of statements
?       github.com/apache/apisix-ingress-controller/pkg/kube    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1      [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2      [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta2 [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta3 [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned  [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/fake     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/scheme   [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2  [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2/fake     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta2     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta2/fake        [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta3     [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/typed/config/v2beta3/fake        [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions   [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v2 [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v2beta2    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v2beta3    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/internalinterfaces        [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v2    [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v2beta2       [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v2beta3       [no test files]
?       github.com/apache/apisix-ingress-controller/pkg/kube/apisix/const       [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/kube/translation        1.951s  coverage: 49.7% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/kube/translation/annotations    0.009s  coverage: 38.8% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/log     0.021s  coverage: 88.5% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/metrics 0.028s  coverage: 59.5% of statements
ok      github.com/apache/apisix-ingress-controller/pkg/types   0.008s  coverage: 87.0% of statements
?       github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1 [no test files]
ok      github.com/apache/apisix-ingress-controller/pkg/version 0.003s  coverage: 100.0% of statements

You can try to merge the latest code

…bsocket

dickens7 · 2022-06-30T16:39:26Z

You can try to merge the latest code

Found the problem after merging the latest code

tao12345666333 · 2022-07-04T02:55:40Z

Thanks.

It's on my list. I will finish review today.

tao12345666333

LGTM

let's move forward.

Thanks!

dickens7 added 3 commits June 24, 2022 09:48

feat: annotations support enable websocket

f710783

feat: annotations enable websocket test case

e6d7de6

docs: ingress annotations enable websocket

fd975fd

tao12345666333 requested review from tokers, tao12345666333, gxthrj and lingsamuel June 24, 2022 04:07

tao12345666333 added the area/controller label Jun 24, 2022

tao12345666333 reviewed Jun 24, 2022

View reviewed changes

tao12345666333 added the enhancement New feature or request label Jun 24, 2022

dickens7 added 3 commits June 26, 2022 23:12

feat: add annotations websocket e2e test case

b2d342b

fix: cases naming

b684cc2

docs: fix markdown lint

672322c

fix: e2e test

4330f05

dickens7 added 2 commits July 1, 2022 00:32

Merge remote-tracking branch 'origin/master' into feat-annotations-we…

ef94efe

…bsocket

fix: translate ingress skipVerify

427089e

tao12345666333 self-assigned this Jul 4, 2022

lingsamuel approved these changes Jul 4, 2022

View reviewed changes

tao12345666333 approved these changes Jul 4, 2022

View reviewed changes

tao12345666333 merged commit aae2105 into apache:master Jul 4, 2022

tao12345666333 added this to the 1.5.0 milestone Jul 4, 2022

dickens7 deleted the feat-annotations-websocket branch July 5, 2022 01:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ingress annotations support enable websocket #1101

feat: ingress annotations support enable websocket #1101

dickens7 commented Jun 24, 2022 •

edited

tao12345666333 left a comment

codecov-commenter commented Jun 24, 2022 •

edited

dickens7 commented Jun 24, 2022

lingsamuel commented Jun 28, 2022

dickens7 commented Jun 29, 2022

tao12345666333 commented Jun 29, 2022

AlinsRan commented Jun 30, 2022

dickens7 commented Jun 30, 2022

tao12345666333 commented Jul 4, 2022

tao12345666333 left a comment

feat: ingress annotations support enable websocket #1101

feat: ingress annotations support enable websocket #1101

Conversation

dickens7 commented Jun 24, 2022 • edited

Type of change:

What this PR does / why we need it:

Pre-submission checklist:

tao12345666333 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 24, 2022 • edited

Codecov Report

dickens7 commented Jun 24, 2022

lingsamuel commented Jun 28, 2022

dickens7 commented Jun 29, 2022

tao12345666333 commented Jun 29, 2022

AlinsRan commented Jun 30, 2022

dickens7 commented Jun 30, 2022

tao12345666333 commented Jul 4, 2022

tao12345666333 left a comment

Choose a reason for hiding this comment

dickens7 commented Jun 24, 2022 •

edited

codecov-commenter commented Jun 24, 2022 •

edited