Use Lua5.4 as default version when building haproxy 2.9+

[Darlelet]

Following @TimWolla suggestion in f389054#r119087804:

Encouraging Lua 5.4 adoption by building newer haproxy versions with the lua5.4-dev package.

Versions prior to 2.9 will still be built with the lua5.3-dev package to prevent any breakage.

(only Dockerfile.template was modified, and then ./update.sh was used to update Dockerfiles)

[tianon]

Nice! Is there a good documentation page from upstream we could point to here to help justify these choices? 😅

I think I'd also like to avoid creating another explicit environment variable that I don't think is particularly interesting to downstream users, so I think we should adjust this slightly to use a template function instead:

Suggested change

	{{ if ([ "2.0", "2.2", "2.4", "2.6", "2.7", "2.8" ] | index(env.version)) then ( -}}
	ENV HAPROXY_LUA_VERSION 5.3
	{{ ) else ( -}}
	ENV HAPROXY_LUA_VERSION 5.4
	{{ ) end -}}
	{{
	def lua:
	# TODO some useful upstream link here? 😅
	if ([ "2.0", "2.2", "2.4", "2.6", "2.7", "2.8" ] | index(env.version)) then
	"5.3"
	else
	"5.4"
	end
	-}}

(and then it can be used as {{ lua }} later in the script in the couple places we need it)

[Darlelet]

Thanks for your quick-reply!

Is there a good documentation page from upstream we could point to here to help justify these choices?

Well there are multiple motivations 😄 :

• haproxy offers Lua 5.4 support (haproxy/haproxy@08ed98f) since mid 2020
• Lua 5.3 is EOL since September 2020 (https://www.lua.org/versions.html#5.3)

So it's probably best to consider the 5.4 version as the default one from now on, do you agree?

I think I'd also like to avoid creating another explicit environment variable that I don't think is particularly interesting to downstream users, so I think we should adjust this slightly to use a template function instead:

Of course, thanks for the suggestion is does seem clearer indeed, I didn't know about this as I'm not very familiar with the templating format

About the remaining files in the commit, is it correct to include the Dockerfile modifications from running update.sh or only the template file should be included?

Thanks :)

[tianon]

Ah, interesting. That makes me very seriously wonder if we should actually be bumping to 5.4 more aggressively (like, in all versions 2.3+), but I imagine the user impact of that is probably high? I really don't know much about Lua in general or how it's integrated in HAProxy, sorry. 😅

About the remaining files in the commit, is it correct to include the Dockerfile modifications from running update.sh or only the template file should be included?

Yep, it's correct to include them! However, you might have a more enjoyable inner loop if you only run apply-templates.sh because that won't go look for new versions too. 😁

[Darlelet]

if we should actually be bumping to 5.4 more aggressively (like, in all versions 2.3+)

Well, theoretically there would be no impact (5.3->5.4 upgrade should only expose new features and bugfixes and is not supposed to cause breakage in existing scripts).. but I'm never confident when it comes to changing something that's been stable for a long time, how do you feel about that?

Or perhaps we should only target edge versions (2.7+) to make the risk more acceptable.. I cannot make my mind actually and that's why I initially suggested the 2.9 since it is currently under development 😄

Yep, it's correct to include them! However, you might have a more enjoyable inner loop if you only run apply-templates.sh because that won't go look for new versions too

Copy that, thanks for the info! I'll update the PR shortly once we decided which versions we should be targeting! :)

[Darlelet]

5.3->5.4 upgrade should only expose new features and bugfixes and is not supposed to cause breakage in existing scripts

It's not 100% accurate, there are some minor incompatibilities that could possibly affect existing user scripts:
https://www.lua.org/manual/5.4/manual.html#8

Although being minor, if users don't expect any visible change, then it's probably not safe to change defaults on older stable versions.. right?

[tianon]

I don't think I'm worried too much about "minor incompatibilities" in general here (usage of deprecated/EOL software concerns me a lot, on the other hand).

