karmada-io
/
karmada
mirror of https://github.com/karmada-io/karmada.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4591 from jwcesign/fix-mcs-mci-conflict 

feat: add manager label to eps work to avoid the conflicts between mcs/serviceexport
This commit is contained in:
karmada-bot 2024-01-29 21:19:49 +08:00 committed by GitHub
parent ddfe903fb7 cebb1eac03
commit 0126b90fc8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 172 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
pkg/controllers/mcs/endpointslice_controller.go
View File

@ -71,6 +71,17 @@ func (c *EndpointSliceController) Reconcile(ctx context.Context, req controllerr
return controllerruntime.Result{}, nil
}
// TBD: The work is managed by service-export-controller and endpointslice-collect-controller now,
// after the ServiceExport is deleted, the corresponding works' labels will be cleaned, so we should delete the EndpointSlice in control plane.
// Once the conflict between service_export_controller.go and endpointslice_collect_controller.go is fixed, the following code should be deleted.
if serviceName := util.GetLabelValue(work.Labels, util.ServiceNameLabel); serviceName == "" {
err := helper.DeleteEndpointSlice(c.Client, labels.Set{
workv1alpha1.WorkNamespaceLabel: req.Namespace,
workv1alpha1.WorkNameLabel: req.Name,
})
return controllerruntime.Result{}, err
}
return c.collectEndpointSliceFromWork(work)
}
@ -81,7 +92,10 @@ func (c *EndpointSliceController) SetupWithManager(mgr controllerruntime.Manager
return util.GetLabelValue(createEvent.Object.GetLabels(), util.ServiceNameLabel) != ""
},
UpdateFunc: func(updateEvent event.UpdateEvent) bool {
return util.GetLabelValue(updateEvent.ObjectNew.GetLabels(), util.ServiceNameLabel) != ""
// TBD: We care about the work with label util.MultiClusterServiceNameLabel because the work is
// managed by service-export-controller and endpointslice-collect-controller now, We should delete this after the conflict is fixed.
return (util.GetLabelValue(updateEvent.ObjectNew.GetLabels(), util.ServiceNameLabel) != "" ||
util.GetLabelValue(updateEvent.ObjectNew.GetLabels(), util.MultiClusterServiceNameLabel) != "")
},
DeleteFunc: func(deleteEvent event.DeleteEvent) bool {
return util.GetLabelValue(deleteEvent.Object.GetLabels(), util.ServiceNameLabel) != ""

82
pkg/controllers/mcs/service_export_controller.go
View File

@ -20,6 +20,7 @@ import (
"context"
"fmt"
"reflect"
"strings"
"sync"
discoveryv1 "k8s.io/api/discovery/v1"
@ -32,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2"
@ -383,17 +385,11 @@ func (c *ServiceExportController) reportEndpointSliceWithEndpointSliceCreateOrUp
// reportEndpointSlice report EndPointSlice objects to control-plane.
func reportEndpointSlice(c client.Client, endpointSlice *unstructured.Unstructured, clusterName string) error {
executionSpace := names.GenerateExecutionSpaceName(clusterName)
workName := names.GenerateWorkName(endpointSlice.GetKind(), endpointSlice.GetName(), endpointSlice.GetNamespace())
workMeta := metav1.ObjectMeta{
Name: names.GenerateWorkName(endpointSlice.GetKind(), endpointSlice.GetName(), endpointSlice.GetNamespace()),
Namespace: executionSpace,
Labels: map[string]string{
util.ServiceNamespaceLabel: endpointSlice.GetNamespace(),
util.ServiceNameLabel: endpointSlice.GetLabels()[discoveryv1.LabelServiceName],
// indicate the Work should be not propagated since it's collected resource.
util.PropagationInstruction: util.PropagationInstructionSuppressed,
util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue,
},
workMeta, err := getEndpointSliceWorkMeta(c, executionSpace, workName, endpointSlice)
if err != nil {
return err
}
if err := helper.CreateOrUpdateWork(c, workMeta, endpointSlice); err != nil {
@ -403,6 +399,41 @@ func reportEndpointSlice(c client.Client, endpointSlice *unstructured.Unstructur
return nil
}
func getEndpointSliceWorkMeta(c client.Client, ns string, workName string, endpointSlice *unstructured.Unstructured) (metav1.ObjectMeta, error) {
existWork := &workv1alpha1.Work{}
var err error
if err = c.Get(context.Background(), types.NamespacedName{
Namespace: ns,
Name: workName,
}, existWork); err != nil && !apierrors.IsNotFound(err) {
klog.Errorf("Get EndpointSlice work(%s/%s) error:%v", ns, workName, err)
return metav1.ObjectMeta{}, err
}
labels := map[string]string{
util.ServiceNamespaceLabel: endpointSlice.GetNamespace(),
util.ServiceNameLabel: endpointSlice.GetLabels()[discoveryv1.LabelServiceName],
// indicate the Work should be not propagated since it's collected resource.
util.PropagationInstruction: util.PropagationInstructionSuppressed,
util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue,
util.EndpointSliceWorkManagedByLabel: util.ServiceExportKind,
}
if existWork.Labels == nil || (err != nil && apierrors.IsNotFound(err)) {
workMeta := metav1.ObjectMeta{Name: workName, Namespace: ns, Labels: labels}
return workMeta, nil
}
labels = util.DedupeAndMergeLabels(labels, existWork.Labels)
if value, ok := existWork.Labels[util.EndpointSliceWorkManagedByLabel]; ok {
controllerSet := sets.New[string]()
controllerSet.Insert(strings.Split(value, ".")...)
controllerSet.Insert(util.ServiceExportKind)
labels[util.EndpointSliceWorkManagedByLabel] = strings.Join(controllerSet.UnsortedList(), ".")
}
workMeta := metav1.ObjectMeta{Name: workName, Namespace: ns, Labels: labels}
return workMeta, nil
}
func cleanupWorkWithServiceExportDelete(c client.Client, serviceExportKey keys.FederatedKey) error {
executionSpace := names.GenerateExecutionSpaceName(serviceExportKey.Cluster)
@ -420,9 +451,8 @@ func cleanupWorkWithServiceExportDelete(c client.Client, serviceExportKey keys.F
}
var errs []error
for index, work := range workList.Items {
if err := c.Delete(context.TODO(), &workList.Items[index]); err != nil {
klog.Errorf("Failed to delete work(%s/%s), Error: %v", work.Namespace, work.Name, err)
for _, work := range workList.Items {
if err := cleanEndpointSliceWork(c, work.DeepCopy()); err != nil {
errs = append(errs, err)
}
}
@ -446,8 +476,32 @@ func cleanupWorkWithEndpointSliceDelete(c client.Client, endpointSliceKey keys.F
return err
}
return cleanEndpointSliceWork(c, work.DeepCopy())
}
// TBD: Currently, the EndpointSlice work can be handled by both service-export-controller and endpointslice-collect-controller. To indicate this, we've introduced the label endpointslice.karmada.io/managed-by. Therefore,
// if managed by both controllers, simply remove each controller's respective labels.
// If managed solely by its own controller, delete the work accordingly.
// This logic should be deleted after the conflict is fixed.
func cleanEndpointSliceWork(c client.Client, work *workv1alpha1.Work) error {
controllers := util.GetLabelValue(work.Labels, util.EndpointSliceWorkManagedByLabel)
controllerSet := sets.New[string]()
controllerSet.Insert(strings.Split(controllers, ".")...)
controllerSet.Delete(util.ServiceExportKind)
if controllerSet.Len() > 0 {
delete(work.Labels, util.ServiceNameLabel)
delete(work.Labels, util.ServiceNamespaceLabel)
work.Labels[util.EndpointSliceWorkManagedByLabel] = strings.Join(controllerSet.UnsortedList(), ".")
if err := c.Update(context.TODO(), work); err != nil {
klog.Errorf("Failed to update work(%s/%s): %v", work.Namespace, work.Name, err)
return err
}
return nil
}
if err := c.Delete(context.TODO(), work); err != nil {
klog.Errorf("Failed to delete work(%s), Error: %v", workNamespaceKey, err)
klog.Errorf("Failed to delete work(%s/%s), Error: %v", work.Namespace, work.Name, err)
return err
}

78
pkg/controllers/multiclusterservice/endpointslice_collect_controller.go
View File

@ -20,6 +20,7 @@ import (
"context"
"fmt"
"reflect"
"strings"
"sync"
discoveryv1 "k8s.io/api/discovery/v1"
@ -31,6 +32,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
controllerruntime "sigs.k8s.io/controller-runtime"
@ -374,18 +376,11 @@ func (c *EndpointSliceCollectController) reportEndpointSliceWithEndpointSliceCre
// reportEndpointSlice report EndPointSlice objects to control-plane.
func reportEndpointSlice(c client.Client, endpointSlice *unstructured.Unstructured, clusterName string) error {
executionSpace := names.GenerateExecutionSpaceName(clusterName)
workName := names.GenerateWorkName(endpointSlice.GetKind(), endpointSlice.GetName(), endpointSlice.GetNamespace())
workMeta := metav1.ObjectMeta{
// Karmada will synchronize this work to other cluster namespaces and add the cluster name to prevent conflicts.
Name: names.GenerateWorkName(endpointSlice.GetKind(), endpointSlice.GetName(), endpointSlice.GetNamespace()),
Namespace: executionSpace,
Labels: map[string]string{
util.MultiClusterServiceNamespaceLabel: endpointSlice.GetNamespace(),
util.MultiClusterServiceNameLabel: endpointSlice.GetLabels()[discoveryv1.LabelServiceName],
// indicate the Work should be not propagated since it's collected resource.
util.PropagationInstruction: util.PropagationInstructionSuppressed,
util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue,
},
workMeta, err := getEndpointSliceWorkMeta(c, executionSpace, workName, endpointSlice)
if err != nil {
return err
}
if err := helper.CreateOrUpdateWork(c, workMeta, endpointSlice); err != nil {
@ -396,6 +391,41 @@ func reportEndpointSlice(c client.Client, endpointSlice *unstructured.Unstructur
return nil
}
func getEndpointSliceWorkMeta(c client.Client, ns string, workName string, endpointSlice *unstructured.Unstructured) (metav1.ObjectMeta, error) {
existWork := &workv1alpha1.Work{}
var err error
if err = c.Get(context.Background(), types.NamespacedName{
Namespace: ns,
Name: workName,
}, existWork); err != nil && !apierrors.IsNotFound(err) {
klog.Errorf("Get EndpointSlice work(%s/%s) error:%v", ns, workName, err)
return metav1.ObjectMeta{}, err
}
labels := map[string]string{
util.MultiClusterServiceNamespaceLabel: endpointSlice.GetNamespace(),
util.MultiClusterServiceNameLabel: endpointSlice.GetLabels()[discoveryv1.LabelServiceName],
// indicate the Work should be not propagated since it's collected resource.
util.PropagationInstruction: util.PropagationInstructionSuppressed,
util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue,
util.EndpointSliceWorkManagedByLabel: util.MultiClusterServiceKind,
}
if existWork.Labels == nil || (err != nil && apierrors.IsNotFound(err)) {
workMeta := metav1.ObjectMeta{Name: workName, Namespace: ns, Labels: labels}
return workMeta, nil
}
labels = util.DedupeAndMergeLabels(labels, existWork.Labels)
if value, ok := existWork.Labels[util.EndpointSliceWorkManagedByLabel]; ok {
controllerSet := sets.New[string]()
controllerSet.Insert(strings.Split(value, ".")...)
controllerSet.Insert(util.MultiClusterServiceKind)
labels[util.EndpointSliceWorkManagedByLabel] = strings.Join(controllerSet.UnsortedList(), ".")
}
workMeta := metav1.ObjectMeta{Name: workName, Namespace: ns, Labels: labels}
return workMeta, nil
}
func cleanupWorkWithEndpointSliceDelete(c client.Client, endpointSliceKey keys.FederatedKey) error {
executionSpace := names.GenerateExecutionSpaceName(endpointSliceKey.Cluster)
@ -412,8 +442,32 @@ func cleanupWorkWithEndpointSliceDelete(c client.Client, endpointSliceKey keys.F
return err
}
return cleanProviderClustersEndpointSliceWork(c, work.DeepCopy())
}
// TBD: Currently, the EndpointSlice work can be handled by both service-export-controller and endpointslice-collect-controller. To indicate this, we've introduced the label endpointslice.karmada.io/managed-by. Therefore,
// if managed by both controllers, simply remove each controller's respective labels.
// If managed solely by its own controller, delete the work accordingly.
// This logic should be deleted after the conflict is fixed.
func cleanProviderClustersEndpointSliceWork(c client.Client, work *workv1alpha1.Work) error {
controllers := util.GetLabelValue(work.Labels, util.EndpointSliceWorkManagedByLabel)
controllerSet := sets.New[string]()
controllerSet.Insert(strings.Split(controllers, ".")...)
controllerSet.Delete(util.MultiClusterServiceKind)
if controllerSet.Len() > 0 {
delete(work.Labels, util.MultiClusterServiceNameLabel)
delete(work.Labels, util.MultiClusterServiceNamespaceLabel)
work.Labels[util.EndpointSliceWorkManagedByLabel] = strings.Join(controllerSet.UnsortedList(), ".")
if err := c.Update(context.TODO(), work); err != nil {
klog.Errorf("Failed to update work(%s/%s): %v", work.Namespace, work.Name, err)
return err
}
return nil
}
if err := c.Delete(context.TODO(), work); err != nil {
klog.Errorf("Failed to delete work(%s): %v", workNamespaceKey.String(), err)
klog.Errorf("Failed to delete work(%s/%s): %v", work.Namespace, work.Name, err)
return err
}

9
pkg/controllers/multiclusterservice/endpointslice_dispatch_controller.go
View File

@ -76,7 +76,8 @@ func (c *EndpointsliceDispatchController) Reconcile(ctx context.Context, req con
return controllerruntime.Result{}, nil
}
if !work.DeletionTimestamp.IsZero() {
mcsName := util.GetLabelValue(work.Labels, util.MultiClusterServiceNameLabel)
if !work.DeletionTimestamp.IsZero() || mcsName == "" {
if err := c.cleanupEndpointSliceFromConsumerClusters(ctx, work); err != nil {
klog.Errorf("Failed to cleanup EndpointSlice from consumer clusters for work %s/%s:%v", work.Namespace, work.Name, err)
return controllerruntime.Result{Requeue: true}, err
@ -84,7 +85,6 @@ func (c *EndpointsliceDispatchController) Reconcile(ctx context.Context, req con
return controllerruntime.Result{}, nil
}
mcsName := util.GetLabelValue(work.Labels, util.MultiClusterServiceNameLabel)
mcsNS := util.GetLabelValue(work.Labels, util.MultiClusterServiceNamespaceLabel)
mcs := &networkingv1alpha1.MultiClusterService{}
if err := c.Client.Get(ctx, types.NamespacedName{Namespace: mcsNS, Name: mcsName}, mcs); err != nil {
@ -152,7 +152,10 @@ func (c *EndpointsliceDispatchController) SetupWithManager(mgr controllerruntime
},
UpdateFunc: func(updateEvent event.UpdateEvent) bool {
// We only care about the EndpointSlice work from provider clusters
return util.GetLabelValue(updateEvent.ObjectNew.GetLabels(), util.MultiClusterServiceNameLabel) != "" &&
// TBD: We care about the work with label util.ServiceNameLabel because now service-export-controller and endpointslice-collect-controller
// will manage the work together, should delete this after the conflict is fixed
return (util.GetLabelValue(updateEvent.ObjectNew.GetLabels(), util.MultiClusterServiceNameLabel) != "" ||
util.GetLabelValue(updateEvent.ObjectNew.GetLabels(), util.ServiceNameLabel) != "") &&
util.GetAnnotationValue(updateEvent.ObjectNew.GetAnnotations(), util.EndpointSliceProvisionClusterAnnotation) == ""
},
DeleteFunc: func(deleteEvent event.DeleteEvent) bool {

18
pkg/controllers/multiclusterservice/mcs_controller.go
View File

@ -193,8 +193,7 @@ func (c *MCSController) cleanProviderEndpointSliceWork(work *workv1alpha1.Work)
continue
}
if err := c.Delete(context.TODO(), work.DeepCopy()); err != nil {
klog.Errorf("Failed to delete work(%s/%s), Error: %v", work.Namespace, work.Name, err)
if err := cleanProviderClustersEndpointSliceWork(c.Client, work.DeepCopy()); err != nil {
errs = append(errs, err)
}
}
@ -366,8 +365,19 @@ func (c *MCSController) retrieveService(mcs *networkingv1alpha1.MultiClusterServ
return err
}
if err := c.Client.Delete(context.Background(), rb); err != nil {
klog.Errorf("Failed to delete ResourceBinding(%s/%s):%v", mcs.Namespace, names.GenerateBindingName(svc.Kind, svc.Name), err)
// MultiClusterService is a specialized instance of PropagationPolicy, and when deleting PropagationPolicy, the resource ResourceBinding
// will not be removed to ensure service stability. MultiClusterService should also adhere to this design.
rbCopy := rb.DeepCopy()
if rbCopy.Annotations != nil {
delete(rbCopy.Annotations, networkingv1alpha1.MultiClusterServiceNameAnnotation)
delete(rbCopy.Annotations, networkingv1alpha1.MultiClusterServiceNamespaceAnnotation)
}
if rbCopy.Labels != nil {
delete(rbCopy.Labels, workv1alpha2.BindingManagedByLabel)
delete(rbCopy.Labels, networkingv1alpha1.MultiClusterServicePermanentIDLabel)
}
if err := c.Client.Update(context.Background(), rbCopy); err != nil {
klog.Errorf("Failed to update ResourceBinding(%s/%s):%v", mcs.Namespace, names.GenerateBindingName(svc.Kind, svc.Name), err)
return err
}

3
pkg/util/constants.go
View File

@ -65,6 +65,9 @@ const (
// ResourceTemplateClaimedByLabel is added to the ResourceTemplate, indicating which resource is in charge of propagating the ResourceTemplate.
ResourceTemplateClaimedByLabel = "resourcetemplate.karmada.io/claimed-by"
// EndpointSliceWorkManagedByLabel is added to the EndpointSlice work collected from member clusters, represents which manage the endpointslice work
EndpointSliceWorkManagedByLabel = "endpointslice.karmada.io/managed-by"
)
const (

17
test/e2e/mcs_test.go
View File

@ -544,9 +544,6 @@ var _ = ginkgo.Describe("CrossCluster MultiClusterService testing", func() {
ginkgo.AfterEach(func() {
framework.RemoveMultiClusterService(karmadaClient, testNamespace, mcsName)
framework.RemovePropagationPolicy(karmadaClient, testNamespace, policyName)
framework.WaitServiceDisappearOnCluster(member1Name, testNamespace, serviceName)
framework.WaitServiceDisappearOnCluster(member2Name, testNamespace, serviceName)
})
ginkgo.It("Test dispatch EndpointSlice from the provider clusters to the consumer clusters", func() {
@ -580,9 +577,6 @@ var _ = ginkgo.Describe("CrossCluster MultiClusterService testing", func() {
ginkgo.AfterEach(func() {
framework.RemoveMultiClusterService(karmadaClient, testNamespace, mcsName)
framework.RemovePropagationPolicy(karmadaClient, testNamespace, policyName)
framework.WaitServiceDisappearOnCluster(member1Name, testNamespace, serviceName)
framework.WaitServiceDisappearOnCluster(member2Name, testNamespace, serviceName)
})
ginkgo.It("Test dispatch EndpointSlice from the provider clusters to the consumer clusters", func() {
@ -618,9 +612,6 @@ var _ = ginkgo.Describe("CrossCluster MultiClusterService testing", func() {
ginkgo.AfterEach(func() {
framework.RemoveMultiClusterService(karmadaClient, testNamespace, mcsName)
framework.RemovePropagationPolicy(karmadaClient, testNamespace, policyName)
framework.WaitServiceDisappearOnCluster(member1Name, testNamespace, serviceName)
framework.WaitServiceDisappearOnCluster(member2Name, testNamespace, serviceName)
})
ginkgo.It("Test dispatch EndpointSlice from the provider clusters to the consumer clusters", func() {
@ -666,8 +657,6 @@ var _ = ginkgo.Describe("CrossCluster MultiClusterService testing", func() {
ginkgo.AfterEach(func() {
framework.RemoveMultiClusterService(karmadaClient, testNamespace, mcsName)
framework.RemovePropagationPolicy(karmadaClient, testNamespace, policyName)
framework.WaitServiceDisappearOnCluster(member2Name, testNamespace, serviceName)
})
ginkgo.It("Test dispatch EndpointSlice from the provider clusters to the consumer clusters", func() {
@ -701,9 +690,6 @@ var _ = ginkgo.Describe("CrossCluster MultiClusterService testing", func() {
ginkgo.AfterEach(func() {
framework.RemoveMultiClusterService(karmadaClient, testNamespace, mcsName)
framework.RemovePropagationPolicy(karmadaClient, testNamespace, policyName)
framework.WaitServiceDisappearOnCluster(member1Name, testNamespace, serviceName)
framework.WaitServiceDisappearOnCluster(member2Name, testNamespace, serviceName)
})
ginkgo.It("Test dispatch EndpointSlice from the provider clusters to the consumer clusters", func() {
@ -737,9 +723,6 @@ var _ = ginkgo.Describe("CrossCluster MultiClusterService testing", func() {
ginkgo.AfterEach(func() {
framework.RemoveMultiClusterService(karmadaClient, testNamespace, mcsName)
framework.RemovePropagationPolicy(karmadaClient, testNamespace, policyName)
framework.WaitServiceDisappearOnCluster(member1Name, testNamespace, serviceName)
framework.WaitServiceDisappearOnCluster(member2Name, testNamespace, serviceName)
})
ginkgo.It("Test dispatch EndpointSlice from the provider clusters to the consumer clusters", func() {