From 3aa241a74c0fe89e01aa937efc9b8eccff5f222c Mon Sep 17 00:00:00 2001 From: Timo Reimann Date: Fri, 27 Jul 2018 10:53:05 +0200 Subject: [PATCH] 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 --- pkg/restore/restore_test.go | 13 +++- pkg/restore/service_action.go | 39 ++++++++++ pkg/restore/service_action_test.go | 114 +++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 2 deletions(-) diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index a374888d7..51435a7e6 100644 --- 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 index 3f48b91de..fdeb67770 100644 --- 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,6 +59,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 @@ -59,8 +71,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 +} diff --git a/pkg/restore/service_action_test.go b/pkg/restore/service_action_test.go index 3e6ed173b..7b9198756 100644 --- 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 {