Skip to content

Commit

Permalink
Remove saved ClusterSet from Multi-cluster ClusterSet reconciler (#5634)
Browse files Browse the repository at this point in the history 
The ClusterSet resource saved in the leader and member ClusterSet
reconcilers is not really useful. This commit removes this field from
the reconcilers and uses other fields to read ClusterSet information
instead.

Signed-off-by: Jianjun Shen <[email protected]>
  • Loading branch information
@jianjuns
jianjuns authored Oct 30, 2023
1 parent ae65982 commit 78c3310
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 102 deletions.
10 changes: 3 additions & 7 deletions multicluster/cmd/multicluster-controller/leader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,9 @@ func runLeader(o *Options) error {
role: leaderRole},
})

clusterSetReconciler := &leader.LeaderClusterSetReconciler{
Client: mgrClient,
Scheme: mgrScheme,
StatusManager: memberClusterStatusManager,
ClusterCalimCRDAvailable: o.ClusterCalimCRDAvailable,
}
if err = clusterSetReconciler.SetupWithManager(mgr); err != nil {
clusterSetReconciler := leader.NewLeaderClusterSetReconciler(mgrClient, podNamespace,
o.ClusterCalimCRDAvailable, memberClusterStatusManager)
if err := clusterSetReconciler.SetupWithManager(mgr); err != nil {
return fmt.Errorf("error creating ClusterSet controller: %v", err)
}

Expand Down
63 changes: 38 additions & 25 deletions multicluster/controllers/multicluster/leader/clusterset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -46,15 +45,26 @@ var (
// Namespace, so a MC Controller will be handling only a single ClusterSet in the given Namespace.
type LeaderClusterSetReconciler struct {
client.Client
Scheme *runtime.Scheme
mutex sync.Mutex
ClusterCalimCRDAvailable bool
namespace string
clusterCalimCRDAvailable bool
statusManager MemberClusterStatusManager

clusterSetConfig *mcv1alpha2.ClusterSet
clusterSetID common.ClusterSetID
clusterID common.ClusterID
clusterSetID common.ClusterSetID
clusterID common.ClusterID
mutex sync.Mutex
}

StatusManager MemberClusterStatusManager
func NewLeaderClusterSetReconciler(client client.Client, namespace string,
clusterCalimCRDAvailable bool,
statusManager MemberClusterStatusManager) *LeaderClusterSetReconciler {
return &LeaderClusterSetReconciler{
Client: client,
namespace: namespace,
clusterCalimCRDAvailable: clusterCalimCRDAvailable,
statusManager: statusManager,
clusterID: common.InvalidClusterID,
clusterSetID: common.InvalidClusterSetID,
}
}

//+kubebuilder:rbac:groups=multicluster.crd.antrea.io,resources=clustersets,verbs=get;list;watch;create;update;patch;delete
Expand All @@ -72,10 +82,10 @@ func (r *LeaderClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{}, err
}
klog.InfoS("Received ClusterSet delete", "clusterset", req.NamespacedName)
if r.clusterSetConfig != nil && r.clusterSetConfig.Name != req.Name {
if r.clusterSetID != common.ClusterSetID(req.Name) {
// Not the current ClusterSet.
return ctrl.Result{}, nil
}
r.clusterSetConfig = nil
r.clusterID = common.InvalidClusterID
r.clusterSetID = common.InvalidClusterSetID
return ctrl.Result{}, nil
Expand All @@ -84,8 +94,8 @@ func (r *LeaderClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
klog.InfoS("Received ClusterSet add/update", "clusterset", klog.KObj(clusterSet))

// Handle create or update
if r.clusterSetConfig == nil {
r.clusterID, err = common.GetClusterID(r.ClusterCalimCRDAvailable, req, r.Client, clusterSet)
if r.clusterID == common.InvalidClusterID {
r.clusterID, err = common.GetClusterID(r.clusterCalimCRDAvailable, req, r.Client, clusterSet)
if err != nil {
return ctrl.Result{}, err
}
Expand All @@ -109,7 +119,6 @@ func (r *LeaderClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
}
}

r.clusterSetConfig = clusterSet.DeepCopy()
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -158,14 +167,27 @@ func (r *LeaderClusterSetReconciler) updateStatus() {
r.mutex.Lock()
defer r.mutex.Unlock()

if r.clusterSetConfig == nil {
if r.clusterID == common.InvalidClusterID {
// Nothing to do.
return
}

namespacedName := types.NamespacedName{
Namespace: r.namespace,
Name: string(r.clusterSetID),
}
clusterSet := &mcv1alpha2.ClusterSet{}
err := r.Get(context.TODO(), namespacedName, clusterSet)
if err != nil {
if !apierrors.IsNotFound(err) {
klog.ErrorS(err, "Failed to get ClusterSet", "name", namespacedName)
}
return
}

status := mcv1alpha2.ClusterSetStatus{}
status.ObservedGeneration = r.clusterSetConfig.Generation
clusterStatuses := r.StatusManager.GetMemberClusterStatuses()
status.ObservedGeneration = clusterSet.Generation
clusterStatuses := r.statusManager.GetMemberClusterStatuses()
status.ClusterStatuses = clusterStatuses
sizeOfMembers := len(clusterStatuses)
status.TotalClusters = int32(sizeOfMembers)
Expand Down Expand Up @@ -199,15 +221,6 @@ func (r *LeaderClusterSetReconciler) updateStatus() {
overallCondition.Message = "All clusters have an unknown status"
}

namespacedName := types.NamespacedName{
Namespace: r.clusterSetConfig.Namespace,
Name: r.clusterSetConfig.Name,
}
clusterSet := &mcv1alpha2.ClusterSet{}
err := r.Get(context.TODO(), namespacedName, clusterSet)
if err != nil {
klog.ErrorS(err, "Failed to read ClusterSet", "name", namespacedName)
}
status.Conditions = clusterSet.Status.Conditions
if (len(clusterSet.Status.Conditions) == 1 && clusterSet.Status.Conditions[0].Status != overallCondition.Status) ||
len(clusterSet.Status.Conditions) == 0 {
Expand Down
60 changes: 24 additions & 36 deletions multicluster/controllers/multicluster/leader/clusterset_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ var (
}
)

func createMockClients(t *testing.T, objects ...client.Object) (*runtime.Scheme, client.Client, *MockMemberClusterStatusManager) {
func createMockClients(t *testing.T, objects ...client.Object) (client.Client, *MockMemberClusterStatusManager) {
scheme := runtime.NewScheme()
mcv1alpha1.AddToScheme(scheme)
mcv1alpha2.AddToScheme(scheme)
Expand All @@ -90,16 +90,13 @@ func createMockClients(t *testing.T, objects ...client.Object) (*runtime.Scheme,

mockCtrl := gomock.NewController(t)
mockStatusManager := NewMockMemberClusterStatusManager(mockCtrl)
return scheme, fakeRemoteClient, mockStatusManager
return fakeRemoteClient, mockStatusManager
}

func TestLeaderClusterSetAdd(t *testing.T) {
scheme, fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := LeaderClusterSetReconciler{
Client: fakeRemoteClient,
Scheme: scheme,
StatusManager: mockStatusManager,
}
fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := NewLeaderClusterSetReconciler(
fakeRemoteClient, "mcs1", false, mockStatusManager)
req := ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: "mcs1",
Expand Down Expand Up @@ -135,13 +132,9 @@ func TestLeaderClusterSetAddWithoutClusterID(t *testing.T) {
},
Value: "leader1",
}
scheme, fakeRemoteClient, mockStatusManager := createMockClients(t, clusterSetWithoutClusterID, clusterClaim)
leaderClusterSetReconcilerUnderTest := LeaderClusterSetReconciler{
Client: fakeRemoteClient,
Scheme: scheme,
StatusManager: mockStatusManager,
ClusterCalimCRDAvailable: true,
}
fakeRemoteClient, mockStatusManager := createMockClients(t, clusterSetWithoutClusterID, clusterClaim)
leaderClusterSetReconcilerUnderTest := NewLeaderClusterSetReconciler(
fakeRemoteClient, "mcs1", true, mockStatusManager)
req := ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: "mcs1",
Expand All @@ -160,13 +153,11 @@ func TestLeaderClusterSetAddWithoutClusterID(t *testing.T) {
}

func TestLeaderClusterSetUpdate(t *testing.T) {
scheme, fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := LeaderClusterSetReconciler{
Client: fakeRemoteClient,
Scheme: scheme,
StatusManager: mockStatusManager,
clusterSetConfig: existingClusterSet.DeepCopy(),
}
fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := NewLeaderClusterSetReconciler(
fakeRemoteClient, "mcs1", false, mockStatusManager)
leaderClusterSetReconcilerUnderTest.clusterID = common.ClusterID(existingClusterSet.Spec.ClusterID)

clusterSet := &mcv1alpha2.ClusterSet{}
err := fakeRemoteClient.Get(context.TODO(), types.NamespacedName{Name: "clusterset1", Namespace: "mcs1"}, clusterSet)
assert.Equal(t, nil, err)
Expand All @@ -192,13 +183,12 @@ func TestLeaderClusterSetUpdate(t *testing.T) {
}

func TestLeaderClusterSetDelete(t *testing.T) {
scheme, fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := LeaderClusterSetReconciler{
Client: fakeRemoteClient,
Scheme: scheme,
StatusManager: mockStatusManager,
clusterSetConfig: existingClusterSet.DeepCopy(),
}
fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := NewLeaderClusterSetReconciler(
fakeRemoteClient, "mcs1", false, mockStatusManager)
leaderClusterSetReconcilerUnderTest.clusterID = common.ClusterID(existingClusterSet.Spec.ClusterID)
leaderClusterSetReconcilerUnderTest.clusterSetID = common.ClusterSetID(existingClusterSet.Name)

clusterSet := &mcv1alpha2.ClusterSet{}
err := fakeRemoteClient.Get(context.TODO(), types.NamespacedName{Name: "clusterset1", Namespace: "mcs1"}, clusterSet)
assert.Equal(t, nil, err)
Expand All @@ -219,13 +209,11 @@ func TestLeaderClusterSetDelete(t *testing.T) {
}

func TestLeaderClusterStatus(t *testing.T) {
scheme, fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := LeaderClusterSetReconciler{
Client: fakeRemoteClient,
Scheme: scheme,
StatusManager: mockStatusManager,
clusterSetConfig: existingClusterSet.DeepCopy(),
}
fakeRemoteClient, mockStatusManager := createMockClients(t, existingClusterSet)
leaderClusterSetReconcilerUnderTest := NewLeaderClusterSetReconciler(
fakeRemoteClient, "mcs1", false, mockStatusManager)
leaderClusterSetReconcilerUnderTest.clusterID = common.ClusterID(existingClusterSet.Spec.ClusterID)
leaderClusterSetReconcilerUnderTest.clusterSetID = common.ClusterSetID(existingClusterSet.Name)

mockStatusManager.EXPECT().GetMemberClusterStatuses().Return(statuses).Times(1)
leaderClusterSetReconcilerUnderTest.updateStatus()
Expand Down
60 changes: 29 additions & 31 deletions multicluster/controllers/multicluster/member/clusterset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,17 @@ var getRemoteConfigAndClient = commonarea.GetRemoteConfigAndClient
// MemberClusterSetReconciler reconciles a ClusterSet object in the member cluster deployment.
type MemberClusterSetReconciler struct {
client.Client
Scheme *runtime.Scheme
Namespace string
ClusterCalimCRDAvailable bool
scheme *runtime.Scheme
namespace string
clusterCalimCRDAvailable bool

// commonAreaLock protects the access to RemoteCommonArea.
commonAreaLock sync.RWMutex
commonAreaCreationCh chan struct{}

clusterSetConfig *mcv1alpha2.ClusterSet
clusterSetID common.ClusterSetID
clusterID common.ClusterID
installedLeader leaderClusterInfo
clusterSetID common.ClusterSetID
clusterID common.ClusterID
installedLeader leaderClusterInfo

remoteCommonArea commonarea.RemoteCommonArea
enableStretchedNetworkPolicy bool
Expand All @@ -77,10 +76,10 @@ func NewMemberClusterSetReconciler(client client.Client,
) *MemberClusterSetReconciler {
return &MemberClusterSetReconciler{
Client: client,
Scheme: scheme,
Namespace: namespace,
scheme: scheme,
namespace: namespace,
enableStretchedNetworkPolicy: enableStretchedNetworkPolicy,
ClusterCalimCRDAvailable: clusterCalimCRDAvailable,
clusterCalimCRDAvailable: clusterCalimCRDAvailable,
commonAreaCreationCh: commonAreaCreationCh,
clusterID: common.InvalidClusterID,
clusterSetID: common.InvalidClusterSetID,
Expand Down Expand Up @@ -129,7 +128,6 @@ func (r *MemberClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
clusterSetCreated = r.clusterID != common.ClusterID(clusterSet.Spec.ClusterID) || r.clusterSetID != common.ClusterSetID(clusterSet.Name)
leaderChanged := r.installedLeader.clusterID != newLeader.ClusterID || r.installedLeader.serverUrl != newLeader.Server ||
r.installedLeader.secretName != newLeader.Secret
r.clusterSetConfig = clusterSet.DeepCopy()

if !leaderChanged && !clusterSetCreated {
klog.V(2).InfoS("No change for leader cluster configuration")
Expand All @@ -144,7 +142,7 @@ func (r *MemberClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
return err
}

r.clusterID, err = common.GetClusterID(r.ClusterCalimCRDAvailable, req, r.Client, clusterSet)
r.clusterID, err = common.GetClusterID(r.clusterCalimCRDAvailable, req, r.Client, clusterSet)
if err != nil {
return err
}
Expand Down Expand Up @@ -193,11 +191,10 @@ func (r *MemberClusterSetReconciler) cleanUpResources(ctx context.Context) error
}
r.remoteCommonArea.Stop()
r.remoteCommonArea = nil
r.clusterSetConfig = nil
r.installedLeader = leaderClusterInfo{}
}

if r.clusterID != common.InvalidClusterID || r.clusterSetID != common.InvalidClusterSetID {
if r.clusterID != common.InvalidClusterID {
klog.InfoS("Clean up all resources created by Antrea Multi-cluster Controller")
if err := cleanUpResourcesCreatedByMC(ctx, r.Client); err != nil {
return err
Expand Down Expand Up @@ -261,14 +258,14 @@ func (r *MemberClusterSetReconciler) createRemoteCommonArea(clusterSet *mcv1alph
return err
}

config, remoteCommonAreaMgr, remoteClient, err := getRemoteConfigAndClient(secret, url, clusterID, clusterSet, r.Scheme)
config, remoteCommonAreaMgr, remoteClient, err := getRemoteConfigAndClient(secret, url, clusterID, clusterSet, r.scheme)
if err != nil {
return err
}

remoteNamespace := clusterSet.Spec.Namespace
r.remoteCommonArea, err = commonarea.NewRemoteCommonArea(clusterID, r.clusterSetID, r.clusterID,
remoteCommonAreaMgr, remoteClient, r.Scheme, r.Client, remoteNamespace, r.Namespace,
remoteCommonAreaMgr, remoteClient, r.scheme, r.Client, remoteNamespace, r.namespace,
config, r.enableStretchedNetworkPolicy)
if err != nil {
klog.ErrorS(err, "Unable to create RemoteCommonArea", "cluster", clusterID)
Expand All @@ -282,7 +279,7 @@ func (r *MemberClusterSetReconciler) createRemoteCommonArea(clusterSet *mcv1alph
remoteCommonAreaMgr.GetScheme(),
r.Client,
string(r.clusterID),
r.Namespace,
r.namespace,
r.remoteCommonArea,
)
r.remoteCommonArea.AddImportReconciler(resImportReconciler)
Expand Down Expand Up @@ -332,13 +329,26 @@ func (r *MemberClusterSetReconciler) getSecretForLeader(secretName string, secre
}

func (r *MemberClusterSetReconciler) updateStatus() {
if r.clusterSetConfig == nil {
if r.clusterID == common.InvalidClusterID {
// Nothing to do.
return
}

namespacedName := types.NamespacedName{
Namespace: r.namespace,
Name: string(r.clusterSetID),
}
clusterSet := &mcv1alpha2.ClusterSet{}
err := r.Get(context.TODO(), namespacedName, clusterSet)
if err != nil {
if !apierrors.IsNotFound(err) {
klog.ErrorS(err, "Failed to get ClusterSet", "name", namespacedName)
}
return
}

status := mcv1alpha2.ClusterSetStatus{}
status.ObservedGeneration = r.clusterSetConfig.Generation
status.ObservedGeneration = clusterSet.Generation
status.ClusterStatuses = []mcv1alpha2.ClusterStatus{}
r.commonAreaLock.RLock()
if r.remoteCommonArea != nil {
Expand Down Expand Up @@ -394,18 +404,6 @@ func (r *MemberClusterSetReconciler) updateStatus() {
status.TotalClusters = 1
status.ReadyClusters = int32(readyClusters)

namespacedName := types.NamespacedName{
Namespace: r.clusterSetConfig.Namespace,
Name: r.clusterSetConfig.Name,
}
clusterSet := &mcv1alpha2.ClusterSet{}
err := r.Get(context.TODO(), namespacedName, clusterSet)
if err != nil {
if !apierrors.IsNotFound(err) {
klog.ErrorS(err, "Failed to read ClusterSet", "name", namespacedName)
}
return
}
status.Conditions = clusterSet.Status.Conditions
if (len(clusterSet.Status.Conditions) == 1 && clusterSet.Status.Conditions[0].Status != overallCondition.Status) ||
len(clusterSet.Status.Conditions) == 0 {
Expand Down
Loading

0 comments on commit 78c3310

Please sign in to comment.