Address final comments

Signed-off-by: Yang Ding <[email protected]>

docs/multicluster/getting-started.md

@@ -420,8 +420,8 @@ the ClusterSet to be applied with a consistent security posture (for example, al
 clusters can only communicate with Pods in their own namespaces). For more information regarding
 Antrea ClusterNetworkPolicy (ACNP), refer to [this document](../antrea-network-policy.md).
 
-To achieve such ACNP copy-span replication across clusters, admins can, in the acting leader cluster of
-a Multi-cluster deployment, create a ResourceExport of kind `AntreaClusterNetworkPolicy` which contains
+To achieve such ACNP replication across clusters, admins can, in the acting leader cluster of a
+Multi-cluster deployment, create a ResourceExport of kind `AntreaClusterNetworkPolicy` which contains
 the ClusterNetworkPolicy spec they wish to be replicated. The ResourceExport should be created in the
 Namespace which implements the  Common Area of the ClusterSet. In future releases, some additional tooling
 may become available to automate the creation of such ResourceExport and make ACNP replication easier.

multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml

@@ -39,10 +39,6 @@ rules:
   - events
   verbs:
   - create
-  - get
-  - list
-  - patch
-  - update
 - apiGroups:
   - ""
   resources:
@@ -352,20 +348,6 @@ rules:
   - events
   verbs:
   - create
-  - get
-  - list
-  - patch
-  - update
-- apiGroups:
-  - ""
-  resources:
-  - events
-  verbs:
-  - create
-  - get
-  - list
-  - patch
-  - update
 - apiGroups:
   - multicluster.crd.antrea.io
   resources:

multicluster/build/yamls/antrea-multicluster-member.yml

@@ -5282,10 +5282,6 @@ rules:
   - events
   verbs:
   - create
-  - get
-  - list
-  - patch
-  - update
 - apiGroups:
   - ""
   resources:

multicluster/config/overlays/leader-ns/member_cluster_role.yaml

@@ -12,10 +12,6 @@ rules:
       - events
     verbs:
       - create
-      - get
-      - list
-      - patch
-      - update
   - apiGroups:
       - multicluster.crd.antrea.io
     resources:

multicluster/config/rbac/role.yaml

@@ -24,10 +24,6 @@ rules:
   - events
   verbs:
   - create
-  - get
-  - list
-  - patch
-  - update
 - apiGroups:
   - ""
   resources:

multicluster/controllers/multicluster/commonarea/acnp_resourceimport_controller.go

@@ -17,7 +17,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"math/rand"
 
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -33,19 +32,14 @@ import (
 	"antrea.io/antrea/pkg/apis/crd/v1alpha1"
 )
 
-const (
-	nameSuffixLength       int    = 5
-	acnpImportStatusPrefix string = "acnp-import-status-"
-	acnpImportFailed       string = "ACNPImportFailed"
-)
+const acnpImportFailed string = "ACNPImportFailed"
 
 var (
 	resourceImportAPIVersion     = "multicluster.crd.antrea.io/v1alpha1"
 	resourceImportKind           = "ResourceImport"
 	acnpEventReportingController = "resourceimport-controller"
 	// TODO(yang): add run-time pod suffix
 	acnpEventReportingInstance = "antrea-mc-controller"
-	lettersAndDigits           = []rune("abcdefghijklmnopqrstuvwxyz0123456789")
 )
 
 func (r *ResourceImportReconciler) handleResImpUpdateForClusterNetworkPolicy(ctx context.Context, resImp *multiclusterv1alpha1.ResourceImport) (ctrl.Result, error) {
@@ -71,9 +65,14 @@ func (r *ResourceImportReconciler) handleResImpUpdateForClusterNetworkPolicy(ctx
 		}
 	}
 	acnpObj := getMCAntreaClusterPolicy(resImp)
-	tierKind, tierName := &v1alpha1.Tier{}, acnpObj.Spec.Tier
-	err = r.localClusterClient.Get(ctx, types.NamespacedName{Namespace: "", Name: tierName}, tierKind)
+	tierObj, tierName := &v1alpha1.Tier{}, acnpObj.Spec.Tier
+	err = r.localClusterClient.Get(ctx, types.NamespacedName{Namespace: "", Name: tierName}, tierObj)
 	tierNotFound := apierrors.IsNotFound(err)
