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

chore: endpointslice controller #574

Merged
merged 24 commits into from
Jul 10, 2021

Conversation

tokers
Copy link
Contributor

@tokers tokers commented Jul 7, 2021

Please answer these questions before submitting a pull request

  • Why submit this pull request?

  • Bugfix

  • New feature provided

  • Improve performance

  • Backport patches

  • Related issues


Bugfix

  • Description

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

Backport patches

  • Why need to backport?

  • Source branch

  • Related commits and pull requests

  • Target branch

@tao12345666333
Copy link
Member

tao12345666333 commented Jul 8, 2021

Can you rebase or merge master? When #563 be merged

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2021

Codecov Report

Merging #574 (2126fd3) into master (a754f69) will decrease coverage by 0.71%.
The diff coverage is 0.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #574      +/-   ##
==========================================
- Coverage   34.90%   34.18%   -0.72%     
==========================================
  Files          55       55              
  Lines        4558     4677     +119     
==========================================
+ Hits         1591     1599       +8     
- Misses       2744     2858     +114     
+ Partials      223      220       -3     
Impacted Files Coverage Δ
pkg/ingress/controller.go 0.92% <0.00%> (-0.20%) ⬇️
pkg/ingress/endpoint.go 0.00% <0.00%> (ø)
pkg/ingress/endpointslice.go 0.00% <0.00%> (ø)
cmd/ingress/ingress.go 77.10% <100.00%> (+0.27%) ⬆️
test/e2e/e2e.go
pkg/apisix/cluster.go 25.17% <0.00%> (+1.79%) ⬆️
pkg/apisix/route.go 35.29% <0.00%> (+2.20%) ⬆️

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 a754f69...2126fd3. Read the comment docs.

@tokers
Copy link
Contributor Author

tokers commented Jul 9, 2021

Can you rebase or merge master? When #563 be merged

Done.

@tokers tokers changed the title Chore/endpointslice controller chore: endpointslice controller Jul 9, 2021
@@ -49,7 +49,7 @@ Tips: The failure caused by empty upstream nodes is a limitation of Apache APISI

6. What is the retry rule of `apisix-ingress-controller`?

If an error occurs during the process of `apisix-ingress-controller` parsing CRD and distributing the configuration to APISIX, a retry will be triggered.
If an error occurs duriREADME.mdng the process of `apisix-ingress-controller` parsing CRD and distributing the configuration to APISIX, a retry will be triggered.
Copy link
Contributor

Choose a reason for hiding this comment

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

Misspell

@@ -504,6 +516,85 @@ func (c *Controller) syncConsumer(ctx context.Context, consumer *apisixv1.Consum
}
return
}

func (c *Controller) syncEndpoint(ctx context.Context, ep kube.Endpoint) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why write sync in the controller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because these logics can be reused by both the endpoint controller and endpointslice controller.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean put them in another file maybe better , not in controller .

@@ -256,6 +264,7 @@ spec:
- %s
- --apisix-route-version
- %s
- --watch-endpointslices
Copy link
Contributor

Choose a reason for hiding this comment

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

The default value of watch-endpointslices is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The minimal Kubernetes version that we supporting is v1.15 while the alpha version of endpointslice was introduced in Kubernetes v1.16. So by default we disable it.

Copy link
Contributor

@gxthrj gxthrj left a comment

Choose a reason for hiding this comment

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

LGTM, Except for some minor issues

@@ -58,7 +58,7 @@ This project is currently general availability.

## Prerequisites

Apisix ingress controller requires Kubernetes version 1.14+.
Apisix ingress controller requires Kubernetes version 1.15+.
Copy link
Member

Choose a reason for hiding this comment

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

@gxthrj gxthrj merged commit 67f3fd9 into apache:master Jul 10, 2021
fgksgf pushed a commit to fgksgf/apisix-ingress-controller that referenced this pull request Aug 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants