Merge pull request openshift#12942 from eparis/no-chcon-volume-dir

Merged by openshift-bot
jpeeler · Jun 27, 2017 · d0ed5a3 · d0ed5a3
2 parents 3b22095 + 88e4c71
commit d0ed5a3
Show file tree

Hide file tree

Showing 7 changed files with 11 additions and 101 deletions.
diff --git a/docs/debugging-openshift.md b/docs/debugging-openshift.md
@@ -66,20 +66,6 @@ If this shows up in your build logs, restart docker and then resubmit a build:
     $ sudo systemctl restart docker
     $ oc start-build --from-build=<your build identifier>
 
-Another item seen stems from how OpenShift operates in a SELinux environment.  The SELinux policy requires that host directories that are bind mounted have the svirt_sandbox_file_t label.  Generally
-this simply happens for you under the covers, but there is a growing list of user operations which hamper the registry deployment to the point where the svrt_sandbox_file_t label ends up missing, and you can see
-various authentication or push failures.  One example, when initiating a build:
-
-     Failed to push image: Error pushing to registry: Server error: unexpected 500 response status trying to initiate upload of test/origin-ruby-sample
-
-And when inspecting the Docker registry, you will see messages like this:
-
-    173.17.42.1 - - [03/Jun/2015:13:26:19 +0000] "POST /v2/test/origin-ruby-sample/blobs/uploads/ HTTP/1.1" 500 203 "" "docker/1.6.0 go/go1.4.2 kernel/3.17.4-301.fc21.x86_64 os/linux arch/amd64"
-
-When this sequence occurs, without needing to restart Docker nor OpenShift, you can work around it by running the following command:
-
-     $ sudo chcon -R -t svirt_sandbox_file_t < path to >/openshift.local.volumes
-
 Docker Registry
 ---------------
 

diff --git a/hack/build-in-docker.sh b/hack/build-in-docker.sh
@@ -12,16 +12,6 @@ STARTTIME=$(date +%s)
 source "$(dirname "${BASH_SOURCE}")/lib/init.sh"
 origin_path="src/github.com/openshift/origin"
 
-# TODO: Remove this check and fix the docker command instead after the
-#       following PR is merged: https://github.com/docker/docker/pull/5910
-#       should be done in docker 1.6.
-if [ -d /sys/fs/selinux ]; then
-    if ! ls --context "$(absolute_path $OS_ROOT)" | grep --quiet svirt_sandbox_file_t; then
-        os::text::print_red "Warning: SELinux labels are not set correctly; run chcon -Rt svirt_sandbox_file_t $(absolute_path $OS_ROOT)"
-        exit 1
-    fi
-fi
-
-docker run -e "OWNER_GROUP=$UID:$GROUPS" --rm -v "$(absolute_path $OS_ROOT):/go/${origin_path}" openshift/origin-release /usr/bin/openshift-origin-build.sh $@
+docker run -e "OWNER_GROUP=$UID:$GROUPS" --rm -v "$(absolute_path $OS_ROOT):/go/${origin_path}:z" openshift/origin-release /usr/bin/openshift-origin-build.sh $@
 
 ret=$?; ENDTIME=$(date +%s); echo "$0 took $(($ENDTIME - $STARTTIME)) seconds"; exit "$ret"
