-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
build: use the system provided LuaJIT on s390x #9172
Conversation
eb0d7dd
to
4153987
Compare
Hi, @cosmo0920, would you please help to review this PR? Thanks! |
4153987
to
41a50b9
Compare
1e3011c
to
484a7ad
Compare
Hi, @cosmo0920, I just updated the doc and the warning message in the code change, would you please help to trigger the CI again? Thanks! |
CI is already triggered automatically. |
@cosmo0920, How does the change look to you? Could you please give an approval? |
README.md
Outdated
@@ -19,7 +19,7 @@ Fluent Bit allows to collect log events or metrics from different sources, proce | |||
|
|||
Fluent Bit comes with full SQL [Stream Processing](https://docs.fluentbit.io/manual/stream-processing/introduction) capabilities: data manipulation and analytics using SQL queries. | |||
|
|||
Fluent Bit runs on x86_64, x86, arm32v7, and arm64v8 architectures. | |||
Fluent Bit runs on x86_64, x86, s390x, arm32v7, and arm64v8 architectures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't specify s390x as a tier1 support. So, personally, I didn't want to declare here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't support s390x, we don't have any infrastructure or CI runners for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
we don't support s390x, we don't have any infrastructure or CI runners for it.
@edsiper , The IBM LinuxONE Community Cloud provides free access to developers, registering an account and creating a s390x VM costs less than 30 minutes. https://community.ibm.com/zsystems/l1cc/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't support s390x, we don't have any infrastructure or CI runners for it.
Hi, @edsiper, would you please clarify what do you mean by fluent bit don't support s390x? Do you mean
- the change in the readme, or,
- you won't support any change made for s390x and thus not merge this PR?
-
If for the firts reason, I have updated the readme already,
-
If for the second reason, IBM has provided free s390x infrastructures for OSS communities here https://community.ibm.com/zsystems/l1cc/, we can work together to add Z nodes into your CI pipelines.
-
The last thing I want to mention is that the code change in this PR won't break anything outside of s390x platform, it just allows the user to build fluent bit with system luajit lib, it should be OK to be merged, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to mention that I have demoed this change to @pwhelan and Adriana in the community meeting last week and it works perfectly on Z.
so would you guys please help to merge this PR? @edsiper @pwhelan @cosmo0920
Hi, @edsiper @cosmo0920 @pwhelan , would you please help to review again?
test case output of flb-rt-filter_lua
|
@leonardo-albertovich, would you please try this PR on your s390x vm and give it a try?
|
@rightblank my last concern to fix is the word
|
Signed-off-by: YingJie Fu <[email protected]>
b001711
to
d265f08
Compare
https://github.com/openresty/luajit2?tab=readme-ov-file#description
|
And if fluent bit uses openresty/luajit2, fluent bit can even support embedded LuaJIT for s390x. |
Lua Test case output with openresty/luajit2
|
cc: @tarruda |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not familiar with s390x, but LGTM
Hi, @edsiper, would you please merge this PR? |
Signed-off-by: YingJie Fu <[email protected]>
Hi, @edsiper, this PR follows #7286 to allow to use system provided LuaJIT on s390x, please help to review and merge it, thanks!
Enter
[N/A]
in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
fluent.conf
test.lua
fake.log
Debug output
Valgrind output
ok-package-test
label to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.