Skip to content

Commit

Permalink
refactor: patcher for service sync
Browse files Browse the repository at this point in the history
  • Loading branch information
@FabianKramm
FabianKramm committed Jul 19, 2024
1 parent 175c2ff commit d982b9c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 108 deletions.
83 changes: 52 additions & 31 deletions pkg/controllers/resources/services/syncer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/loft-sh/vcluster/pkg/mappings"
"github.com/loft-sh/vcluster/pkg/patcher"
"github.com/loft-sh/vcluster/pkg/specialservices"
"github.com/loft-sh/vcluster/pkg/util/translate"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
Expand Down Expand Up @@ -65,17 +66,28 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, v
return ctrl.Result{RequeueAfter: time.Second * 3}, nil
}

shouldPatch := false
// check if recreating service is necessary
if vService.Spec.ClusterIP != pService.Spec.ClusterIP {
vService.Spec.ClusterIPs = nil
vService.Spec.ClusterIP = pService.Spec.ClusterIP
ctx.Log.Infof("recreating virtual service %s/%s, because cluster ip differs %s != %s", vService.Namespace, vService.Name, pService.Spec.ClusterIP, vService.Spec.ClusterIP)

// recreate the new service with the correct cluster ip
err := recreateService(ctx, ctx.VirtualClient, vService)
if err != nil {
ctx.Log.Errorf("error creating virtual service: %s/%s", vService.Namespace, vService.Name)
return ctrl.Result{}, err
}

return ctrl.Result{Requeue: true}, err
}

// patch the service
patch, err := patcher.NewSyncerPatcher(ctx, pService, vService)
if err != nil {
return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err)
}

defer func() {
if !shouldPatch {
return
}

if err := patch.Patch(ctx, pService, vService); err != nil {
retErr = utilerrors.NewAggregate([]error{retErr, err})
}
Expand All @@ -84,29 +96,38 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, v
}
}()

// check if backwards update is necessary
newService := s.translateUpdateBackwards(pService, vService)
if newService != nil {
if vService.Spec.ClusterIP != pService.Spec.ClusterIP {
newService.Spec.ClusterIPs = nil
ctx.Log.Infof("recreating virtual service %s/%s, because cluster ip differs %s != %s", vService.Namespace, vService.Name, pService.Spec.ClusterIP, vService.Spec.ClusterIP)

// recreate the new service with the correct cluster ip
vService, err = recreateService(ctx, ctx.VirtualClient, newService)
if err != nil {
ctx.Log.Errorf("error creating virtual service: %s/%s", vService.Namespace, vService.Name)
}
} else {
newService.DeepCopyInto(vService)
shouldPatch = true
}
return ctrl.Result{Requeue: true}, err
}
shouldPatch = true
// check
sourceService, targetService := synccontext.SyncSourceTarget(ctx, pService, vService)

// update spec bidirectionally
targetService.Spec.ExternalIPs = sourceService.Spec.ExternalIPs
targetService.Spec.LoadBalancerIP = sourceService.Spec.LoadBalancerIP
targetService.Spec.Ports = sourceService.Spec.Ports
targetService.Spec.PublishNotReadyAddresses = sourceService.Spec.PublishNotReadyAddresses
targetService.Spec.Type = sourceService.Spec.Type
targetService.Spec.ExternalName = sourceService.Spec.ExternalName
targetService.Spec.ExternalTrafficPolicy = sourceService.Spec.ExternalTrafficPolicy
targetService.Spec.SessionAffinity = sourceService.Spec.SessionAffinity
targetService.Spec.SessionAffinityConfig = sourceService.Spec.SessionAffinityConfig
targetService.Spec.LoadBalancerSourceRanges = sourceService.Spec.LoadBalancerSourceRanges
targetService.Spec.HealthCheckNodePort = sourceService.Spec.HealthCheckNodePort

// update status
vService.Status = pService.Status
// forward update
s.translateUpdate(ctx, pService, vService)

