From d354db491963f077da3024066affb099792b265b Mon Sep 17 00:00:00 2001 From: Lan Luo Date: Thu, 6 Apr 2023 17:10:42 +0800 Subject: [PATCH] Disable ExternalName type of Service exporting ExternalName type of Service shouldn't be exported since it has no ClusterIP assinged, and it's not supported by KEP1645 either. So add a few validation codes to disable ExternalName type of Service exporting. Signed-off-by: Lan Luo --- .../member/serviceexport_controller.go | 14 +++++++++ .../member/serviceexport_controller_test.go | 31 ++++++++++++++++--- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/multicluster/controllers/multicluster/member/serviceexport_controller.go b/multicluster/controllers/multicluster/member/serviceexport_controller.go index d1073f76d61..4aa3e07c214 100644 --- a/multicluster/controllers/multicluster/member/serviceexport_controller.go +++ b/multicluster/controllers/multicluster/member/serviceexport_controller.go @@ -79,6 +79,7 @@ type reason int const ( serviceNotFound reason = iota + serviceNotSupported isImportedService serviceWithoutEndpoints serviceExported @@ -199,6 +200,16 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, err } + // The ExternalName type of Service is not supported since it has no ClusterIP + // assgined to the Service. + if svc.Spec.Type == corev1.ServiceTypeExternalName { + err = r.updateSvcExportStatus(ctx, req, serviceNotSupported) + if err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + // Skip if ServiceExport is trying to export a multi-cluster Service. if !svcInstalled { if _, ok := svc.Annotations[common.AntreaMCServiceAnnotation]; ok { @@ -402,6 +413,9 @@ func (r *ServiceExportReconciler) updateSvcExportStatus(ctx context.Context, req case serviceNotFound: newCondition.Reason = getStringPointer("service_not_found") newCondition.Message = getStringPointer("the Service does not exist") + case serviceNotSupported: + newCondition.Reason = getStringPointer("service_type_not_supported") + newCondition.Message = getStringPointer("the ExternalName type of Service is not supported") case serviceWithoutEndpoints: newCondition.Reason = getStringPointer("service_without_endpoints") newCondition.Message = getStringPointer("the Service has no Endpoints, failed to export") diff --git a/multicluster/controllers/multicluster/member/serviceexport_controller_test.go b/multicluster/controllers/multicluster/member/serviceexport_controller_test.go index 3dae666080d..d57c5c94abf 100644 --- a/multicluster/controllers/multicluster/member/serviceexport_controller_test.go +++ b/multicluster/controllers/multicluster/member/serviceexport_controller_test.go @@ -178,6 +178,17 @@ func TestServiceExportReconciler_CheckExportStatus(t *testing.T) { nginx2SvcExportWithStatus.Name = "nginx2" existingMessage := "the Service has no related Endpoints" nginx2SvcExportWithStatus.Status.Conditions[0].Message = &existingMessage + + nginx3Svc := common.SvcNginx.DeepCopy() + nginx3Svc.Name = "nginx3" + nginx3Svc.Spec.Type = corev1.ServiceTypeExternalName + nginx3SvcExport := &k8smcsv1alpha1.ServiceExport{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "nginx3", + }, + } + tests := []struct { name string expectedReason string @@ -185,9 +196,19 @@ func TestServiceExportReconciler_CheckExportStatus(t *testing.T) { req ctrl.Request }{ { - name: "export non-existing Service", - expectedReason: "service_not_found", - req: nginxReq, + name: "export non-existing Service", + expectedReason: "service_not_found", + expectedMessage: "the Service does not exist", + req: nginxReq, + }, + { + name: "export ExternalName type of Service", + expectedReason: "service_type_not_supported", + expectedMessage: "the ExternalName type of Service is not supported", + req: ctrl.Request{NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "nginx3", + }}, }, { name: "export multi-cluster Service", @@ -224,8 +245,8 @@ func TestServiceExportReconciler_CheckExportStatus(t *testing.T) { }, } - fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects(mcsSvc, nginx0Svc, nginx1Svc, nginx1EP, nginx2Svc, existSvcExport, - nginx0SvcExport, nginx1SvcExportWithStatus, nginx2SvcExportWithStatus, mcsSvcExport).Build() + fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects(mcsSvc, nginx0Svc, nginx1Svc, nginx3Svc, nginx1EP, nginx2Svc, existSvcExport, + nginx0SvcExport, nginx1SvcExportWithStatus, nginx2SvcExportWithStatus, nginx3SvcExport, mcsSvcExport).Build() fakeRemoteClient := fake.NewClientBuilder().WithScheme(common.TestScheme).Build() commonArea := commonarea.NewFakeRemoteCommonArea(fakeRemoteClient, "leader-cluster", common.LocalClusterID, "default", nil)