namespaces test - refactoring and cleanup - #3186
Conversation
Followup to podman-container-tools#3173 - just a little further cleanup of idmapping test. Plus, oops, fix a longstanding error-failure bug Signed-off-by: Ed Santiago <santiago@redhat.com>
edsantiago
left a comment
edsantiago
left a comment
There was a problem hiding this comment.
Hints for reviewers.
| run_buildah run $RUNOPTS --net=container "$ctr" readlink /proc/self/ns/net | ||
| if [[ $output == $mynetns ]]; then | ||
| expect_output "[output should not be '$mynetns']" | ||
| die "[/proc/self/ns/net (--net=container) should not be '$mynetns']" |
There was a problem hiding this comment.
Oops: this and the next were mistakes I made in #2029
| idmapping_check_namespace() { | ||
| local _uidmapargs=$1 | ||
| local _gidmapargs=$2 | ||
| local _mynamespace=$3 | ||
| local _output=$4 |
There was a problem hiding this comment.
This is now a localized function, defined within the appropriate for loop, because the uid/gid/mynamespace args really should not be args
| idmapping_check_map() { | ||
| local _output_uidmap=$1 | ||
| local _output_gidmap=$2 | ||
| local _expect_uidmap=$3 | ||
| local _expect_gidmap=$4 |
There was a problem hiding this comment.
Converted to check a generic X id map, to avoid the duplicate complicated sed lines. It's better IMO to have two calls to this function, one with UID and one with GID, than to duplicate the complicated sed stuff.
|
|
||
| expect_output --from="${_output_file_stat}" "1:1" "Check if a copied file gets the right permissions" | ||
| expect_output --from="${_output_dir_stat}" "0:0" "Check if a copied directory gets the right permissions" | ||
| expect_output --from="${_output_otherfile_stat}" "${_expect_otherfile_stat}" "Check if another copied file gets the right permissions" |
There was a problem hiding this comment.
There really is no reason for this to be in a helper; it just complicates arg passing.
|
LGTM |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Holy sed! Let's hope the uid/gidmap format never ever changes 😀 /lgtm |
5 participants
Followup to #3173 - just a little further cleanup of idmapping
test. Plus, oops, fix a longstanding error-failure bug
Signed-off-by: Ed Santiago santiago@redhat.com