// check annotations
_, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vObj, pObj)

// remove the ServiceBlockDeletion annotation if it's not needed
if vService.Spec.ClusterIP == pService.Spec.ClusterIP {
delete(updatedAnnotations, ServiceBlockDeletion)
}

pService.Annotations = updatedAnnotations
pService.Labels = updatedLabels

// translate selector
pService.Spec.Selector = translate.Default.TranslateLabels(vService.Spec.Selector, vService.Namespace, nil)
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -135,11 +156,11 @@ func (s *serviceSyncer) SyncToVirtual(ctx *synccontext.SyncContext, pObj client.
return syncer.DeleteHostObject(ctx, pObj, "virtual object was deleted")
}

func recreateService(ctx context.Context, virtualClient client.Client, vService *corev1.Service) (*corev1.Service, error) {
func recreateService(ctx context.Context, virtualClient client.Client, vService *corev1.Service) error {
// delete & create with correct ClusterIP
err := virtualClient.Delete(ctx, vService)
if err != nil && !kerrors.IsNotFound(err) {
return nil, err
return err
}

// make sure we don't set the resource version during create
Expand All @@ -153,10 +174,10 @@ func recreateService(ctx context.Context, virtualClient client.Client, vService
err = virtualClient.Create(ctx, vService)
if err != nil {
klog.Errorf("error recreating virtual service: %s/%s: %v", vService.Namespace, vService.Name, err)
return nil, err
return err
}

return vService, nil
return nil
}

var _ syncertypes.Starter = &serviceSyncer{}
Expand Down
5 changes: 5 additions & 0 deletions pkg/controllers/resources/services/syncer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func TestSync(t *testing.T) {
Namespace: vObjectMeta.Namespace,
},
Spec: corev1.ServiceSpec{
ExternalName: "backwardExternal",
ExternalIPs: []string{"123:221:123:221"},
LoadBalancerIP: "123:213:123:213",
},
Expand Down Expand Up @@ -244,6 +245,7 @@ func TestSync(t *testing.T) {
{
Name: "test",
Port: 123,
NodePort: 567,
TargetPort: intstr.FromInt(10),
},
},
Expand Down Expand Up @@ -332,6 +334,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
syncCtx.EventSource = synccontext.EventSourceHost
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServicePorts1.DeepCopy(), vServicePorts1.DeepCopy())
assert.NilError(t, err)
},
Expand Down Expand Up @@ -398,6 +401,7 @@ func TestSync(t *testing.T) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
baseService := baseService.DeepCopy()
updateBackwardSpecService := updateBackwardSpecService.DeepCopy()
syncCtx.EventSource = synccontext.EventSourceHost
_, err := syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardSpecService, baseService)
assert.NilError(t, err)

Expand Down Expand Up @@ -435,6 +439,7 @@ func TestSync(t *testing.T) {
err = ctx.PhysicalManager.GetClient().Get(ctx, types.NamespacedName{Namespace: updateBackwardSpecRecreateService.Namespace, Name: updateBackwardSpecRecreateService.Name}, updateBackwardSpecRecreateService)
assert.NilError(t, err)

syncCtx.EventSource = synccontext.EventSourceHost
baseService.Spec.ExternalName = updateBackwardSpecService.Spec.ExternalName
_, err = syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardSpecRecreateService.DeepCopy(), baseService.DeepCopy())
assert.NilError(t, err)
Expand Down
77 changes: 0 additions & 77 deletions pkg/controllers/resources/services/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ package services

import (
"context"
"slices"

"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
"github.com/loft-sh/vcluster/pkg/util/translate"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
)

func (s *serviceSyncer) translate(ctx context.Context, vObj *corev1.Service) *corev1.Service {
Expand Down Expand Up @@ -37,77 +34,3 @@ func StripNodePorts(vObj *corev1.Service) {
vObj.Spec.Ports[i].NodePort = 0
}
}

