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

Adds support for EndpointSlices API in master #959

Merged
merged 1 commit into from
May 21, 2023

Conversation

lafolle
Copy link
Contributor

@lafolle lafolle commented Nov 10, 2022

Adds support for endpoint slices api to fetch endpoints.

Corresponding clusterrole changes haproxy-ingress/charts#66

@lafolle lafolle marked this pull request as draft November 10, 2022 21:13
@lafolle lafolle changed the title Adds support for EndpointSlice API Adds support for EndpointSlices API in master Nov 11, 2022
@monkeymorgan
Copy link

Note that @lafolle is not currently working on this (The 'in progress' comment might make you think there's more to come). He's away for the foreseeable, but I can try and answer any follow ups.

@jcmoraisjr
Copy link
Owner

Thanks for the update. I'll do my best to add it in the first v0.15 snapshot and I'll also try to add it in the first v0.14 stable or maybe in the first patch of that branch.

@lafolle lafolle force-pushed the support-endpointslices-api branch from 9d6fd22 to f3f598f Compare January 27, 2023 07:12
@lafolle lafolle marked this pull request as ready for review February 6, 2023 23:56
Copy link
Owner

@jcmoraisjr jcmoraisjr left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! Sorry about taking so much time to revise it.

See a few comments below.

pkg/common/ingress/controller/launch.go Outdated Show resolved Hide resolved
pkg/controller/listers.go Outdated Show resolved Hide resolved
pkg/converters/configmap/tcpservices.go Outdated Show resolved Hide resolved
pkg/converters/types/interfaces.go Outdated Show resolved Hide resolved
pkg/converters/types/options.go Outdated Show resolved Hide resolved
@jcmoraisjr
Copy link
Owner

Folks, needed to merge #933, sorry. You can see some conflicts but all of them should be pretty simple to fix. The main issue however is that all the lister, informer and cache code base need to be written for the new controller, but you don't need to worry about that, I can take care of that if you prefer. Just run the controller with HAPROXY_INGRESS_RUNTIME=LEGACY, or adjust the code below, to make your local test.

if strings.ToUpper(os.Getenv("HAPROXY_INGRESS_RUNTIME")) != "LEGACY" {
launch.Run()
return
}
hc := legacy.NewHAProxyController()

@lafolle lafolle force-pushed the support-endpointslices-api branch from 1ee353e to c24886f Compare April 24, 2023 21:52
@lafolle lafolle closed this Apr 24, 2023
@lafolle lafolle force-pushed the support-endpointslices-api branch from c24886f to 868db8e Compare April 24, 2023 22:36
@lafolle lafolle reopened this Apr 24, 2023
@lafolle
Copy link
Contributor Author

lafolle commented Apr 26, 2023

Linter seems to be failing with new golangci-lint 1.52.2 while it passes for 1.51.2. Two options: 1) fix haproxy-ingress to satisfy the new linter 2) pin golangci-lint to 1.51.x.

@lafolle lafolle requested a review from jcmoraisjr April 26, 2023 00:31
Copy link
Owner

@jcmoraisjr jcmoraisjr left a comment

Choose a reason for hiding this comment

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

Thank you! See below a few more suggestions, and also:

  • please remove revive from .golangci.yaml, this should fix the lint errors. I'll revisit the lint config and also the errors in a future pr;
  • add the new option in command-line.md doc file;
  • try to squash all the commits into a single one.

After that I'm done and we can merge. I'll prepare a cherry-pick for v0.14 later, as well as the v0.15 controller implementation. Thanks for all the effort put in this controller, that's very much appreciated.

pkg/common/ingress/controller/launch.go Outdated Show resolved Hide resolved
pkg/controller/legacy/cache.go Show resolved Hide resolved
pkg/converters/types/options.go Outdated Show resolved Hide resolved
pkg/converters/utils/services.go Outdated Show resolved Hide resolved
@lafolle lafolle force-pushed the support-endpointslices-api branch from 65c4623 to af4a740 Compare May 16, 2023 18:46
@lafolle lafolle requested a review from jcmoraisjr May 18, 2023 22:28
@jcmoraisjr
Copy link
Owner

LGTM thanks! Merging now, v0.14 cherry-pick and v0.15 controller implementation coming soon.

@jcmoraisjr jcmoraisjr merged commit 057bfa2 into jcmoraisjr:master May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants