Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added patch logic for services #1960

Merged
merged 2 commits into from
Jul 19, 2024
Merged

added patch logic for services #1960

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
175c2ff
added patch logic for services
facchettos Jul 19, 2024
d982b9c
refactor: patcher for service sync
FabianKramm Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 31 additions & 33 deletions pkg/controllers/resources/services/syncer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@ package services
import (
"context"
"errors"
"fmt"
"time"

"github.com/loft-sh/vcluster/pkg/controllers/syncer"
synccontext "github.com/loft-sh/vcluster/pkg/controllers/syncer/context"
"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
syncertypes "github.com/loft-sh/vcluster/pkg/controllers/syncer/types"
"github.com/loft-sh/vcluster/pkg/mappings"
"github.com/loft-sh/vcluster/pkg/patcher"
"github.com/loft-sh/vcluster/pkg/specialservices"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
kerrors "k8s.io/apimachinery/pkg/api/errors"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -54,7 +56,7 @@ func (s *serviceSyncer) SyncToHost(ctx *synccontext.SyncContext, vObj client.Obj
return s.SyncToHostCreate(ctx, vObj, s.translate(ctx, vObj.(*corev1.Service)))
}

func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) {
func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) {
vService := vObj.(*corev1.Service)
pService := pObj.(*corev1.Service)

Expand All @@ -63,6 +65,25 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, v
return ctrl.Result{RequeueAfter: time.Second * 3}, nil
}

shouldPatch := false
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})
}
if retErr != nil {
s.EventRecorder().Eventf(vObj, "Warning", "SyncError", "Error syncing: %v", retErr)
}
}()

// check if backwards update is necessary
newService := s.translateUpdateBackwards(pService, vService)
if newService != nil {
Expand All @@ -71,45 +92,22 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, v
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, newService)
vService, err = recreateService(ctx, ctx.VirtualClient, newService)
if err != nil {
ctx.Log.Errorf("error creating virtual service: %s/%s", vService.Namespace, vService.Name)
return ctrl.Result{}, err
}
} else {
// update with correct ports
ctx.Log.Infof("update virtual service %s/%s, because spec is out of sync", vService.Namespace, vService.Name)
err := ctx.VirtualClient.Update(ctx, newService)
if err != nil {
return ctrl.Result{}, err
}
newService.DeepCopyInto(vService)
shouldPatch = true
}

// we will requeue anyways
return ctrl.Result{Requeue: true}, nil
}

// check if backwards status update is necessary
if !equality.Semantic.DeepEqual(vService.Status, pService.Status) {
newService := vService.DeepCopy()
newService.Status = pService.Status
ctx.Log.Infof("update virtual service %s/%s, because status is out of sync", vService.Namespace, vService.Name)
translator.PrintChanges(vService, newService, ctx.Log)
err := ctx.VirtualClient.Status().Update(ctx, newService)
if err != nil {
return ctrl.Result{}, err
}

return ctrl.Result{Requeue: true}, nil
return ctrl.Result{Requeue: true}, err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go after line 98

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are failing if I move it there

}

shouldPatch = true
vService.Status = pService.Status
// forward update
newService = s.translateUpdate(ctx, pService, vService)
if newService != nil {
translator.PrintChanges(pService, newService, ctx.Log)
}
s.translateUpdate(ctx, pService, vService)

return s.SyncToHostUpdate(ctx, vObj, newService)
return ctrl.Result{}, nil
}

func isSwitchingFromExternalName(pService *corev1.Service, vService *corev1.Service) bool {
Expand Down
20 changes: 10 additions & 10 deletions pkg/controllers/resources/services/syncer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServicePorts1, vServicePorts1)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServicePorts1.DeepCopy(), vServicePorts1.DeepCopy())
assert.NilError(t, err)
},
},
Expand All @@ -348,7 +348,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServicePorts2, vServicePorts1)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServicePorts2.DeepCopy(), vServicePorts1.DeepCopy())
assert.NilError(t, err)
},
},
Expand All @@ -364,7 +364,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, createdByServerService, updateForwardService)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, createdByServerService.DeepCopy(), updateForwardService.DeepCopy())
assert.NilError(t, err)
},
},
Expand All @@ -380,7 +380,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, createdService, baseService)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, createdService.DeepCopy(), baseService.DeepCopy())
assert.NilError(t, err)
},
},
Expand Down Expand Up @@ -408,7 +408,7 @@ func TestSync(t *testing.T) {
assert.NilError(t, err)

baseService.Spec.ExternalName = updateBackwardSpecService.Spec.ExternalName
_, err = syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardSpecService, baseService)
_, err = syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardSpecService.DeepCopy(), baseService.DeepCopy())
assert.NilError(t, err)
},
},
Expand Down Expand Up @@ -436,7 +436,7 @@ func TestSync(t *testing.T) {
assert.NilError(t, err)

baseService.Spec.ExternalName = updateBackwardSpecService.Spec.ExternalName
_, err = syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardSpecRecreateService, baseService)
_, err = syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardSpecRecreateService.DeepCopy(), baseService.DeepCopy())
assert.NilError(t, err)
},
},
Expand All @@ -452,7 +452,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardStatusService, baseService)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, updateBackwardStatusService.DeepCopy(), baseService.DeepCopy())
assert.NilError(t, err)
},
},
Expand All @@ -468,7 +468,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, createdService, baseService)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, createdService.DeepCopy(), baseService.DeepCopy())
assert.NilError(t, err)
},
},
Expand Down Expand Up @@ -556,7 +556,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServiceExternal, vServiceClusterIPFromExternal)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServiceExternal.DeepCopy(), vServiceClusterIPFromExternal.DeepCopy())
assert.NilError(t, err)
},
},
Expand All @@ -572,7 +572,7 @@ func TestSync(t *testing.T) {
},
Sync: func(ctx *synccontext.RegisterContext) {
syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServiceExternal, vServiceNodePortFromExternal)
_, err := syncer.(*serviceSyncer).Sync(syncCtx, pServiceExternal.DeepCopy(), vServiceNodePortFromExternal.DeepCopy())
assert.NilError(t, err)
},
},
Expand Down
79 changes: 16 additions & 63 deletions pkg/controllers/resources/services/translate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package services

