namespaces test - refactoring and cleanup #3186
Conversation
Followup to containers#3173 - just a little further cleanup of idmapping test. Plus, oops, fix a longstanding error-failure bug Signed-off-by: Ed Santiago <[email protected]>
| @@ -63,58 +63,37 @@ load helpers | |||
| # Check that with settings that don't require a user namespace, we can request to use a per-container network namespace. | |||
| 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
Needs 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 |
Followup to #3173 - just a little further cleanup of idmapping
test. Plus, oops, fix a longstanding error-failure bug
Signed-off-by: Ed Santiago [email protected]