Merge pull request #26781 from aveshagarwal/master-dapi-volume-annotations-labels-issue

Automatic merge from submit-queue

Remove an empty line being output when exposing annotations and labels via downward api volume

The issue is that formatMap function (for annotations and labels) in pkg/fieldpath/fieldpath.go appends a "\n" after each key value pair which is correct for all pairs except the last pair because then a complete string is returned with a "\n" in the end. It is inconsistent with other strings (metadata.name, namespace and resources) being returned as they dont have "\n" in the end. These returned strings are processed by sortLines function in pkg/volume/downwardapi/downwardapi.go and the function finally appends "\n" to each  string, but incorrectly outputs an empty line if there is an already "\n" in the end with the  input string. To illustrate:

The sortLines works as follows: lets say the input string is : "a\nb\nc\n". 

1. It splits them as "a", "b", "c", ""  (note empty string in the end). 
2. it sort them:  "", "a", b", "c"  
3. And then it appends "\n" again to each string:  "\n",  "a\n" ,"b\n", "c\n"

So we can see that it is erroneously creating an empty string in the beginning when the input string to sortLines has "\n" in the end.  As I said above, it is not an issue with metadata.name, namespace and resources as their input strings are without \n" in the end.

So now, the output in the downward api volume, (using the example in http://kubernetes.io/docs/user-guide/downward-api/):

```
# cat /etc/annotations

 zone="us-est-coast"
 cluster="test-cluster1"
 rack="rack-22"
```

After this patch, the output will be correct and without the erroneous empty line in the beginning.
I could think other ways to solve this but I found the way in this patch with minimal code changes.

@kubernetes/rh-cluster-infra
This commit is contained in:
k8s-merge-robot 2016-06-18 09:19:21 -07:00 committed by GitHub
commit 7e88b0ef0e
3 changed files with 25 additions and 30 deletions

View File

@ -20,6 +20,7 @@ import (
"fmt"
"math"
"strconv"
"strings"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/meta"
@ -27,10 +28,11 @@ import (
)
// formatMap formats map[string]string to a string.
func formatMap(m map[string]string) (fmtStr string) {
func FormatMap(m map[string]string) (fmtStr string) {
for key, value := range m {
fmtStr += fmt.Sprintf("%v=%q\n", key, value)
}
fmtStr = strings.TrimSuffix(fmtStr, "\n")
return
}
@ -51,9 +53,9 @@ func ExtractFieldPathAsString(obj interface{}, fieldPath string) (string, error)
switch fieldPath {
case "metadata.annotations":
return formatMap(accessor.GetAnnotations()), nil
return FormatMap(accessor.GetAnnotations()), nil
case "metadata.labels":
return formatMap(accessor.GetLabels()), nil
return FormatMap(accessor.GetLabels()), nil
case "metadata.name":
return accessor.GetName(), nil
case "metadata.namespace":

View File

@ -65,7 +65,7 @@ func TestExtractFieldPathAsString(t *testing.T) {
Labels: map[string]string{"key": "value"},
},
},
expectedValue: "key=\"value\"\n",
expectedValue: "key=\"value\"",
},
{
name: "ok - labels bslash n",
@ -75,7 +75,7 @@ func TestExtractFieldPathAsString(t *testing.T) {
Labels: map[string]string{"key": "value\n"},
},
},
expectedValue: "key=\"value\\n\"\n",
expectedValue: "key=\"value\\n\"",
},
{
name: "ok - annotations",
@ -85,7 +85,7 @@ func TestExtractFieldPathAsString(t *testing.T) {
Annotations: map[string]string{"builder": "john-doe"},
},
},
expectedValue: "builder=\"john-doe\"\n",
expectedValue: "builder=\"john-doe\"",
},
{

View File

@ -26,6 +26,7 @@ import (
"k8s.io/kubernetes/pkg/api"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/fieldpath"
"k8s.io/kubernetes/pkg/types"
utiltesting "k8s.io/kubernetes/pkg/util/testing"
"k8s.io/kubernetes/pkg/volume"
@ -35,14 +36,6 @@ import (
const downwardAPIDir = "..data"
func formatMap(m map[string]string) (fmtstr string) {
for key, value := range m {
fmtstr += fmt.Sprintf("%v=%q\n", key, value)
}
return
}
func newTestHost(t *testing.T, clientset clientset.Interface) (string, volume.VolumeHost) {
tempDir, err := utiltesting.MkTmpdir("downwardApi_volume_test.")
if err != nil {
@ -155,8 +148,8 @@ func TestLabels(t *testing.T) {
if err != nil {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, formatMap(labels))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(labels))
}
CleanEverything(plugin, testVolumeName, volumePath, testPodUID, t)
@ -222,8 +215,8 @@ func TestAnnotations(t *testing.T) {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(annotations)) {
t.Errorf("Found `%s` expected %s", data, formatMap(annotations))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(annotations)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(annotations))
}
CleanEverything(plugin, testVolumeName, volumePath, testPodUID, t)
@ -433,8 +426,8 @@ func TestWriteTwiceNoUpdate(t *testing.T) {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, formatMap(labels))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(labels))
}
CleanEverything(plugin, testVolumeName, volumePath, testPodUID, t)
@ -503,8 +496,8 @@ func TestWriteTwiceWithUpdate(t *testing.T) {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, formatMap(labels))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(labels))
}
newLabels := map[string]string{
@ -534,8 +527,8 @@ func TestWriteTwiceWithUpdate(t *testing.T) {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(newLabels)) {
t.Errorf("Found `%s` expected %s", data, formatMap(newLabels))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(newLabels)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(newLabels))
}
CleanEverything(plugin, testVolumeName, volumePath, testPodUID, t)
}
@ -606,16 +599,16 @@ func TestWriteWithUnixPath(t *testing.T) {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, formatMap(labels))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(labels))
}
data, err = ioutil.ReadFile(path.Join(volumePath, "this/is/yours/annotations"))
if err != nil {
t.Errorf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(annotations)) {
t.Errorf("Found `%s` expected %s", data, formatMap(annotations))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(annotations)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(annotations))
}
CleanEverything(plugin, testVolumeName, volumePath, testPodUID, t)
}
@ -687,7 +680,7 @@ func TestWriteWithUnixPathBadPath(t *testing.T) {
t.Fatalf(err.Error())
}
if sortLines(string(data)) != sortLines(formatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, formatMap(labels))
if sortLines(string(data)) != sortLines(fieldpath.FormatMap(labels)) {
t.Errorf("Found `%s` expected %s", data, fieldpath.FormatMap(labels))
}
}