build.sh: move chmod /usr/lib/containers/storage/overlay-* to install_rpms
#4322
Conversation
When we update the release index, plume checks if the release we want to add already exists by checking for a release with the same version _and_ metadata URL. The latter check seems unnecessary and in fact harmful because there shouldn't ever be two releases in the list with the same version number at all. So drop that check. Noticed this when testing that code locally (with `--local-mode`) in which we don't pass S3 information, so it's normal that the metadata URL isn't the same.
…l_rpms` We already have a few other `chmod`s in `install_rpms` to loosen restrictions. This should fix supermin for people like me who run cosa in their pet containers and only run _some_ of the build.sh commands (notably we run `install_rpms`, but not `configure_user`).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily moves chmod commands in build.sh to a more appropriate function, which is a good improvement for workflows that don't run the full build script. However, there's also an unrelated change in mantle/cmd/plume/release.go that modifies release comparison logic. This change is not documented and should be explained or moved to a separate pull request. I've added specific comments on both files.
| } | ||
|
|
||
| func compareStaticReleaseInfo(a, b release.IndexRelease) bool { | ||
| if a.Version != b.Version || a.MetadataURL != b.MetadataURL { |
There was a problem hiding this comment.
This change seems unrelated to the PR's stated goal of modifying build.sh. Removing the MetadataURL comparison is a significant logical change that could have unintended side effects. Could you please explain the reasoning for this? It would be best to revert this change and submit it as a separate pull request with a proper description and justification.
|
@jlebon: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
We already have a few other
chmods ininstall_rpmsto loosen restrictions.This should fix supermin for people like me who run cosa in their pet containers and only run some of the build.sh commands (notably we run
install_rpms, but notconfigure_user).