Preserve node ports during restore when annotations hold specification.

This is to better reflect the intent of the user when node ports are specified explicitly (as opposed to being assigned by Kubernetes). The `last-applied-configuration` annotation added by `kubectl apply` is one such indicator we are now leveraging. We still default to omitting the node ports when the annotation is missing. Signed-off-by: Timo Reimann <ttr314@googlemail.com>
vmware-tanzu · Aug 7, 2018 · 3aa241a · 3aa241a
1 parent 652069b
commit 3aa241a
Show file tree

Hide file tree

Showing 3 changed files with 164 additions and 2 deletions.
diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
@@ -1404,9 +1404,18 @@ func (obj *testUnstructured) WithStatusField(field string, value interface{}) *t
 }
 
 func (obj *testUnstructured) WithAnnotations(fields ...string) *testUnstructured {
-	annotations := make(map[string]interface{})
+	vals := map[string]string{}
 	for _, field := range fields {
-		annotations[field] = "foo"
+		vals[field] = "foo"
+	}
+
+	return obj.WithAnnotationValues(vals)
+}
+
+func (obj *testUnstructured) WithAnnotationValues(fieldVals map[string]string) *testUnstructured {
+	annotations := make(map[string]interface{})
+	for field, val := range fieldVals {
+		annotations[field] = val
 	}
 
 	obj = obj.WithMetadataField("annotations", annotations)

diff --git a/pkg/restore/service_action.go b/pkg/restore/service_action.go
@@ -17,14 +17,21 @@ limitations under the License.
 package restore
 
 import (
+	"encoding/json"
+
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
+	corev1api "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
 
 	api "github.com/heptio/ark/pkg/apis/ark/v1"
 	"github.com/heptio/ark/pkg/util/collections"
 )
 
+const annotationLastAppliedConfig = "kubectl.kubernetes.io/last-applied-configuration"
+
 type serviceAction struct {
 	log logrus.FieldLogger
 }
@@ -52,15 +59,47 @@ func (a *serviceAction) Execute(obj runtime.Unstructured, restore *api.Restore)
 		delete(spec, "clusterIP")
 	}
 
+	preservedPorts, err := getPreservedPorts(obj)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	ports, err := collections.GetSlice(obj.UnstructuredContent(), "spec.ports")
 	if err != nil {
 		return nil, nil, err
 	}
 
 	for _, port := range ports {
 		p := port.(map[string]interface{})
+		var name string
+		if nameVal, ok := p["name"]; ok {
+			name = nameVal.(string)
+		}
+		if preservedPorts[name] {
+			continue
+		}
 		delete(p, "nodePort")
 	}
 
 	return obj, nil, nil
 }
+
+func getPreservedPorts(obj runtime.Unstructured) (map[string]bool, error) {
+	preservedPorts := map[string]bool{}
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+	if lac, ok := metadata.GetAnnotations()[annotationLastAppliedConfig]; ok {
+		var svc corev1api.Service
+		if err := json.Unmarshal([]byte(lac), &svc); err != nil {
+			return nil, errors.WithStack(err)
+		}
+		for _, port := range svc.Spec.Ports {
+			if port.NodePort > 0 {
+				preservedPorts[port.Name] = true
+			}
+		}
+	}
+	return preservedPorts, nil
+}
diff --git a/pkg/restore/service_action_test.go b/pkg/restore/service_action_test.go
@@ -17,15 +17,33 @@ limitations under the License.
 package restore
 
 import (
+	"encoding/json"
 	"testing"
 
 	arktest "github.com/heptio/ark/pkg/util/test"
 	"github.com/stretchr/testify/assert"
 
+	corev1api "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+func svcJSON(ports ...corev1api.ServicePort) string {
+	svc := corev1api.Service{
+		Spec: corev1api.ServiceSpec{
+			Ports: ports,
+		},
+	}
+
+	data, err := json.Marshal(svc)
+	if err != nil {
+		panic(err)
+	}
+
+	return string(data)
+}
+
 func TestServiceActionExecute(t *testing.T) {
+
 	tests := []struct {
 		name        string
 		obj         runtime.Unstructured
@@ -37,6 +55,11 @@ func TestServiceActionExecute(t *testing.T) {
 			obj:         NewTestUnstructured().WithName("svc-1").Unstructured,
 			expectedErr: true,
 		},
+		{
+			name:        "no spec ports should error",
+			obj:         NewTestUnstructured().WithName("svc-1").WithSpec().Unstructured,
+			expectedErr: true,
+		},
 		{
 			name:        "clusterIP (only) should be deleted from spec",
 			obj:         NewTestUnstructured().WithName("svc-1").WithSpec("clusterIP", "foo").WithSpecField("ports", []interface{}{}).Unstructured,
@@ -63,6 +86,97 @@ func TestServiceActionExecute(t *testing.T) {
 					map[string]interface{}{"foo": "bar"},
 				}).Unstructured,
 		},
+		{
+			name: "unnamed nodePort should be deleted when missing in annotation",
+			obj: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{"nodePort": 8080},
+				}).Unstructured,
+			expectedErr: false,
+			expectedRes: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{},
+				}).Unstructured,
+		},
+		{
+			name: "unnamed nodePort should be preserved when specified in annotation",
+			obj: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{NodePort: 8080}),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{
+						"nodePort": 8080,
+					},
+				}).Unstructured,
+			expectedErr: false,
+			expectedRes: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{NodePort: 8080}),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{
+						"nodePort": 8080,
+					},
+				}).Unstructured,
+		},
+		{
+			name: "unnamed nodePort should be deleted when named nodePort specified in annotation",
+			obj: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{
+						"nodePort": 8080,
+					},
+				}).Unstructured,
+			expectedErr: false,
+			expectedRes: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{},
+				}).Unstructured,
+		},
+		{
+			name: "named nodePort should be preserved when specified in annotation",
+			obj: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{
+						"name":     "http",
+						"nodePort": 8080,
+					},
+					map[string]interface{}{
+						"name":     "admin",
+						"nodePort": 9090,
+					},
+				}).Unstructured,
+			expectedErr: false,
+			expectedRes: NewTestUnstructured().WithName("svc-1").
+				WithAnnotationValues(map[string]string{
+					annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}),
+				}).
+				WithSpecField("ports", []interface{}{
+					map[string]interface{}{
+						"name":     "http",
+						"nodePort": 8080,
+					},
+					map[string]interface{}{
+						"name": "admin",
+					},
+				}).Unstructured,
+		},
 	}
 
 	for _, test := range tests {