Merge pull request #47514 from jsafrane/move-savepodtofile

Automatic merge from submit-queue (batch tested with PRs 48074, 47971, 48044, 47514, 47647)

Move LoadPodFromFile to volume utils

SavePodToFile is not used anywhere and LoadPodFromFile is used only by PV
recycler.

Fix #16970

```release-note
NONE
```

cmd/kube-controller-manager/app/BUILD

@@ -71,7 +71,6 @@ go_library(
         "//pkg/quota/install:go_default_library",
         "//pkg/serviceaccount:go_default_library",
         "//pkg/util/configz:go_default_library",
-        "//pkg/util/io:go_default_library",
         "//pkg/version:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/aws_ebs:go_default_library",
@@ -91,6 +90,7 @@ go_library(
         "//pkg/volume/rbd:go_default_library",
         "//pkg/volume/scaleio:go_default_library",
         "//pkg/volume/storageos:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//pkg/volume/vsphere_volume:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",

cmd/kube-controller-manager/app/plugins.go

@@ -36,7 +36,6 @@ import (
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/openstack"
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/photon"
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere"
-	"k8s.io/kubernetes/pkg/util/io"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/aws_ebs"
 	"k8s.io/kubernetes/pkg/volume/azure_dd"
@@ -55,6 +54,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume/rbd"
 	"k8s.io/kubernetes/pkg/volume/scaleio"
 	"k8s.io/kubernetes/pkg/volume/storageos"
+	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/vsphere_volume"
 )
 
@@ -152,7 +152,7 @@ func ProbeControllerVolumePlugins(cloud cloudprovider.Interface, config componen
 // If unsuccessful, an error is returned. Function is exported for reuse downstream.
 func AttemptToLoadRecycler(path string, config *volume.VolumeConfig) error {
 	if path != "" {
-		recyclerPod, err := io.LoadPodFromFile(path)
+		recyclerPod, err := volumeutil.LoadPodFromFile(path)
 		if err != nil {
 			return err
 		}

pkg/util/io/BUILD

@@ -5,37 +5,13 @@ licenses(["notice"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
-    "go_test",
 )
 
 go_library(
     name = "go_default_library",
-    srcs = [
+    srcs = ["writer.go"],
-        "io.go",
-        "writer.go",
-    ],
     tags = ["automanaged"],
-    deps = [
+    deps = ["//vendor/github.com/golang/glog:go_default_library"],
-        "//pkg/api:go_default_library",
-        "//vendor/github.com/golang/glog:go_default_library",
-        "//vendor/k8s.io/api/core/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
-    ],
-)
-
-go_test(
-    name = "go_default_xtest",
-    srcs = ["io_test.go"],
-    tags = ["automanaged"],
-    deps = [
-        "//pkg/api:go_default_library",
-        "//pkg/util/io:go_default_library",
-        "//pkg/volume:go_default_library",
-        "//vendor/github.com/pborman/uuid:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
-        "//vendor/k8s.io/client-go/util/testing:go_default_library",
-    ],
 )
 
 filegroup(

pkg/util/io/io.go

@@ -1,61 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package io
-
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-
-	"k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/kubernetes/pkg/api"
-)
-
-// LoadPodFromFile will read, decode, and return a Pod from a file.
-func LoadPodFromFile(filePath string) (*v1.Pod, error) {
-	if filePath == "" {
-		return nil, fmt.Errorf("file path not specified")
-	}
-	podDef, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read file path %s: %+v", filePath, err)
-	}
-	if len(podDef) == 0 {
-		return nil, fmt.Errorf("file was empty: %s", filePath)
-	}
-	pod := &v1.Pod{}
-
-	codec := api.Codecs.LegacyCodec(api.Registry.GroupOrDie(v1.GroupName).GroupVersion)
-	if err := runtime.DecodeInto(codec, podDef, pod); err != nil {
-		return nil, fmt.Errorf("failed decoding file: %v", err)
-	}
-	return pod, nil
-}
-
-// SavePodToFile will encode and save a pod to a given path & permissions
-func SavePodToFile(pod *v1.Pod, filePath string, perm os.FileMode) error {
-	if filePath == "" {
-		return fmt.Errorf("file path not specified")
-	}
-	codec := api.Codecs.LegacyCodec(api.Registry.GroupOrDie(v1.GroupName).GroupVersion)
-	data, err := runtime.Encode(codec, pod)
-	if err != nil {
-		return fmt.Errorf("failed encoding pod: %v", err)
-	}
-	return ioutil.WriteFile(filePath, data, perm)
-}

pkg/util/io/io_test.go

@@ -1,56 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package io_test
-
-import (
-	"fmt"
-	"os"
-	"testing"
-
-	"github.com/pborman/uuid"
-	apiequality "k8s.io/apimachinery/pkg/api/equality"
-	"k8s.io/apimachinery/pkg/runtime"
-	utiltesting "k8s.io/client-go/util/testing"
-	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/util/io"
-	"k8s.io/kubernetes/pkg/volume"
-)
-
-func TestSavePodToFile(t *testing.T) {
-	pod := volume.NewPersistentVolumeRecyclerPodTemplate()
-
-	// sets all default values on a pod for equality comparison after decoding from file
-	codec := api.Codecs.LegacyCodec(api.Registry.GroupOrDie(api.GroupName).GroupVersion)
-	encoded, err := runtime.Encode(codec, pod)
-	runtime.DecodeInto(codec, encoded, pod)
-
-	tmpDir := utiltesting.MkTmpdirOrDie("kube-io-test")
-	defer os.RemoveAll(tmpDir)
-	path := fmt.Sprintf("/%s/kube-io-test-%s", tmpDir, uuid.New())
-
-	if err := io.SavePodToFile(pod, path, 777); err != nil {
-		t.Fatalf("failed to save pod to file: %v", err)
-	}
-
-	podFromFile, err := io.LoadPodFromFile(path)
-	if err != nil {
-		t.Fatalf("failed to load pod from file: %v", err)
-	}
-	if !apiequality.Semantic.DeepEqual(pod, podFromFile) {
-		t.Errorf("\nexpected %#v\ngot	%#v\n", pod, podFromFile)
-	}
-}

pkg/util/io/writer.go

@@ -28,6 +28,7 @@ import (
 
 // Writer is an interface which allows to write data to a file.
 type Writer interface {
+	// WriteFile mimics ioutil.WriteFile.
 	WriteFile(filename string, data []byte, perm os.FileMode) error
 }
 
@@ -36,31 +37,32 @@ type Writer interface {
 type StdWriter struct {
 }
 
+// WriteFile directly calls ioutil.WriteFile.
 func (writer *StdWriter) WriteFile(filename string, data []byte, perm os.FileMode) error {
 	return ioutil.WriteFile(filename, data, perm)
 }
 
-// Alternative implementation of Writer interface that allows writing data to file
+// NsenterWriter is implementation of Writer interface that allows writing data
-// using nsenter command.
+// to file using nsenter command.
 // If a program (e.g. kubelet) runs in a container it may want to write data to
 // a mounted device. Since in Docker, mount propagation mode is set to private,
 // it will not see the mounted device in its own namespace. To work around this
-// limitaion one has to first enter hosts namespace (by using 'nsenter') and only
+// limitation one has to first enter hosts namespace (by using 'nsenter') and
-// then write data.
+// only then write data.
-type NsenterWriter struct {
+type NsenterWriter struct{}
-}
 
-// TODO: should take a writer, not []byte
+// WriteFile calls 'nsenter cat - > <the file>' and 'nsenter chmod' to create a
+// file on the host.
 func (writer *NsenterWriter) WriteFile(filename string, data []byte, perm os.FileMode) error {
 	cmd := "nsenter"
-	base_args := []string{
+	baseArgs := []string{
 		"--mount=/rootfs/proc/1/ns/mnt",
 		"--",
 	}
 
-	echo_args := append(base_args, "sh", "-c", fmt.Sprintf("cat > %s", filename))
+	echoArgs := append(baseArgs, "sh", "-c", fmt.Sprintf("cat > %s", filename))
-	glog.V(5).Infof("Command to write data to file: %v %v", cmd, echo_args)
+	glog.V(5).Infof("Command to write data to file: %v %v", cmd, echoArgs)
-	command := exec.Command(cmd, echo_args...)
+	command := exec.Command(cmd, echoArgs...)
 	command.Stdin = bytes.NewBuffer(data)
 	outputBytes, err := command.CombinedOutput()
 	if err != nil {
@@ -68,9 +70,9 @@ func (writer *NsenterWriter) WriteFile(filename string, data []byte, perm os.Fil
 		return err
 	}
 
-	chmod_args := append(base_args, "chmod", fmt.Sprintf("%o", perm), filename)
+	chmodArgs := append(baseArgs, "chmod", fmt.Sprintf("%o", perm), filename)
-	glog.V(5).Infof("Command to change permissions to file: %v %v", cmd, chmod_args)
+	glog.V(5).Infof("Command to change permissions to file: %v %v", cmd, chmodArgs)
-	outputBytes, err = exec.Command(cmd, chmod_args...).CombinedOutput()
+	outputBytes, err = exec.Command(cmd, chmodArgs...).CombinedOutput()
 	if err != nil {
 		glog.Errorf("Output from chmod command: %v", string(outputBytes))
 		return err

pkg/volume/util/BUILD

@@ -21,6 +21,7 @@ go_library(
     ],
     tags = ["automanaged"],
     deps = [
+        "//pkg/api:go_default_library",
         "//pkg/api/v1/helper:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/util/mount:go_default_library",
@@ -30,6 +31,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
     ],
 )

pkg/volume/util/util.go

@@ -18,6 +18,7 @@ package util
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path"
 
@@ -26,6 +27,8 @@ import (
 	storage "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/kubernetes/pkg/api"
 	v1helper "k8s.io/kubernetes/pkg/api/v1/helper"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	"k8s.io/kubernetes/pkg/util/mount"
@@ -192,3 +195,24 @@ func CheckNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]string) er
 	}
 	return nil
 }
+
+// LoadPodFromFile will read, decode, and return a Pod from a file.
+func LoadPodFromFile(filePath string) (*v1.Pod, error) {
+	if filePath == "" {
+		return nil, fmt.Errorf("file path not specified")
+	}
+	podDef, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read file path %s: %+v", filePath, err)
+	}
+	if len(podDef) == 0 {
+		return nil, fmt.Errorf("file was empty: %s", filePath)
+	}
+	pod := &v1.Pod{}
+
+	codec := api.Codecs.LegacyCodec(api.Registry.GroupOrDie(v1.GroupName).GroupVersion)
+	if err := runtime.DecodeInto(codec, podDef, pod); err != nil {
+		return nil, fmt.Errorf("failed decoding file: %v", err)
+	}
+	return pod, nil
+}

pkg/volume/util/util_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package util
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"k8s.io/api/core/v1"
@@ -140,3 +142,87 @@ func testVolumeWithNodeAffinity(t *testing.T, affinity *v1.NodeAffinity) *v1.Per
 		ObjectMeta: objMeta,
 	}
 }
+
+func TestLoadPodFromFile(t *testing.T) {
+	tests := []struct {
+		name        string
+		content     string
+		expectError bool
+	}{
+		{
+			"yaml",
+			`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: testpod
+spec:
+  containers:
+    - image: gcr.io/google_containers/busybox
+`,
+			false,
+		},
+
+		{
+			"json",
+			`
+{
+  "apiVersion": "v1",
+  "kind": "Pod",
+  "metadata": {
+    "name": "testpod"
+  },
+  "spec": {
+    "containers": [
+      {
+        "image": "gcr.io/google_containers/busybox"
+      }
+    ]
+  }
+}`,
+			false,
+		},
+
+		{
+			"invalid pod",
+			`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: testpod
+spec:
+  - image: gcr.io/google_containers/busybox
+`,
+			true,
+		},
+	}
+
+	for _, test := range tests {
+		tempFile, err := ioutil.TempFile("", "podfile")
+		defer os.Remove(tempFile.Name())
+		if err != nil {
+			t.Fatalf("cannot create temporary file: %v", err)
+		}
+		if _, err = tempFile.Write([]byte(test.content)); err != nil {
+			t.Fatalf("cannot save temporary file: %v", err)
+		}
+		if err = tempFile.Close(); err != nil {
+			t.Fatalf("cannot close temporary file: %v", err)
+		}
+
+		pod, err := LoadPodFromFile(tempFile.Name())
+		if test.expectError {
+			if err == nil {
+				t.Errorf("test %q expected error, got nil", test.name)
+			}
+		} else {
+			// no error expected
+			if err != nil {
+				t.Errorf("error loading pod %q: %v", test.name, err)
+			}
+			if pod == nil {
+				t.Errorf("test %q expected pod, got nil", test.name)
+			}
+		}
+	}
+}