From f19be845afb0317c70d6b392b49dc75478e1aac4 Mon Sep 17 00:00:00 2001 From: hjiajing Date: Wed, 22 Mar 2023 15:07:09 +0800 Subject: [PATCH] Add cluster's Service CIDR to Multi-cluster Gateway Signed-off-by: hjiajing --- .../multicluster/v1alpha1/gateway_types.go | 2 + .../yamls/antrea-multicluster-member.yml | 3 ++ .../gateway_webhook.go | 8 ++++ .../gateway_webhook_test.go | 36 ++++++++++++++++++ .../cmd/multicluster-controller/member.go | 1 + .../multicluster.crd.antrea.io_gateways.yaml | 3 ++ .../multicluster/member/gateway_controller.go | 18 +-------- .../member/gateway_controller_test.go | 8 ---- .../multicluster/member/node_controller.go | 38 ++++++++++++++++--- .../member/node_controller_test.go | 16 +++++++- 10 files changed, 100 insertions(+), 33 deletions(-) diff --git a/multicluster/apis/multicluster/v1alpha1/gateway_types.go b/multicluster/apis/multicluster/v1alpha1/gateway_types.go index c1c0ec10d34..f5bfdab20a3 100644 --- a/multicluster/apis/multicluster/v1alpha1/gateway_types.go +++ b/multicluster/apis/multicluster/v1alpha1/gateway_types.go @@ -41,6 +41,8 @@ type Gateway struct { GatewayIP string `json:"gatewayIP,omitempty"` // In-cluster tunnel IP of the Gateway. InternalIP string `json:"internalIP,omitempty"` + // Service CIDR of the local cluster. + ServiceCIDR string `json:"serviceCIDR,omitempty"` } type ClusterInfo struct { diff --git a/multicluster/build/yamls/antrea-multicluster-member.yml b/multicluster/build/yamls/antrea-multicluster-member.yml index 715205ea511..10d174ad3a1 100644 --- a/multicluster/build/yamls/antrea-multicluster-member.yml +++ b/multicluster/build/yamls/antrea-multicluster-member.yml @@ -398,6 +398,9 @@ spec: type: string metadata: type: object + serviceCIDR: + description: Service CIDR of the local cluster. + type: string type: object served: true storage: true diff --git a/multicluster/cmd/multicluster-controller/gateway_webhook.go b/multicluster/cmd/multicluster-controller/gateway_webhook.go index b22377c3164..b1ca405e294 100644 --- a/multicluster/cmd/multicluster-controller/gateway_webhook.go +++ b/multicluster/cmd/multicluster-controller/gateway_webhook.go @@ -59,6 +59,14 @@ func (v *gatewayValidator) Handle(ctx context.Context, req admission.Request) ad klog.ErrorS(err, "failed to create Gateway", "Gateway", klog.KObj(gateway), "Namespace", v.namespace) return admission.Errored(http.StatusPreconditionFailed, err) } + if req.Operation == admissionv1.Update && len(gatewayList.Items) > 0 { + oldGateway := gatewayList.Items[0] + if oldGateway.ServiceCIDR != gateway.ServiceCIDR { + klog.ErrorS(err, "The field 'serviceCIDR' is immutable", "Gateway", klog.KObj(gateway)) + return admission.Denied("the field 'serviceCIDR' is immutable") + } + return admission.Allowed("") + } return admission.Allowed("") } diff --git a/multicluster/cmd/multicluster-controller/gateway_webhook_test.go b/multicluster/cmd/multicluster-controller/gateway_webhook_test.go index 268cb78ed72..14bb6417024 100644 --- a/multicluster/cmd/multicluster-controller/gateway_webhook_test.go +++ b/multicluster/cmd/multicluster-controller/gateway_webhook_test.go @@ -52,8 +52,16 @@ func TestWebhookGatewayEvents(t *testing.T) { Name: "node-2", }, } + updatedGateway := &mcsv1alpha1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "node-2", + }, + ServiceCIDR: "10.100.0.0/16", + } newGW, _ := json.Marshal(newGateway) + updatedGW, _ := json.Marshal(updatedGateway) newReq := admission.Request{ AdmissionRequest: v1.AdmissionRequest{ @@ -77,6 +85,28 @@ func TestWebhookGatewayEvents(t *testing.T) { }, } + updateReq := admission.Request{ + AdmissionRequest: v1.AdmissionRequest{ + UID: "07e52e8d-4513-11e9-a716-42010a800270", + Kind: metav1.GroupVersionKind{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Kind: "Gateway", + }, + Resource: metav1.GroupVersionResource{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Resource: "Gateways", + }, + Name: "node-2", + Namespace: "default", + Operation: v1.Update, + Object: runtime.RawExtension{ + Raw: updatedGW, + }, + }, + } + newReqCopy := newReq.DeepCopy() invalidReq := admission.Request{ AdmissionRequest: *newReqCopy, @@ -106,6 +136,12 @@ func TestWebhookGatewayEvents(t *testing.T) { req: invalidReq, isAllowed: false, }, + { + name: "failed to update a Gateway's ServiceCIDR field", + existingGateway: existingGateway, + req: updateReq, + isAllowed: false, + }, } newScheme := runtime.NewScheme() diff --git a/multicluster/cmd/multicluster-controller/member.go b/multicluster/cmd/multicluster-controller/member.go index a5d0538da6a..1c292f9f45b 100644 --- a/multicluster/cmd/multicluster-controller/member.go +++ b/multicluster/cmd/multicluster-controller/member.go @@ -107,6 +107,7 @@ func runMember(o *Options) error { mgr.GetClient(), mgr.GetScheme(), env.GetPodNamespace(), + opts.ServiceCIDR, opts.GatewayIPPrecedence) if err = nodeReconciler.SetupWithManager(mgr); err != nil { return fmt.Errorf("error creating Node controller: %v", err) diff --git a/multicluster/config/crd/bases/multicluster.crd.antrea.io_gateways.yaml b/multicluster/config/crd/bases/multicluster.crd.antrea.io_gateways.yaml index 6973bd054e8..1c1c2ce40b3 100644 --- a/multicluster/config/crd/bases/multicluster.crd.antrea.io_gateways.yaml +++ b/multicluster/config/crd/bases/multicluster.crd.antrea.io_gateways.yaml @@ -50,6 +50,9 @@ spec: type: string metadata: type: object + serviceCIDR: + description: Service CIDR of the local cluster. + type: string type: object served: true storage: true diff --git a/multicluster/controllers/multicluster/member/gateway_controller.go b/multicluster/controllers/multicluster/member/gateway_controller.go index 2dd50715983..f58fa799a81 100644 --- a/multicluster/controllers/multicluster/member/gateway_controller.go +++ b/multicluster/controllers/multicluster/member/gateway_controller.go @@ -18,7 +18,6 @@ package member import ( "context" - "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -85,10 +84,6 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{Requeue: true}, err } r.leaderNamespace = commonArea.GetNamespace() - err = r.getServiceCIDR(ctx) - if err != nil { - return ctrl.Result{}, err - } resExportName := common.NewClusterInfoResourceExportName(r.localClusterID) resExportNamespacedName := types.NamespacedName{ @@ -132,6 +127,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } return ctrl.Result{}, nil } + r.serviceCIDR = gw.ServiceCIDR if err := createOrUpdate(gw.GatewayIP); err != nil { return ctrl.Result{}, err @@ -206,15 +202,3 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { }). Complete(r) } - -// getServiceCIDR gets Service ClusterIP CIDR used in the member cluster. -func (r *GatewayReconciler) getServiceCIDR(ctx context.Context) error { - if len(r.serviceCIDR) == 0 { - serviceCIDR, err := common.DiscoverServiceCIDRByInvalidServiceCreation(ctx, r.Client, r.namespace) - if err != nil { - return fmt.Errorf("failed to find Service ClusterIP range automatically, you may set the 'serviceCIDR' config as an alternative, err: %v, ", err) - } - r.serviceCIDR = serviceCIDR - } - return nil -} diff --git a/multicluster/controllers/multicluster/member/gateway_controller_test.go b/multicluster/controllers/multicluster/member/gateway_controller_test.go index 8d78eda6da7..9f972afe3f9 100644 --- a/multicluster/controllers/multicluster/member/gateway_controller_test.go +++ b/multicluster/controllers/multicluster/member/gateway_controller_test.go @@ -17,7 +17,6 @@ limitations under the License. package member import ( - "context" "reflect" "testing" "time" @@ -186,10 +185,3 @@ func TestGatewayReconciler(t *testing.T) { }) } } - -func TestGetServiceCIDR(t *testing.T) { - fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects().Build() - r := NewGatewayReconciler(fakeClient, common.TestScheme, "default", "", []string{"10.200.1.1/16"}, nil) - err := r.getServiceCIDR(context.TODO()) - assert.Contains(t, err.Error(), "expected a specific error but none was returned") -} diff --git a/multicluster/controllers/multicluster/member/node_controller.go b/multicluster/controllers/multicluster/member/node_controller.go index 89683db3a21..98a087cba76 100644 --- a/multicluster/controllers/multicluster/member/node_controller.go +++ b/multicluster/controllers/multicluster/member/node_controller.go @@ -44,6 +44,7 @@ type ( precedence mcsv1alpha1.Precedence gatewayCandidates map[string]bool activeGateway string + serviceCIDR string initialized bool } ) @@ -57,6 +58,7 @@ func NewNodeReconciler( client client.Client, scheme *runtime.Scheme, namespace string, + serviceCIDR string, precedence mcsv1alpha1.Precedence) *NodeReconciler { if string(precedence) == "" { precedence = mcsv1alpha1.PrecedenceInternal @@ -65,6 +67,7 @@ func NewNodeReconciler( Client: client, Scheme: scheme, namespace: namespace, + serviceCIDR: serviceCIDR, precedence: precedence, gatewayCandidates: make(map[string]bool), } @@ -116,12 +119,15 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. var isValidGateway bool if stillGatewayNode { + gw.ServiceCIDR, err = r.getServiceCIDR() + if err != nil { + klog.ErrorS(err, "Failed to discover Service CIDR in cluster") + } gw.InternalIP, gw.GatewayIP, err = r.getGatawayNodeIP(node) if err != nil { klog.ErrorS(err, "There is no valid Gateway IP for Node", "node", node.Name) - } else { - isValidGateway = true } + isValidGateway = err == nil } if isActiveGateway { @@ -194,11 +200,13 @@ func (r *NodeReconciler) updateActiveGateway(ctx context.Context, newGateway *mc if err := r.Client.Get(ctx, types.NamespacedName{Name: newGateway.Name, Namespace: r.namespace}, existingGW); err != nil { return err } - if existingGW.GatewayIP == newGateway.GatewayIP && existingGW.InternalIP == newGateway.InternalIP { + if existingGW.GatewayIP == newGateway.GatewayIP && existingGW.InternalIP == newGateway.InternalIP && + existingGW.ServiceCIDR == newGateway.ServiceCIDR { return nil } existingGW.GatewayIP = newGateway.GatewayIP existingGW.InternalIP = newGateway.InternalIP + existingGW.ServiceCIDR = newGateway.ServiceCIDR // If the Gateway version in the client cache is stale, the update operation will fail, // then the reconciler will retry with latest state again. if err := r.Client.Update(ctx, existingGW, &client.UpdateOptions{}); err != nil { @@ -230,7 +238,7 @@ func (r *NodeReconciler) recreateActiveGateway(ctx context.Context, gateway *mcs // creates a Gateway. It returns no error if no good Gateway candidate. func (r *NodeReconciler) getValidGatewayFromCandidates() (*mcsv1alpha1.Gateway, error) { var activeGateway *mcsv1alpha1.Gateway - var internalIP, gwIP string + var internalIP, gwIP, serviceCIDR string var err error gatewayNode := &corev1.Node{} for name := range r.gatewayCandidates { @@ -242,13 +250,19 @@ func (r *NodeReconciler) getValidGatewayFromCandidates() (*mcsv1alpha1.Gateway, klog.V(2).ErrorS(err, "Node has no valid IP", "node", gatewayNode.Name) continue } + if serviceCIDR, err = r.getServiceCIDR(); err != nil { + klog.V(2).ErrorS(err, "Failed to discover Service CIDR in local cluster") + continue + } + activeGateway = &mcsv1alpha1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Name: gatewayNode.Name, Namespace: r.namespace, }, - GatewayIP: gwIP, - InternalIP: internalIP, + GatewayIP: gwIP, + InternalIP: internalIP, + ServiceCIDR: serviceCIDR, } klog.InfoS("Found good Gateway candidate", "node", gatewayNode.Name) return activeGateway, nil @@ -297,6 +311,18 @@ func (r *NodeReconciler) getGatawayNodeIP(node *corev1.Node) (string, string, er return internalIP, gatewayIP, nil } +func (r *NodeReconciler) getServiceCIDR() (string, error) { + if r.serviceCIDR != "" { + return r.serviceCIDR, nil + } + serviceCIDR, err := common.DiscoverServiceCIDRByInvalidServiceCreation(context.TODO(), r.Client, r.namespace) + if err != nil { + return "", err + } + r.serviceCIDR = serviceCIDR + return r.serviceCIDR, nil +} + // SetupWithManager sets up the controller with the Manager. func (r *NodeReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/multicluster/controllers/multicluster/member/node_controller_test.go b/multicluster/controllers/multicluster/member/node_controller_test.go index 31d382ef728..9deaff6b012 100644 --- a/multicluster/controllers/multicluster/member/node_controller_test.go +++ b/multicluster/controllers/multicluster/member/node_controller_test.go @@ -231,7 +231,7 @@ func TestNodeReconciler(t *testing.T) { obj = append(obj, tt.existingGW) } fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects(obj...).Build() - r := NewNodeReconciler(fakeClient, common.TestScheme, "default", tt.precedence) + r := NewNodeReconciler(fakeClient, common.TestScheme, "default", "10.100.0.0/16", tt.precedence) r.activeGateway = tt.activeGateway if _, err := r.Reconcile(common.TestCtx, tt.req); err != nil { t.Errorf("Node Reconciler should handle Node events successfully but got error = %v", err) @@ -307,7 +307,7 @@ func TestInitialize(t *testing.T) { obj = append(obj, tt.existingGW) } fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects(obj...).Build() - r := NewNodeReconciler(fakeClient, common.TestScheme, "default", mcsv1alpha1.PrecedencePublic) + r := NewNodeReconciler(fakeClient, common.TestScheme, "default", "10.100.0.0/16", mcsv1alpha1.PrecedencePublic) if err := r.initialize(); err != nil { t.Errorf("Expected initialize() successfully but got err: %v", err) } else { @@ -325,3 +325,15 @@ func TestInitialize(t *testing.T) { }) } } + +func TestGetServiceCIDR(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects().Build() + r := NewNodeReconciler(fakeClient, common.TestScheme, "default", "", mcsv1alpha1.PrecedencePublic) + + _, err := r.getServiceCIDR() + assert.Contains(t, err.Error(), "expected a specific error but none was returned") + r.serviceCIDR = "10.100.0.0/16" + serviceCIDR, err := r.getServiceCIDR() + assert.Nil(t, err) + assert.Equal(t, "10.100.0.0/16", serviceCIDR) +}