Skip to content

Commit

Permalink
[release-2.11] Make sure local cluster is always in the managedcluste…
Browse files Browse the repository at this point in the history 
…r list (#1641)

* Make sure local cluster is always in the managedcluster list (#1637)

* make sure local cluster is part of list all the time

Signed-off-by: Coleen Iona Quadros <[email protected]>

* fix tests

Signed-off-by: Coleen Iona Quadros <[email protected]>

* use sync map

Signed-off-by: Coleen Iona Quadros <[email protected]>

* lint

Signed-off-by: Coleen Iona Quadros <[email protected]>

---------

Signed-off-by: Coleen Iona Quadros <[email protected]>

* update

Signed-off-by: Coleen Iona Quadros <[email protected]>

* fix lock

Signed-off-by: Coleen Iona Quadros <[email protected]>

---------

Signed-off-by: Coleen Iona Quadros <[email protected]>
  • Loading branch information
@coleenquadros
coleenquadros authored Sep 30, 2024
1 parent a7f9a36 commit 3de380d
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 49 deletions.
40 changes: 24 additions & 16 deletions operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ var (
clusterAddon = &addonv1alpha1.ClusterManagementAddOn{}
defaultAddonDeploymentConfig = &addonv1alpha1.AddOnDeploymentConfig{}
isplacementControllerRunnning = false
managedClusterList = map[string]string{}
managedClusterList = sync.Map{}
managedClusterListMutex = &sync.RWMutex{}
installMetricsWithoutAddon = false
)
Expand Down Expand Up @@ -107,7 +107,7 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
if err != nil {
if k8serrors.IsNotFound(err) {
deleteAll = true
delete(managedClusterList, "local-cluster")
managedClusterList.Delete("local-cluster")
} else {
// Error reading the object - requeue the request.
return ctrl.Result{}, err
Expand All @@ -124,9 +124,21 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
// We want to ensure that the local-cluster is always in the managedClusterList
// In the case when hubSelfManagement is enabled, we will delete it from the list and modify the object
// to cater to the use case of deploying in open-cluster-management-observability namespace
if req.Name == "local-cluster" {
managedClusterList.Delete("local-cluster")
if _, ok := managedClusterList.Load("local-cluster"); !ok {
obj := &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "local-cluster",
Namespace: config.GetDefaultNamespace(),
Labels: map[string]string{
"openshiftVersion": "mimical",
},
},
}
installMetricsWithoutAddon = true
updateManagedClusterList(obj)
}

if !deleteAll && !mco.Spec.ObservabilityAddonSpec.EnableMetrics {
reqLogger.Info("EnableMetrics is set to false. Delete Observability addons")
deleteAll = true
Expand Down Expand Up @@ -368,7 +380,9 @@ func createAllRelatedRes(

failedCreateManagedClusterRes := false
managedClusterListMutex.RLock()
for managedCluster, openshiftVersion := range managedClusterList {
managedClusterList.Range(func(key, value interface{}) bool {
managedCluster := key.(string)
openshiftVersion := value.(string)
if isReconcileRequired(request, managedCluster) {
log.Info(
"Monitoring operator should be installed in cluster",
Expand Down Expand Up @@ -409,20 +423,20 @@ func createAllRelatedRes(
log.Error(err, "Failed to create managedcluster resources", "namespace", managedCluster)
}
if request.Namespace == managedCluster {
break
return false
}
}
}
return true
})

// Look through the obsAddonList items and find clusters
// which are no longer to be managed and therefore needs deletion
clustersToCleanup := []string{}
for _, ep := range obsAddonList.Items {
if _, ok := managedClusterList[ep.Namespace]; !ok {
if _, ok := managedClusterList.Load(ep.Namespace); !ok {
clustersToCleanup = append(clustersToCleanup, ep.Namespace)
}
}

managedClusterListMutex.RUnlock()

failedDeleteOba := false
Expand Down Expand Up @@ -589,16 +603,10 @@ func areManagedClusterLabelsReady(obj client.Object) bool {
func updateManagedClusterList(obj client.Object) {
managedClusterListMutex.Lock()
defer managedClusterListMutex.Unlock()
//ACM 8509: Special case for local-cluster, we deploy endpoint and metrics collector in the hub
//whether hubSelfManagement is enabled or not
if obj.GetName() == localClusterName {
managedClusterList[obj.GetName()] = "mimical"
return
}
if version, ok := obj.GetLabels()["openshiftVersion"]; ok {
managedClusterList[obj.GetName()] = version
managedClusterList.Store(obj.GetName(), version)
} else {
managedClusterList[obj.GetName()] = nonOCP
managedClusterList.Store(obj.GetName(), nonOCP)
}
}

Expand Down
13 changes: 8 additions & 5 deletions ...tors/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,9 @@ func TestObservabilityAddonController(t *testing.T) {
},
}

managedClusterList = map[string]string{
namespace: "4",
namespace2: "4",
}
managedClusterList.Store(namespace, "4")
managedClusterList.Store(namespace2, "4")

_, err := r.Reconcile(context.TODO(), req)
if err != nil {
t.Fatalf("reconcile: (%v)", err)
Expand All @@ -228,7 +227,11 @@ func TestObservabilityAddonController(t *testing.T) {
t.Fatalf("Deprecated role not removed")
}

managedClusterList = map[string]string{namespace: "4"}
managedClusterList.Range(func(key, value interface{}) bool {
managedClusterList.Delete(key)
return true
})
managedClusterList.Store(namespace, "4")
_, err = r.Reconcile(context.TODO(), req)
if err != nil {
t.Fatalf("reconcile: (%v)", err)
Expand Down
26 changes: 18 additions & 8 deletions operators/multiclusterobservability/controllers/placementrule/predicate_func.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"reflect"
"strings"

clusterv1 "open-cluster-management.io/api/cluster/v1"

"github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config"
operatorconfig "github.com/stolostron/multicluster-observability-operator/operators/pkg/config"

Expand All @@ -31,7 +33,9 @@ func getClusterPreds() predicate.Funcs {
if !areManagedClusterLabelsReady(e.Object) {
return false
}
updateManagedClusterList(e.Object)
if e.Object.GetName() != localClusterName {
updateManagedClusterList(e.Object)
}

return true
}
Expand All @@ -48,9 +52,7 @@ func getClusterPreds() predicate.Funcs {

if e.ObjectNew.GetDeletionTimestamp() != nil {
log.Info("managedcluster is in terminating state", "managedCluster", e.ObjectNew.GetName())
managedClusterListMutex.Lock()
delete(managedClusterList, e.ObjectNew.GetName())
managedClusterListMutex.Unlock()
managedClusterList.Delete(e.ObjectNew.GetName())
managedClusterImageRegistryMutex.Lock()
delete(managedClusterImageRegistry, e.ObjectNew.GetName())
managedClusterImageRegistryMutex.Unlock()
Expand All @@ -59,7 +61,15 @@ func getClusterPreds() predicate.Funcs {
if !areManagedClusterLabelsReady(e.ObjectNew) {
return false
}
updateManagedClusterList(e.ObjectNew)
if e.ObjectNew.GetName() != localClusterName {
updateManagedClusterList(e.ObjectNew)
}

}
//log the diff in managedccluster object
if !reflect.DeepEqual(e.ObjectNew.(*clusterv1.ManagedCluster), e.ObjectOld.(*clusterv1.ManagedCluster)) {
log.Info("managedcluster object New diff", "managedCluster", e.ObjectNew.GetName(), "diff", fmt.Sprintf("%+v", e.ObjectNew.(*clusterv1.ManagedCluster)))
log.Info("managedcluster object Old diff", "managedCluster", e.ObjectOld.GetName(), "diff", fmt.Sprintf("%+v", e.ObjectOld.(*clusterv1.ManagedCluster)))
}

return true
Expand All @@ -72,9 +82,9 @@ func getClusterPreds() predicate.Funcs {
return false
}

managedClusterListMutex.Lock()
delete(managedClusterList, e.Object.GetName())
managedClusterListMutex.Unlock()
if e.Object.GetName() != localClusterName {
managedClusterList.Delete(e.Object.GetName())
}
managedClusterImageRegistryMutex.Lock()
delete(managedClusterImageRegistry, e.Object.GetName())
managedClusterImageRegistryMutex.Unlock()
Expand Down
42 changes: 22 additions & 20 deletions operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (
"testing"
"time"

clusterv1 "open-cluster-management.io/api/cluster/v1"

addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"

appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/event"
)
Expand Down Expand Up @@ -66,15 +67,16 @@ func TestClusterPred(t *testing.T) {
t.Run(c.caseName, func(t *testing.T) {
pred := getClusterPreds()
create_event := event.CreateEvent{
Object: &appsv1.Deployment{
Object: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
Annotations: c.annotations,
Labels: map[string]string{"vendor": "OpenShift", "openshiftVersion": "4.6.0"},
Name: name,
Namespace: c.namespace,
Annotations: c.annotations,
Labels: map[string]string{"vendor": "OpenShift", "openshiftVersion": "4.6.0"},
ResourceVersion: "1",
},
Spec: appsv1.DeploymentSpec{
Replicas: int32Ptr(2),
Spec: clusterv1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
}
Expand All @@ -90,7 +92,7 @@ func TestClusterPred(t *testing.T) {
}

update_event := event.UpdateEvent{
ObjectNew: &appsv1.Deployment{
ObjectNew: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
Expand All @@ -100,7 +102,7 @@ func TestClusterPred(t *testing.T) {
Labels: map[string]string{"vendor": "OpenShift", "openshiftVersion": "4.6.0"},
},
},
ObjectOld: &appsv1.Deployment{
ObjectOld: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
Expand All @@ -125,14 +127,14 @@ func TestClusterPred(t *testing.T) {
}

delete_event := event.DeleteEvent{
Object: &appsv1.Deployment{
Object: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
Annotations: c.annotations,
},
Spec: appsv1.DeploymentSpec{
Replicas: int32Ptr(2),
Spec: clusterv1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
}
Expand Down Expand Up @@ -196,19 +198,18 @@ func TestManagedClusterLabelReady(t *testing.T) {
t.Run(c.caseName, func(t *testing.T) {
pred := getClusterPreds()
create_event := event.CreateEvent{
Object: &appsv1.Deployment{
Object: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
Annotations: c.annotations,
Labels: c.labels,
},
Spec: appsv1.DeploymentSpec{
Replicas: int32Ptr(2),
Spec: clusterv1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
}

if c.expectedCreate {
if !pred.CreateFunc(create_event) {
t.Fatalf("pre func return false on applied createevent in case: (%v)", c.caseName)
Expand All @@ -218,8 +219,9 @@ func TestManagedClusterLabelReady(t *testing.T) {
t.Fatalf("pre func return true on non-applied createevent in case: (%v)", c.caseName)
}
}

update_event := event.UpdateEvent{
ObjectNew: &appsv1.Deployment{
ObjectNew: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
Expand All @@ -229,15 +231,15 @@ func TestManagedClusterLabelReady(t *testing.T) {
Labels: c.labels,
},
},
ObjectOld: &appsv1.Deployment{

ObjectOld: &clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: c.namespace,
ResourceVersion: "1",
},
},
}

if c.expectedUpdate {
if !pred.UpdateFunc(update_event) {
t.Fatalf("pre func return false on applied update event in case: (%v)", c.caseName)
Expand Down

0 comments on commit 3de380d

Please sign in to comment.