func portsEqual(pObj, vObj *corev1.Service) bool {
pSpec := pObj.Spec.DeepCopy()
vSpec := vObj.Spec.DeepCopy()
for i := range pSpec.Ports {
pSpec.Ports[i].NodePort = 0
}
for i := range vSpec.Ports {
vSpec.Ports[i].NodePort = 0
}
return equality.Semantic.DeepEqual(pSpec.Ports, vSpec.Ports)
}

func (s *serviceSyncer) translateUpdateBackwards(pObj, vObj *corev1.Service) *corev1.Service {
var updated *corev1.Service

if vObj.Spec.ClusterIP != pObj.Spec.ClusterIP {
updated = translator.NewIfNil(updated, vObj)
updated.Spec.ClusterIP = pObj.Spec.ClusterIP
}

if !equality.Semantic.DeepEqual(vObj.Spec.ExternalIPs, pObj.Spec.ExternalIPs) {
updated = translator.NewIfNil(updated, vObj)
updated.Spec.ExternalIPs = pObj.Spec.ExternalIPs
}

if vObj.Spec.LoadBalancerIP != pObj.Spec.LoadBalancerIP {
updated = translator.NewIfNil(updated, vObj)
updated.Spec.LoadBalancerIP = pObj.Spec.LoadBalancerIP
}

// check if we need to sync node ports from host to virtual
if pObj.Spec.Type == vObj.Spec.Type && portsEqual(pObj, vObj) && !equality.Semantic.DeepEqual(vObj.Spec.Ports, pObj.Spec.Ports) {
updated = translator.NewIfNil(updated, vObj)
updated.Spec.Ports = pObj.Spec.Ports
}

return updated
}

func (s *serviceSyncer) translateUpdate(ctx context.Context, pObj, vObj *corev1.Service) {
// check annotations
_, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vObj, pObj)
// remove the ServiceBlockDeletion annotation if it's not needed
if vObj.Spec.ClusterIP == pObj.Spec.ClusterIP {
delete(updatedAnnotations, ServiceBlockDeletion)
}
pObj.Annotations = updatedAnnotations
pObj.Labels = updatedLabels

pObj.Spec.Ports = slices.Clone(vObj.Spec.Ports)

// make sure node ports will be reset here
StripNodePorts(pObj)

pObj.Spec.PublishNotReadyAddresses = vObj.Spec.PublishNotReadyAddresses

pObj.Spec.Type = vObj.Spec.Type

pObj.Spec.ExternalName = vObj.Spec.ExternalName

pObj.Spec.ExternalTrafficPolicy = vObj.Spec.ExternalTrafficPolicy

pObj.Spec.SessionAffinity = vObj.Spec.SessionAffinity

pObj.Spec.SessionAffinityConfig = vObj.Spec.SessionAffinityConfig

pObj.Spec.LoadBalancerSourceRanges = vObj.Spec.LoadBalancerSourceRanges

pObj.Spec.HealthCheckNodePort = vObj.Spec.HealthCheckNodePort

// translate selector
pObj.Spec.Selector = translate.Default.TranslateLabels(vObj.Spec.Selector, vObj.Namespace, nil)
}
9 changes: 9 additions & 0 deletions pkg/controllers/syncer/context/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ type SyncContext struct {
IsDelete bool
}

func SyncSourceTarget[T any](ctx *SyncContext, pObj, vObj T) (source T, target T) {
if ctx.EventFromHost() {
// sourceObj (Host), targetObj
return pObj, vObj
}
// sourceObj (Virtual), targetObj
return vObj, pObj
}

// Cast returns the given objects as types as well as
func Cast[T any](ctx *SyncContext, pObj, vObj client.Object) (physical T, virtual T, source T, target T) {
if pObj == nil || vObj == nil {
Expand Down

0 comments on commit d982b9c

Please sign in to comment.