arm64: agent_policy build always pulls amd64 opa binary #8375
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
zvonkok
force-pushed
the
opa-binary-fix
branch
from
cd64f27
to
24eefdc
Compare
zvonkok
force-pushed
the
opa-binary-fix
branch
from
24eefdc
to
f9ea997
Compare
|
/ok-to-test |
|
/test |
zvonkok
force-pushed
the
opa-binary-fix
branch
from
f9ea997
to
cd7b899
Compare
|
/ok-to-test |
|
/test |
|
@fidencio PTAL |
fidencio
reviewed
Jan 15, 2024
fidencio
reviewed
Jan 15, 2024
fidencio
force-pushed
the
opa-binary-fix
branch
from
cd7b899
to
2df3b17
Compare
katacontainersbot
added
size/small
zvonkok
commented
Jan 15, 2024
zvonkok
force-pushed
the
opa-binary-fix
branch
from
c10a06c
to
1fdc5a7
Compare
The versions.yaml has a default for the amd64 binary, but there is no code to actually build the arm64 binary, which seems an overlook. Let's simplify the OPA logic by removing the direct link to the binary, and construct that link as part of the checks we do to decide whether we need to build OPA or not. Fixes: kata-containers#8373 Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com> Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
zvonkok
force-pushed
the
opa-binary-fix
branch
from
1fdc5a7
to
ab597a4
Compare
fidencio
reviewed
Jan 23, 2024
Comment on lines
+710
to
+714
| case ${ARCH} in | ||
| x86_64) opa_binary_arch="amd64" ;; | ||
| aarch64) opa_binary_arch="arm64" ;; | ||
| *) die "Unsupported architecture for the OPA binary" ;; | ||
| esac |
There was a problem hiding this comment.
I proposed this, and I'd like to state that I'm not a big fond of this, as we already have this logic in a lot of places in our repo.
However, this is needed here, and building the OPA agent as part of the rootfs will be short-lived (theoretically), as we want to actually just use a binary that was built in a previous state, but that's not for now.
All in all, lgtm, thanks @zvonkok!
fidencio
approved these changes
Jan 23, 2024
|
Thanks for improving this script @zvonkok ! |
danmihai1
approved these changes
Jan 23, 2024
|
/test |
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.
The versions.yaml has only a default amd64 binary configured extend the versions.yaml for aarch64 as well.
Fixes: #8373