I do worry that I don't understand enough about how users typically use the Lua functionality of HAProxy and whether they'd be very likely to experience breakage (I'm guessing they wouldn't, but again, I don't actually know at all). Ideally, we'd have something from the HAProxy upstream project saying "we recommend version X.Y of Lua" but I don't think they have any such specific recommendation. 😞

[Darlelet]

Hmm yes I understand your point of view. Sorry to confuse you, let me try to clarify mine so that you can take the proper decision 😄

how users typically use the Lua functionality of HAProxy and whether they'd be very likely to experience breakage

Lua support within haproxy allows users to extend the core functionality by loading custom-made lua scripts defining custom processing/helper functions that haproxy can use during runtime according to haproxy configuration.

Ideally, we'd have something from the HAProxy upstream project saying "we recommend version X.Y of Lua

Indeed, there is no official position on that specific point. But Lua 5.4 is officially supported within haproxy so it should be considered as the recommended version for new setups indeed.

What I was worried about existing docker images: since Lua scripts are custom made, they are not provided by haproxy, but by third-parties or users themselves. As such, due to minor incompatibilities that can be encountered when upgrading from 5.3 to 5.4 I was worried that some old existing scripts might not be working as before, without the user being able to explain it or find support for it (considering the user picked up the script from an article or a github repo, written by someone else at the time 5.3 was the default, but doesn't maintain it anymore).

To sum up: haproxy 2.4+ integration with lua 5.4 itself is fine and should be the default for new setups. But I was a bit concerned about older versions where users might still be deploying haproxy with unmaintained Lua scripts that could stop working properly if enforcing Lua interpereter to 5.4.

Perhaps we could start by enforcing Lua 5.4 for haproxy 2.9+, and then try to slowly introduce 5.4 for older versions as well (up to haproxy 2.4), while considering revert as an option (for 2.4 .. 2.8) if too many users complain that their existing Lua scripts suddenly stopped working and they are not able to fix the script themselves? Or provide 2 separate haproxy binaries within the image? One for Lua 5.3 support, and the other one for 5.4?

With that said, Lua 5.4 is mainly backward-compatible with 5.3 so hopefully this should not be an issue for the vast majority of existing scripts in the wild, I'm just being extra cautious (maybe too much). FYI, haproxytech docker images already enforce Lua 5.4 for haproxy 2.4+ (haproxytech/haproxy-docker-ubuntu@392c51b)

[Darlelet]

I updated the PR so that it now makes use of version.sh to define the lua variable used in the template, instead of hard-coding the selection logic within the template file (taking this opportunity to expose the lua version in versions.json file as well)

Feel free to suggest or performs modifications to it (ie: apply the 5.4 default lua version to more haproxy versions)

[Darlelet]

Well, not sure committing version.json was a good move since every time the bot bumps versions.json there will be merge conflicts 🥲

Apart from that, do you think we are good this time @tianon? Or do you need an official position from haproxy?
Thanks for your help

[tianon]

FYI, haproxytech docker images already enforce Lua 5.4 for haproxy 2.4+ (haproxytech/haproxy-docker-ubuntu@392c51b)

This is really interesting IMO and a pretty good justification for us to do the same, but I'm OK with staying on 5.3 for now if we want to start conservative. 🙇 ❤️

I think I'd rather not put this in versions.json since it's not actually something that we're scraping externally (or that will factor into the tags we generate in generate-stackbrew-library.sh, for example), so I think I'd rather just have a def lua or similar inside Dockerfile.template that defines it, especially since it might have to be different in Debian vs Alpine in the future (given we're relying on their Lua packages). 😅

Maybe something like this, for now (and we can get more complex / opt more versions into our 5.4 default over time)?

def lua:
	if [ "2.0", "2.2", "2.4", "2.6", "2.7", "2.8" ] | index(env.version) then
		"5.3"
	else
		"5.4"
	end
;

(and then {{ .lua }} becomes {{ lua }})

I really appreciate you staying patient with me and helping me understand! Feel free to let me know if at any point here you'd like to punt and have me take over making the actual changes. 😅 ❤️

[Darlelet]

Ok great!

Oh yes that makes sense indeed, I reverted back to previous implementation as you suggested 😄, we're all set!

[tianon]

Extremely appreciate both your patience and persistence!! ❤️