+	if err != nil && !tierNotFound {
+		msg := fmt.Sprintf("Failed to get Tier %s in member cluster %s", tierName, r.localClusterID)
+		return ctrl.Result{}, r.reportStatusEvent(msg, ctx, resImp)
+	}
+	tierNotFoundMsg := fmt.Sprintf("ACNP Tier %s does not exist in importing cluster %s", tierName, r.localClusterID)
 	if !tierNotFound {
 		// If the ACNP Tier exists in the importing member cluster, then the policy is realizable.
 		// Create or update the ACNP if necessary.
@@ -92,17 +91,17 @@ func (r *ResourceImportReconciler) handleResImpUpdateForClusterNetworkPolicy(ctx
 				return ctrl.Result{}, r.reportStatusEvent(msg, ctx, resImp)
 			}
 		}
-	} else if tierNotFound && !acnpNotFound {
+	} else if !acnpNotFound {
 		// The ACNP Tier does not exist, and the policy cannot be realized in this particular importing member cluster.
 		// If there is an ACNP previously created via import (which has a valid Tier by then), it should be cleaned up.
 		if err = r.localClusterClient.Delete(ctx, acnpObj, &client.DeleteOptions{}); err != nil {
 			msg := "Failed to delete imported Antrea ClusterNetworkPolicy that no longer has a valid Tier for cluster " + r.localClusterID
 			klog.ErrorS(err, msg, "acnp", klog.KObj(acnpObj))
 			return ctrl.Result{}, r.reportStatusEvent(msg, ctx, resImp)
 		}
-	} else if tierNotFound {
-		msg := fmt.Sprintf("ACNP Tier %s does not exist in importing cluster %s", tierName, r.localClusterID)
-		return ctrl.Result{}, r.reportStatusEvent(msg, ctx, resImp)
+		return ctrl.Result{}, r.reportStatusEvent(tierNotFoundMsg, ctx, resImp)
+	} else {
+		return ctrl.Result{}, r.reportStatusEvent(tierNotFoundMsg, ctx, resImp)
 	}
 	return ctrl.Result{}, nil
 }
@@ -132,9 +131,6 @@ func (r *ResourceImportReconciler) handleResImpDeleteForClusterNetworkPolicy(ctx
 }
 
 func getMCAntreaClusterPolicy(resImp *multiclusterv1alpha1.ResourceImport) *v1alpha1.ClusterNetworkPolicy {
-	if resImp.Spec.ClusterNetworkPolicy == nil {
-		return nil
-	}
 	return &v1alpha1.ClusterNetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: common.AntreaMCSPrefix + resImp.Spec.Name,
@@ -147,12 +143,10 @@ func getMCAntreaClusterPolicy(resImp *multiclusterv1alpha1.ResourceImport) *v1al
 }
 
 func (r *ResourceImportReconciler) reportStatusEvent(errMsg string, ctx context.Context, resImp *multiclusterv1alpha1.ResourceImport) error {
-	if errMsg == "" {
-		return nil
-	}
+	t := metav1.Now()
 	statusEvent := &corev1.Event{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      randName(acnpImportStatusPrefix + r.localClusterID + "-"),
+			Name:      fmt.Sprintf("%v.%x", resImp.Name, t.UnixNano()),
 			Namespace: resImp.Namespace,
 		},
 		Type:    corev1.EventTypeWarning,
@@ -177,17 +171,3 @@ func (r *ResourceImportReconciler) reportStatusEvent(errMsg string, ctx context.
 	}
 	return nil
 }
-
-func randSeq(n int) string {
-	b := make([]rune, n)
-	for i := range b {
-		// #nosec G404: random number generator not used for security purposes
-		randIdx := rand.Intn(len(lettersAndDigits))
-		b[i] = lettersAndDigits[randIdx]
-	}
-	return string(b)
-}
-
-func randName(prefix string) string {
-	return prefix + randSeq(nameSuffixLength)
-}

multicluster/controllers/multicluster/commonarea/resourceimport_controller.go

@@ -86,7 +86,7 @@ func NewResourceImportReconciler(client client.Client, scheme *runtime.Scheme, l
 //+kubebuilder:rbac:groups=multicluster.x-k8s.io,resources=serviceimports/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups="",resources=endpoints,verbs=get;list;watch;update;create;patch;delete
 //+kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;update;create;patch;delete
-//+kubebuilder:rbac:groups="",resources=events,verbs=get;list;update;create;patch
+//+kubebuilder:rbac:groups="",resources=events,verbs=create
 
 // Reconcile will attempt to ensure that the imported Resource is installed in local cluster as per the
 // ResourceImport object.