Merge pull request #712 from timoreimann/preserve-node-ports-when-specified-in-annotation

Preserve node ports during restore when annotations hold specification.

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)

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
 }
@@ -50,6 +57,11 @@ 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
@@ -57,8 +69,35 @@ func (a *serviceAction) Execute(obj runtime.Unstructured, restore *api.Restore)
 
 	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
+}

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 {