Fix /etc/hostname backward compatibility issue for in-place upgrade.

Random-Liu · Random-Liu · commit 134c2f35daa7 · 2019-03-12T13:06:49.000-07:00
Signed-off-by: Lantao Liu &lt;lantaol@google.com&gt;
diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go
@@ -466,11 +466,19 @@ func (c *criService) generateContainerMounts(sandboxID string, config *runtime.C
 	var mounts []*runtime.Mount
 	securityContext := config.GetLinux().GetSecurityContext()
 	if !isInCRIMounts(etcHostname, config.GetMounts()) {
-		mounts = append(mounts, &runtime.Mount{
-			ContainerPath: etcHostname,
-			HostPath:      c.getSandboxHostname(sandboxID),
-			Readonly:      securityContext.GetReadonlyRootfs(),
-		})
+		// /etc/hostname is added since 1.1.6, 1.2.4 and 1.3.
+		// For in-place upgrade, the old sandbox doesn't have the hostname file,
+		// do not mount this in that case.
+		// TODO(random-liu): Remove the check and always mount this when
+		// containerd 1.1 and 1.2 are deprecated.
+		hostpath := c.getSandboxHostname(sandboxID)
+		if _, err := c.os.Stat(hostpath); err == nil {
+			mounts = append(mounts, &runtime.Mount{
+				ContainerPath: etcHostname,
+				HostPath:      hostpath,
+				Readonly:      securityContext.GetReadonlyRootfs(),
+			})
+		}
 	}
 
 	if !isInCRIMounts(etcHosts, config.GetMounts()) {
diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package server
 
 import (
+	"os"
 	"path/filepath"
 	"reflect"
 	"strings"
@@ -29,6 +30,7 @@ import (
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
@@ -541,6 +543,7 @@ func TestGenerateVolumeMounts(t *testing.T) {
 func TestGenerateContainerMounts(t *testing.T) {
 	const testSandboxID = "test-id"
 	for desc, test := range map[string]struct {
+		statFn          func(string) (os.FileInfo, error)
 		criMounts       []*runtime.Mount
 		securityContext *runtime.LinuxContainerSecurityContext
 		expectedMounts  []*runtime.Mount
@@ -646,6 +649,30 @@ func TestGenerateContainerMounts(t *testing.T) {
 			securityContext: &runtime.LinuxContainerSecurityContext{},
 			expectedMounts:  nil,
 		},
+		"should skip hostname mount if the old sandbox doesn't have hostname file": {
+			statFn: func(path string) (os.FileInfo, error) {
+				assert.Equal(t, filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"), path)
+				return nil, errors.New("random error")
+			},
+			securityContext: &runtime.LinuxContainerSecurityContext{},
+			expectedMounts: []*runtime.Mount{
+				{
+					ContainerPath: "/etc/hosts",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
+					Readonly:      false,
+				},
+				{
+					ContainerPath: resolvConfPath,
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
+					Readonly:      false,
+				},
+				{
+					ContainerPath: "/dev/shm",
+					HostPath:      filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
+					Readonly:      false,
+				},
+			},
+		},
 	} {
 		config := &runtime.ContainerConfig{
 			Metadata: &runtime.ContainerMetadata{
@@ -658,6 +685,7 @@ func TestGenerateContainerMounts(t *testing.T) {
 			},
 		}
 		c := newTestCRIService()
+		c.os.(*ostesting.FakeOS).StatFn = test.statFn
 		mounts := c.generateContainerMounts(testSandboxID, config)
 		assert.Equal(t, test.expectedMounts, mounts, desc)
 	}
