fix(build): metrics compilation after refactoring into classes #3265
Conversation
|
/hold |
Molter73
changed the title
|
Thanks Mauro for taking care of this! |
|
I'm a bit confused as to why those driver tests are failing, these PRs don't do any changes to driver code or its corresponding userspace components. Any ideas? |
|
It happens sometimes; probably mismatch between kernel headers package and runner kernel (or bugged headers package); rnothing to worry about for you! Restarted :) |
|
Kmod failed again, but I need to fix the cmake files after I merge the libs PR anyways. @FedeDP, what do you think? Am I OK to unhold the libs PR? |
|
Yes no problem, the issue is not dependent on your changes! |
Molter73
force-pushed
the
fix-metrics-compilation
branch
from
19a07cd to
a6bf54d
Compare
|
/unhold |
|
@Molter73 we usually have the habit to bump both libs and driver during the dev phase. Few times we didn't do it and it caused compatibility issues. If you agree we can just continue bumping both together? |
|
/milestone 0.39.0 |
|
Sorry it took a bit, I've bumped the driver version so it matches libs. |
|
I'm guessing the error I'm getting is related to this commit: falcosecurity/libs@fac0ae4 How do we want to handle it? |
|
Yep! I already tagged Gerald in that PR to ask him to double check ;) |
Molter73
force-pushed
the
fix-metrics-compilation
branch
from
970522f to
777f01c
Compare
|
One of these days I'll get this PR to pass CI... |
|
/hold for possible patches #3271 (comment). Do not merge prior to having made a decision. |
Signed-off-by: Mauro Ezequiel Moltrasio <[email protected]>
Molter73
force-pushed
the
fix-metrics-compilation
branch
from
777f01c to
a45a584
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Molter73 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I'm guessing the 3 issues are due to some change that bled in with the latest libs bump, I was having some issues linking unit tests locally, I'll look into them when I get a minute. |
|
@jasondellaluce looks like the unit test errors I got are due to some warning messages being changed in falcosecurity/libs#1831, could you double check I got those right in the last commit? |
|
Now it looks like ASAN is not very happy, tried to run the tests locally with a debug build and the errors seem related to falcosecurity/libs#1992, would appreciate some confirmation. |
|
@Molter73 AFAIK the ASan failures are fixed by falcosecurity/libs#1992 , I'm writing some tests for those and will merge the PR soon. I'm not sure why the asan crashes started appearing now. |
|
No worries, I'll update this PR after you merge! |
Signed-off-by: Mauro Ezequiel Moltrasio <[email protected]>
Signed-off-by: Mauro Ezequiel Moltrasio <[email protected]>
Molter73
force-pushed
the
fix-metrics-compilation
branch
from
0bc43d7 to
c054cea
Compare
|
I'm gonna need some help understanding the last errors I'm getting, seems something has been broken with the legacy rules? |
|
This is the same error we are gettng on #3283 . Needs further investigation... |
|
Closing in favor of #3283, let me know if any help is needed on that PR! |
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area build
What this PR does / why we need it:
Once falcosecurity/libs#1920 is merged, compilation of falco will be broken due to the
new_metricmethod changing the class it lives in. This PR makes falco compatible with this change.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: The commit that changes the libs cmake files will be dropped and replaced with a commit that properly updates the libs commit and SHA once falcosecurity/libs#1920 is merged, please disregard these changes until that merge occurs.
Does this PR introduce a user-facing change?: