[DNM carry/fix #3967] Remove mount fallback flag#3975
Closed
kolyshkin wants to merge 2 commits intoopencontainers:mainfrom
Closed
[DNM carry/fix #3967] Remove mount fallback flag#3975kolyshkin wants to merge 2 commits intoopencontainers:mainfrom
kolyshkin wants to merge 2 commits intoopencontainers:mainfrom
Conversation
Function teardown assumes that every test case will call setup_sshfs. Currently, this assumption is true, but once a test case that won't call setup_sshfs is added (say because it has some extra "requires" or "skip"), it will break bats, so any bats invocation involving such a test case will end up hanging after the last test case is run. The reason is, we have set -u in helpers.bash to help catching the use of undefined variables. In the above scenario, such a variable is DIR, which is referenced in teardown but is only defined after a call to setup_sshfs. As a result, bash that is running the teardown function will exit upon seeing the first $DIR, and thus teardown_bundle won't be run. This, in turn, results in a stale recvtty process, which inherits bats' fd 3. Until that fd is closed, bats waits for test logs. Long story short, the fix is to - check if DIR is set before referencing it; - unset it after unmount. PS it is still not clear why there is no diagnostics about the failed teardown. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The original reasoning for this option was to avoid having mount options be overwritten by runc. However, adding command-line arguments has historically been a bad idea because it forces strict-runc-compatible OCI runtimes to copy out-of-spec features directly from runc and these flags are usually quite difficult to enable by users when using runc through several layers of engines and orchestrators. A far more preferable solution is to have a heuristic which detects whether copying the original mount's mount options would override an explicit mount option specified by the user. In this case, we should return an error. Fixes: da780e4 ("Fix bind mounts of filesystems with certain options set") Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
kolyshkin
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is #3967 with the added commit to fix the issue with the bats test.
For testing purposes.