Remove bootstrap.sh and collector-wrapper.sh #1814
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These two scripts have been pretty much redundant for some time now. bootstrap.sh used to have the responsibility of removing the kernel module when collector stopped and printing some additional system information, but we no longer support kernel modules and printing the system information can be done with the C++ binary all the same. collector-wrapper.sh gave the ability to manipulate how the collector binary was called, allowing things like running collector under valgrind. Without bootstrap.sh, the same thing can be achieved with entrypoint and command.
ovalenti
approved these changes
Aug 29, 2024
| set(COLLECTOR_VERSION "0.0.0") | ||
| endif() | ||
|
|
||
| configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Version.h.in ${CMAKE_CURRENT_BINARY_DIR}/Version.h) |
There was a problem hiding this comment.
I am curious about when this gets reevaluated. For instance, in a dev environment, is the Collector version going to remain the same until the project is reconfigured ?
There was a problem hiding this comment.
From the cmake docs:
If the input file is modified the build system will re-run CMake to re-configure the file and generate the build system again. The generated file is modified and its timestamp updated on subsequent cmake runs only if its content is changed.
So it will retrigger if you change the file or manually re-run cmake. The way we use it right now running with our make files, this file gets re-created on every make image call.
Molter73
added a commit
that referenced
this pull request
Sep 4, 2024
PR #1814 removed the scripts used by collector to run, making it possible for it to be run directly. The Konflux build is still attempting to copy the scripts directory into the image even though it doesn't exist anymore, so we should remove that line from the dockerfile. As an additional cleanup, since we have been linking falco statically for some time now, there's no need to call ldconfig anymore.
6 tasks
Molter73
added a commit
that referenced
this pull request
Sep 4, 2024
Yet another mistake from #1814. The version of collector is now hardcoded into the binary, but that needs to be set at compile time, which was not being done in konflux builds. The label with the version was being set properly, this only affects collector printing its version to the logs.
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
These two scripts have been pretty much redundant for some time now.
bootstrap.sh used to have the responsibility of removing the kernel module when collector stopped and printing some additional system information, but we no longer support kernel modules and printing the system information can be done with the C++ binary all the same.
collector-wrapper.sh gave the ability to manipulate how the collector binary was called, allowing things like running collector under valgrind. Without bootstrap.sh, the same thing can be achieved with entrypoint and command.
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
CI should be enough, but just in case I deployed collector and checked it worked correctly.
Also compared the previous log output with the new one to check the information is the same:
New log
Old log