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

Respect sort-backends flag #241

Closed

Conversation

gjtempleton
Copy link

@gjtempleton gjtempleton commented Oct 23, 2018

Resolves the issue discussed in #240

  • Adds a new field to HAProxyBackendSlots struct called FullSlotIndices of type []string - this is used purely to track the order of processing of endpoint targets

  • Appends new targets to FullSlotIndices as each upstream is processed and added to the backend

  • Updates the template to range over FullSlotIndices for each backend rather than range $target, $slot := $BackendSlots.FullSlots as this causes the map to become ordered alphabetically

  • Tested in a local fork and build with both the default behaviour of sort-backends=false and explicitly setting sort-backends=true

  • Existing configs should continue to work as before as no changes made to the structure of existing data structures used in the template

@jcmoraisjr
Copy link
Owner

Hi, sorry about the delay and thanks for supporting this ingress controller!

The current implementation isn't working with dynamic updates - servers aren't being added to the backend farm. There are other places on dynconfig that FullSlots was updated but FullSlotIndices wasn't.

Besides that I'd also suggest you to convert the FullSlots itself from a map to a slice instead of using another slice (FullSlotIndices) to save the correct order.

@gjtempleton
Copy link
Author

gjtempleton commented Oct 29, 2018

Hi, no worries, and thanks for the feedback.

That's my bad, I didn't test it with dynamic backends as we don't make use of that internally, I'll grab some time today to have another look.

Thanks again for the feedback!

@jcmoraisjr
Copy link
Owner

I've just merged #247 which "unsort" backend in a backward compatible way. I'll shortly notify in #240 as soon as a new tag is pushed and a new image is built. Closing. Please update the issue if you have any problem.

@jcmoraisjr jcmoraisjr closed this Nov 10, 2018
@gjtempleton gjtempleton deleted the Respect-ordering-flag branch January 4, 2019 10:56
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

2 participants