import (
"context"
"slices"

"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
"github.com/loft-sh/vcluster/pkg/util/translate"
Expand Down Expand Up @@ -76,85 +77,37 @@ func (s *serviceSyncer) translateUpdateBackwards(pObj, vObj *corev1.Service) *co
return updated
}

func (s *serviceSyncer) translateUpdate(ctx context.Context, pObj, vObj *corev1.Service) *corev1.Service {
var updated *corev1.Service

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)
}
if !equality.Semantic.DeepEqual(updatedAnnotations, pObj.Annotations) || !equality.Semantic.DeepEqual(updatedLabels, pObj.Labels) {
updated = translator.NewIfNil(updated, pObj)
updated.Annotations = updatedAnnotations
updated.Labels = updatedLabels
}
pObj.Annotations = updatedAnnotations
pObj.Labels = updatedLabels

// check ports
if !equality.Semantic.DeepEqual(vObj.Spec.Ports, pObj.Spec.Ports) {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.Ports = vObj.Spec.Ports
pObj.Spec.Ports = slices.Clone(vObj.Spec.Ports)

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

// publish not ready addresses
if vObj.Spec.PublishNotReadyAddresses != pObj.Spec.PublishNotReadyAddresses {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.PublishNotReadyAddresses = vObj.Spec.PublishNotReadyAddresses
}
pObj.Spec.PublishNotReadyAddresses = vObj.Spec.PublishNotReadyAddresses

// type
if vObj.Spec.Type != pObj.Spec.Type {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.Type = vObj.Spec.Type
}
pObj.Spec.Type = vObj.Spec.Type

// external name
if vObj.Spec.ExternalName != pObj.Spec.ExternalName {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.ExternalName = vObj.Spec.ExternalName
}
pObj.Spec.ExternalName = vObj.Spec.ExternalName

// externalTrafficPolicy
if vObj.Spec.ExternalTrafficPolicy != pObj.Spec.ExternalTrafficPolicy {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.ExternalTrafficPolicy = vObj.Spec.ExternalTrafficPolicy
}
pObj.Spec.ExternalTrafficPolicy = vObj.Spec.ExternalTrafficPolicy

// session affinity
if vObj.Spec.SessionAffinity != pObj.Spec.SessionAffinity {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.SessionAffinity = vObj.Spec.SessionAffinity
}
pObj.Spec.SessionAffinity = vObj.Spec.SessionAffinity

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

// load balancer source ranges
if !equality.Semantic.DeepEqual(vObj.Spec.LoadBalancerSourceRanges, pObj.Spec.LoadBalancerSourceRanges) {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.LoadBalancerSourceRanges = vObj.Spec.LoadBalancerSourceRanges
}
pObj.Spec.LoadBalancerSourceRanges = vObj.Spec.LoadBalancerSourceRanges

// healthCheckNodePort
if vObj.Spec.HealthCheckNodePort != pObj.Spec.HealthCheckNodePort {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.HealthCheckNodePort = vObj.Spec.HealthCheckNodePort
}
pObj.Spec.HealthCheckNodePort = vObj.Spec.HealthCheckNodePort

// translate selector
translated := pObj.DeepCopy()
translated.Spec.Selector = translate.Default.TranslateLabels(vObj.Spec.Selector, vObj.Namespace, nil)
if !equality.Semantic.DeepEqual(translated.Spec.Selector, pObj.Spec.Selector) {
updated = translator.NewIfNil(updated, pObj)
updated.Spec.Selector = translated.Spec.Selector
}

return updated
pObj.Spec.Selector = translate.Default.TranslateLabels(vObj.Spec.Selector, vObj.Namespace, nil)
}
Loading