From 298292089321870c8f8c6d04fc3466459f4f4fb2 Mon Sep 17 00:00:00 2001 From: Yang Ding Date: Fri, 18 Mar 2022 12:04:27 -0700 Subject: [PATCH] Address final comments Signed-off-by: Yang Ding --- docs/multicluster/getting-started.md | 4 +- .../antrea-multicluster-leader-namespaced.yml | 18 ------- .../yamls/antrea-multicluster-member.yml | 4 -- .../leader-ns/member_cluster_role.yaml | 4 -- multicluster/config/rbac/role.yaml | 4 -- .../acnp_resourceimport_controller.go | 48 ++++++------------- .../commonarea/resourceimport_controller.go | 2 +- 7 files changed, 17 insertions(+), 67 deletions(-) diff --git a/docs/multicluster/getting-started.md b/docs/multicluster/getting-started.md index df5c280ffac..d9d42cf9b12 100644 --- a/docs/multicluster/getting-started.md +++ b/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. diff --git a/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml b/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml index 0d82029de18..3b86f1926fd 100644 --- a/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml +++ b/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: diff --git a/multicluster/build/yamls/antrea-multicluster-member.yml b/multicluster/build/yamls/antrea-multicluster-member.yml index 68bf62344a4..54e526df71a 100644 --- a/multicluster/build/yamls/antrea-multicluster-member.yml +++ b/multicluster/build/yamls/antrea-multicluster-member.yml @@ -5282,10 +5282,6 @@ rules: - events verbs: - create - - get - - list - - patch - - update - apiGroups: - "" resources: diff --git a/multicluster/config/overlays/leader-ns/member_cluster_role.yaml b/multicluster/config/overlays/leader-ns/member_cluster_role.yaml index 0357f3f2131..b55887be4de 100644 --- a/multicluster/config/overlays/leader-ns/member_cluster_role.yaml +++ b/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: diff --git a/multicluster/config/rbac/role.yaml b/multicluster/config/rbac/role.yaml index d471b756f65..d5f02c7eea3 100644 --- a/multicluster/config/rbac/role.yaml +++ b/multicluster/config/rbac/role.yaml @@ -24,10 +24,6 @@ rules: - events verbs: - create - - get - - list - - patch - - update - apiGroups: - "" resources: diff --git a/multicluster/controllers/multicluster/commonarea/acnp_resourceimport_controller.go b/multicluster/controllers/multicluster/commonarea/acnp_resourceimport_controller.go index 661a7f66e37..2e8b3ccea3e 100644 --- a/multicluster/controllers/multicluster/commonarea/acnp_resourceimport_controller.go +++ b/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,11 +32,7 @@ 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" @@ -45,7 +40,6 @@ var ( 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,7 +91,7 @@ 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 { @@ -100,9 +99,9 @@ func (r *ResourceImportReconciler) handleResImpUpdateForClusterNetworkPolicy(ctx 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) -} diff --git a/multicluster/controllers/multicluster/commonarea/resourceimport_controller.go b/multicluster/controllers/multicluster/commonarea/resourceimport_controller.go index 9d6b0c08abc..5a2d0b816c1 100644 --- a/multicluster/controllers/multicluster/commonarea/resourceimport_controller.go +++ b/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.