diff --git a/pkg/cmd/server/kubernetes/node/node.go b/pkg/cmd/server/kubernetes/node/node.go
@@ -6,7 +6,6 @@ import (
 	"net"
 	"net/url"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"time"
 
@@ -54,17 +53,6 @@ type commandExecutor interface {
 	Run(command string, args ...string) error
 }
 
-type defaultCommandExecutor struct{}
-
-func (ce defaultCommandExecutor) LookPath(executable string) (string, error) {
-	return exec.LookPath(executable)
-}
-
-func (ce defaultCommandExecutor) Run(command string, args ...string) error {
-	c := exec.Command(command, args...)
-	return c.Run()
-}
-
 const minimumDockerAPIVersionWithPullByID = "1.18"
 
 // EnsureKubeletAccess performs a number of test operations that the Kubelet requires to properly function.
@@ -181,14 +169,14 @@ func (c *NodeConfig) HandleDockerError(message string) {
 // an absolute path and create the directory if it does not exist. Will exit if
 // an error is encountered.
 func (c *NodeConfig) EnsureVolumeDir() {
-	if volumeDir, err := c.initializeVolumeDir(&defaultCommandExecutor{}, c.VolumeDir); err != nil {
+	if volumeDir, err := c.initializeVolumeDir(c.VolumeDir); err != nil {
 		glog.Fatal(err)
 	} else {
 		c.VolumeDir = volumeDir
 	}
 }
 
-func (c *NodeConfig) initializeVolumeDir(ce commandExecutor, path string) (string, error) {
+func (c *NodeConfig) initializeVolumeDir(path string) (string, error) {
 	rootDirectory, err := filepath.Abs(path)
 	if err != nil {
 		return "", fmt.Errorf("Error converting volume directory to an absolute path: %v", err)
@@ -199,14 +187,6 @@ func (c *NodeConfig) initializeVolumeDir(ce commandExecutor, path string) (strin
 			return "", fmt.Errorf("Couldn't create kubelet volume root directory '%s': %s", rootDirectory, err)
 		}
 	}
-	// always try to chcon, in case the volume dir existed prior to the node starting
-	if chconPath, err := ce.LookPath("chcon"); err != nil {
-		glog.V(2).Infof("Couldn't locate 'chcon' to set the kubelet volume root directory SELinux context: %s", err)
-	} else {
-		if err := ce.Run(chconPath, "-t", "svirt_sandbox_file_t", rootDirectory); err != nil {
-			glog.Warningf("Error running 'chcon' to set the kubelet volume root directory SELinux context: %s", err)
-		}
-	}
 	return rootDirectory, nil
 }
 

diff --git a/pkg/cmd/server/kubernetes/node/node_test.go b/pkg/cmd/server/kubernetes/node/node_test.go
@@ -1,34 +1,13 @@
 package node
 
 import (
-	"errors"
 	"io/ioutil"
 	"os"
 	"path"
 )
 
 import "testing"
 
-type fakeCommandExecutor struct {
-	commandFound bool
-	commandErr   error
-	runCalled    bool
-	lookCalled   bool
-}
-
-func (f *fakeCommandExecutor) LookPath(path string) (string, error) {
-	f.lookCalled = true
-	if f.commandFound {
-		return path, nil
-	}
-	return "", errors.New("not found")
-}
-
-func (f *fakeCommandExecutor) Run(command string, args ...string) error {
-	f.runCalled = true
-	return f.commandErr
-}
-
 func TestInitializeVolumeDir(t *testing.T) {
 	parentDir, err := ioutil.TempDir("", "")
 	if err != nil {
@@ -42,22 +21,13 @@ func TestInitializeVolumeDir(t *testing.T) {
 	volumeDir := path.Join(parentDir, "somedir")
 
 	testCases := map[string]struct {
-		chconFound       bool
-		chconRunErr      error
 		dirAlreadyExists bool
 	}{
-		"no chcon":                  {chconFound: false},
-		"have chcon":                {chconFound: true},
-		"chcon error":               {chconFound: true, chconRunErr: errors.New("e")},
-		"volume dir already exists": {chconFound: true, dirAlreadyExists: true},
+		"volume dir does not exist": {dirAlreadyExists: false},
+		"volume dir already exists": {dirAlreadyExists: true},
 	}
 
 	for name, testCase := range testCases {
-		ce := &fakeCommandExecutor{
-			commandFound: testCase.chconFound,
-			commandErr:   testCase.chconRunErr,
-		}
-
 		if testCase.dirAlreadyExists {
 			if err := os.MkdirAll(volumeDir, 0750); err != nil {
 				t.Fatalf("%s: error creating volume dir: %v", name, err)
@@ -69,17 +39,8 @@ func TestInitializeVolumeDir(t *testing.T) {
 		}
 
 		nc := &NodeConfig{VolumeDir: volumeDir}
-		path, err := nc.initializeVolumeDir(ce, nc.VolumeDir)
+		path, err := nc.initializeVolumeDir(nc.VolumeDir)
 
-		if !ce.lookCalled {
-			t.Errorf("%s: expected look for chcon", name)
-		}
-		if !testCase.chconFound && ce.runCalled {
-			t.Errorf("%s: unexpected run after chcon not found", name)
-		}
-		if testCase.chconFound && !ce.runCalled {
-			t.Errorf("%s: expected chcon run", name)
-		}
 		if err != nil {
 			t.Errorf("%s: unexpected err: %s", name, err)
 		}

diff --git a/pkg/diagnostics/cluster/registry.go b/pkg/diagnostics/cluster/registry.go
@@ -108,10 +108,6 @@ ownership/permissions.
 For volume permission problems please consult the Persistent Storage section
 of the Administrator's Guide.
 
-In the case of SELinux this may be resolved on the node by running:
-
-    sudo chcon -R -t svirt_sandbox_file_t [PATH_TO]/openshift.local.volumes
-
 %s`
 
 	clRegNoEP = `

diff --git a/test/extended/setup.sh b/test/extended/setup.sh
@@ -82,13 +82,10 @@ function os::test::extended::setup () {
 		SKIP_NODE=1
 	fi
 
-	# when selinux is enforcing, the volume dir selinux label needs to be
-	# svirt_sandbox_file_t
-	#
-	# TODO: fix the selinux policy to either allow openshift_var_lib_dir_t
-	# or to default the volume dir to svirt_sandbox_file_t.
+	# make sure the volume dir has the same label as we would apply to the default VOLUME_DIR
 	if selinuxenabled; then
-		${sudo} chcon -t svirt_sandbox_file_t ${VOLUME_DIR}
+		local label=$(matchpathcon -m dir /var/lib/openshift/openshift.local.volumes)
+		${sudo} chcon "${label}" ${VOLUME_DIR}
 	fi
 	CONFIG_VERSION=""
 	if [[ -n "${API_SERVER_VERSION:-}" ]]; then
@@ -240,4 +237,4 @@ function os::test::extended::merge_junit () {
 	rm "${TEST_REPORT_DIR}"/*.xml
 	mv "${output}" "${TEST_REPORT_DIR}/junit.xml"
 }
-readonly -f os::test::extended::merge_junit
+readonly -f os::test::extended::merge_junit
diff --git a/test/extended/util/framework.go b/test/extended/util/framework.go
@@ -1192,7 +1192,7 @@ func SetupHostPathVolumes(c kcoreclient.PersistentVolumeInterface, prefix, capac
 			return volumes, err
 		}
 		if _, err = exec.LookPath("chcon"); err == nil {
-			err := exec.Command("chcon", "-t", "svirt_sandbox_file_t", dir).Run()
+			err := exec.Command("chcon", "-t", "container_file_t", dir).Run()
 			if err != nil {
 				return volumes, err
 			}