From 6a000ae0953fe055c6a6a8f50721254b151bfed9 Mon Sep 17 00:00:00 2001 From: raaizik <132667934+raaizik@users.noreply.github.com> Date: Wed, 24 Jul 2024 15:49:47 +0300 Subject: [PATCH 1/2] Available CRDs check feature Reasons for this enhancement: - A controller cannot set up a watch for a CRD that is not installed on the cluster, trying to set up a watch will panic the operator - There is no known way, that we are aware of, to add a watch later without client cache issue How does the enhancement work around the issue: - A new controller to watch creation/deletion for the CRDs of interest to prevent unnecessary reconciles - On start of the operator(main), detect which CRDs are avail (out of a fixed list) - At the start each reconcile of new controller, we fetch the CRDs available again and compare it with CRDs fetched in previous step, If there is any change, we panic the op Signed-off-by: raaizik <132667934+raaizik@users.noreply.github.com> Co-Authored-By: Rewant Soni --- .../ocsinitialization_controller.go | 105 ++++++++++-------- .../initialization_reconciler_test.go | 38 +++++-- controllers/storagecluster/reconcile.go | 21 +++- controllers/storagecluster/storageclasses.go | 5 +- .../storagecluster/storageclasses_test.go | 2 +- .../storagecluster_controller.go | 20 ++-- .../storagecluster_controller_test.go | 3 + controllers/util/k8sutil.go | 21 +--- controllers/util/predicates.go | 33 ++++++ controllers/util/util.go | 9 +- main.go | 43 +++++-- 11 files changed, 200 insertions(+), 100 deletions(-) diff --git a/controllers/ocsinitialization/ocsinitialization_controller.go b/controllers/ocsinitialization/ocsinitialization_controller.go index 150920bf1d..087e43caa2 100644 --- a/controllers/ocsinitialization/ocsinitialization_controller.go +++ b/controllers/ocsinitialization/ocsinitialization_controller.go @@ -8,20 +8,22 @@ import ( "strconv" "strings" - "github.com/go-logr/logr" - secv1client "github.com/openshift/client-go/security/clientset/versioned/typed/security/v1" - opv1a1 "github.com/operator-framework/api/pkg/operators/v1alpha1" - promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1" "github.com/red-hat-storage/ocs-operator/v4/controllers/defaults" "github.com/red-hat-storage/ocs-operator/v4/controllers/platform" "github.com/red-hat-storage/ocs-operator/v4/controllers/storagecluster" "github.com/red-hat-storage/ocs-operator/v4/controllers/util" "github.com/red-hat-storage/ocs-operator/v4/templates" + + "github.com/go-logr/logr" + secv1client "github.com/openshift/client-go/security/clientset/versioned/typed/security/v1" + opv1a1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" rookCephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" + extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -43,8 +45,8 @@ import ( var operatorNamespace string const ( - wrongNamespacedName = "Ignoring this resource. Only one should exist, and this one has the wrong name and/or namespace." - random30CharacterString = "KP7TThmSTZegSGmHuPKLnSaaAHSG3RSgqw6akBj0oVk" + random30CharacterString = "KP7TThmSTZegSGmHuPKLnSaaAHSG3RSgqw6akBj0oVk" + PrometheusOperatorDeploymentName = "prometheus-operator" PrometheusOperatorCSVNamePrefix = "odf-prometheus-operator" ClusterClaimCrdName = "clusterclaims.cluster.open-cluster-management.io" @@ -63,13 +65,14 @@ func InitNamespacedName() types.NamespacedName { // nolint:revive type OCSInitializationReconciler struct { client.Client - ctx context.Context + ctx context.Context + clusters *util.Clusters + Log logr.Logger Scheme *runtime.Scheme SecurityClient secv1client.SecurityV1Interface OperatorNamespace string - clusters *util.Clusters - availableCrds map[string]bool + AvailableCrds map[string]bool } // +kubebuilder:rbac:groups=ocs.openshift.io,resources=*,verbs=get;list;watch;create;update;patch;delete @@ -93,11 +96,26 @@ func (r *OCSInitializationReconciler) Reconcile(ctx context.Context, request rec r.Log.Info("Reconciling OCSInitialization.", "OCSInitialization", klog.KRef(request.Namespace, request.Name)) + crd := &metav1.PartialObjectMetadata{} + crd.SetGroupVersionKind(extv1.SchemeGroupVersion.WithKind("CustomResourceDefinition")) + crd.Name = ClusterClaimCrdName + if err := r.Client.Get(ctx, client.ObjectKeyFromObject(crd), crd); client.IgnoreNotFound(err) != nil { + r.Log.Error(err, "Failed to get CRD", "CRD", ClusterClaimCrdName) + return reconcile.Result{}, err + } + util.AssertEqual(r.AvailableCrds[ClusterClaimCrdName], crd.UID != "", util.ExitCodeThatShouldRestartTheProcess) + initNamespacedName := InitNamespacedName() instance := &ocsv1.OCSInitialization{} if initNamespacedName.Name != request.Name || initNamespacedName.Namespace != request.Namespace { // Ignoring this resource because it has the wrong name or namespace - r.Log.Info(wrongNamespacedName) + r.Log.Info( + "Ignoring this resource. Only one OCSInitialization should exist.", + "Expected", + initNamespacedName, + "Got", + request.NamespacedName, + ) err := r.Client.Get(ctx, request.NamespacedName, instance) if err != nil { // the resource probably got deleted @@ -147,11 +165,6 @@ func (r *OCSInitializationReconciler) Reconcile(ctx context.Context, request rec } } - r.availableCrds, err = util.MapCRDAvailability(r.ctx, r.Client, r.Log, ClusterClaimCrdName) - if err != nil { - return reconcile.Result{}, err - } - r.clusters, err = util.GetClusters(ctx, r.Client) if err != nil { r.Log.Error(err, "Failed to get clusters") @@ -179,7 +192,7 @@ func (r *OCSInitializationReconciler) Reconcile(ctx context.Context, request rec return reconcile.Result{}, err } - if r.availableCrds[ClusterClaimCrdName] { + if r.AvailableCrds[ClusterClaimCrdName] { err = r.ensureClusterClaimExists() if err != nil { r.Log.Error(err, "Failed to ensure odf-info namespacedname ClusterClaim") @@ -271,6 +284,14 @@ func (r *OCSInitializationReconciler) SetupWithManager(mgr ctrl.Manager) error { }, ) + enqueueOCSInit := handler.EnqueueRequestsFromMapFunc( + func(context context.Context, obj client.Object) []reconcile.Request { + return []reconcile.Request{{ + NamespacedName: InitNamespacedName(), + }} + }, + ) + ocsInitializationController := ctrl.NewControllerManagedBy(mgr). For(&ocsv1.OCSInitialization{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&corev1.Service{}). @@ -283,13 +304,7 @@ func (r *OCSInitializationReconciler) SetupWithManager(mgr ctrl.Manager) error { // ocs-operator-config configmap if storagecluster spec changes Watches( &ocsv1.StorageCluster{}, - handler.EnqueueRequestsFromMapFunc( - func(context context.Context, obj client.Object) []reconcile.Request { - return []reconcile.Request{{ - NamespacedName: InitNamespacedName(), - }} - }, - ), + enqueueOCSInit, builder.WithPredicates(predicate.GenerationChangedPredicate{}), ). // Watcher for storageClass required to update values related to replica-1 @@ -317,13 +332,7 @@ func (r *OCSInitializationReconciler) SetupWithManager(mgr ctrl.Manager) error { Namespace: r.OperatorNamespace, }, }, - handler.EnqueueRequestsFromMapFunc( - func(context context.Context, obj client.Object) []reconcile.Request { - return []reconcile.Request{{ - NamespacedName: InitNamespacedName(), - }} - }, - ), + enqueueOCSInit, ). // Watcher for ocs-operator-config cm Watches( @@ -333,26 +342,34 @@ func (r *OCSInitializationReconciler) SetupWithManager(mgr ctrl.Manager) error { Namespace: r.OperatorNamespace, }, }, - handler.EnqueueRequestsFromMapFunc( - func(context context.Context, obj client.Object) []reconcile.Request { - return []reconcile.Request{{ - NamespacedName: InitNamespacedName(), - }} - }, - ), + enqueueOCSInit, ). // Watcher for prometheus operator csv Watches( &opv1a1.ClusterServiceVersion{}, - handler.EnqueueRequestsFromMapFunc( - func(context context.Context, obj client.Object) []reconcile.Request { - return []reconcile.Request{{ - NamespacedName: InitNamespacedName(), - }} - }, - ), + enqueueOCSInit, builder.WithPredicates(prometheusPredicate), + ). + Watches( + &extv1.CustomResourceDefinition{}, + enqueueOCSInit, + builder.WithPredicates( + util.NamePredicate(ClusterClaimCrdName), + util.CrdCreateAndDeletePredicate(&r.Log, ClusterClaimCrdName, r.AvailableCrds[ClusterClaimCrdName]), + ), + builder.OnlyMetadata, ) + + if r.AvailableCrds[ClusterClaimCrdName] { + ocsInitializationController = ocsInitializationController.Watches( + &v1alpha1.ClusterClaim{}, + enqueueOCSInit, + builder.WithPredicates( + util.NamePredicate(util.OdfInfoNamespacedNameClaimName), + predicate.GenerationChangedPredicate{}, + ), + ) + } return ocsInitializationController.Complete(r) } diff --git a/controllers/storagecluster/initialization_reconciler_test.go b/controllers/storagecluster/initialization_reconciler_test.go index bfb444ded2..77bf31fc85 100644 --- a/controllers/storagecluster/initialization_reconciler_test.go +++ b/controllers/storagecluster/initialization_reconciler_test.go @@ -6,18 +6,18 @@ import ( "os" "testing" - "github.com/blang/semver/v4" - oprverion "github.com/operator-framework/api/pkg/lib/version" - opv1a1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + api "github.com/red-hat-storage/ocs-operator/api/v4/v1" + "github.com/red-hat-storage/ocs-operator/v4/controllers/platform" + "github.com/red-hat-storage/ocs-operator/v4/controllers/util" ocsversion "github.com/red-hat-storage/ocs-operator/v4/version" + "github.com/blang/semver/v4" "github.com/imdario/mergo" nbv1 "github.com/noobaa/noobaa-operator/v5/pkg/apis/noobaa/v1alpha1" configv1 "github.com/openshift/api/config/v1" routev1 "github.com/openshift/api/route/v1" - api "github.com/red-hat-storage/ocs-operator/api/v4/v1" - "github.com/red-hat-storage/ocs-operator/v4/controllers/platform" - "github.com/red-hat-storage/ocs-operator/v4/controllers/util" + oprverion "github.com/operator-framework/api/pkg/lib/version" + opv1a1 "github.com/operator-framework/api/pkg/operators/v1alpha1" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" @@ -255,7 +255,6 @@ func initStorageClusterResourceCreateUpdateTest(t *testing.T, runtimeObjs []clie } rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, tbd) } - reconciler := createFakeInitializationStorageClusterReconciler( t, rtObjsToCreateReconciler...) @@ -271,7 +270,6 @@ func initStorageClusterResourceCreateUpdateTest(t *testing.T, runtimeObjs []clie assert.Equal(t, reconcile.Result{}, result) err = os.Setenv("WATCH_NAMESPACE", cr.Namespace) assert.NoError(t, err) - return t, reconciler, cr, requestOCSInit } @@ -395,13 +393,35 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti } } - runtimeObjects = append(runtimeObjects, mockNodeList.DeepCopy(), cbp, cfs, cnfs, cnfsbp, cnfssvc, infrastructure, networkConfig, rookCephMonSecret, csv, workerNode, ocsProviderServiceSecret, ocsProviderServiceDeployment, ocsProviderService) + runtimeObjects = append( + runtimeObjects, + mockNodeList.DeepCopy(), + cbp, + cfs, + cnfs, + cnfsbp, + cnfssvc, + infrastructure, + networkConfig, + rookCephMonSecret, + csv, + workerNode, + ocsProviderServiceSecret, + ocsProviderServiceDeployment, + ocsProviderService, + createVirtualMachineCRD(), + ) client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(runtimeObjects...).WithStatusSubresource(statusSubresourceObjs...).Build() + availCrds := map[string]bool{ + VirtualMachineCrdName: true, + } + return StorageClusterReconciler{ Client: client, Scheme: scheme, OperatorCondition: newStubOperatorCondition(), Log: logf.Log.WithName("controller_storagecluster_test"), + AvailableCrds: availCrds, } } diff --git a/controllers/storagecluster/reconcile.go b/controllers/storagecluster/reconcile.go index 8196fe4fd5..bf888a92d0 100644 --- a/controllers/storagecluster/reconcile.go +++ b/controllers/storagecluster/reconcile.go @@ -7,16 +7,18 @@ import ( "strings" "time" - "github.com/blang/semver/v4" - "github.com/go-logr/logr" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - "github.com/operator-framework/operator-lib/conditions" ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1" ocsv1alpha1 "github.com/red-hat-storage/ocs-operator/api/v4/v1alpha1" "github.com/red-hat-storage/ocs-operator/v4/controllers/util" statusutil "github.com/red-hat-storage/ocs-operator/v4/controllers/util" "github.com/red-hat-storage/ocs-operator/v4/version" + + "github.com/blang/semver/v4" + "github.com/go-logr/logr" + conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" + "github.com/operator-framework/operator-lib/conditions" corev1 "k8s.io/api/core/v1" + extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" @@ -70,6 +72,8 @@ const ( // EBS represents AWS EBS provisioner for StorageClass EBS StorageClassProvisionerType = "kubernetes.io/aws-ebs" + + VirtualMachineCrdName = "virtualmachines.kubevirt.io" ) var storageClusterFinalizer = "storagecluster.ocs.openshift.io" @@ -143,6 +147,15 @@ func (r *StorageClusterReconciler) Reconcile(ctx context.Context, request reconc r.Log = r.Log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) r.ctx = ctrllog.IntoContext(ctx, r.Log) + crd := &metav1.PartialObjectMetadata{} + crd.SetGroupVersionKind(extv1.SchemeGroupVersion.WithKind("CustomResourceDefinition")) + crd.Name = VirtualMachineCrdName + if err := r.Client.Get(ctx, client.ObjectKeyFromObject(crd), crd); client.IgnoreNotFound(err) != nil { + r.Log.Error(err, "Failed to get CRD", "CRD", VirtualMachineCrdName) + return reconcile.Result{}, err + } + util.AssertEqual(r.AvailableCrds[VirtualMachineCrdName], crd.UID != "", util.ExitCodeThatShouldRestartTheProcess) + // Fetch the StorageCluster instance sc := &ocsv1.StorageCluster{} if err := r.Client.Get(ctx, request.NamespacedName, sc); err != nil { diff --git a/controllers/storagecluster/storageclasses.go b/controllers/storagecluster/storageclasses.go index 4f5a98681c..8fedd34b52 100644 --- a/controllers/storagecluster/storageclasses.go +++ b/controllers/storagecluster/storageclasses.go @@ -13,7 +13,6 @@ import ( cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" - extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -452,9 +451,7 @@ func (r *StorageClusterReconciler) newStorageClassConfigurations(initData *ocsv1 // when allowing consumers, creation of storage classes should only be done via storagerequests if !initData.Spec.AllowRemoteStorageConsumers { // If kubevirt crd is present, we create a specialized rbd storageclass for virtualization environment - kvcrd := &extv1.CustomResourceDefinition{} - err := r.Client.Get(context.TODO(), types.NamespacedName{Name: "virtualmachines.kubevirt.io", Namespace: ""}, kvcrd) - if err == nil { + if r.AvailableCrds[VirtualMachineCrdName] { ret = append(ret, newCephBlockPoolVirtualizationStorageClassConfiguration(initData)) } } diff --git a/controllers/storagecluster/storageclasses_test.go b/controllers/storagecluster/storageclasses_test.go index 2bc9fb0516..c05c58111b 100644 --- a/controllers/storagecluster/storageclasses_test.go +++ b/controllers/storagecluster/storageclasses_test.go @@ -62,6 +62,7 @@ var ( }, ObjectMeta: metav1.ObjectMeta{ Name: pluralName + "." + "kubevirt.io", + UID: "uid", }, Spec: extv1.CustomResourceDefinitionSpec{ Group: "kubevirt.io", @@ -99,7 +100,6 @@ func TestCustomEncryptedStorageClasses(t *testing.T) { func testStorageClasses(t *testing.T, pvEncryption bool, customSpec *api.StorageClusterSpec) { runtimeObjs := []client.Object{} - runtimeObjs = append(runtimeObjs, createVirtualMachineCRD()) if pvEncryption { runtimeObjs = append(runtimeObjs, createDummyKMSConfigMap(dummyKmsProvider, dummyKmsAddress, "")) } diff --git a/controllers/storagecluster/storagecluster_controller.go b/controllers/storagecluster/storagecluster_controller.go index 11c6564969..9b9f157535 100644 --- a/controllers/storagecluster/storagecluster_controller.go +++ b/controllers/storagecluster/storagecluster_controller.go @@ -23,7 +23,6 @@ import ( corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -96,6 +95,7 @@ type StorageClusterReconciler struct { IsMultipleStorageClusters bool clusters *util.Clusters OperatorNamespace string + AvailableCrds map[string]bool } // SetupWithManager sets up a controller with manager @@ -228,18 +228,20 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.Secret{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&routev1.Route{}). Owns(&templatev1.Template{}). + // Using builder.OnlyMetadata as we are only interested in the presence and not getting this resource anywhere + Watches( + &extv1.CustomResourceDefinition{}, + enqueueStorageClusterRequest, + builder.WithPredicates( + util.NamePredicate(VirtualMachineCrdName), + util.CrdCreateAndDeletePredicate(&r.Log, VirtualMachineCrdName, r.AvailableCrds[VirtualMachineCrdName]), + ), + builder.OnlyMetadata, + ). Watches(&storagev1.StorageClass{}, enqueueStorageClusterRequest). Watches(&volumesnapshotv1.VolumeSnapshotClass{}, enqueueStorageClusterRequest). Watches(&ocsclientv1a1.StorageClient{}, enqueueStorageClusterRequest). Watches(&ocsv1.StorageProfile{}, enqueueStorageClusterRequest). - Watches( - &extv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "virtualmachines.kubevirt.io", - }, - }, - enqueueStorageClusterRequest, - ). Watches(&ocsv1alpha1.StorageConsumer{}, enqueueStorageClusterRequest, builder.WithPredicates(storageConsumerStatusPredicate)) if os.Getenv("SKIP_NOOBAA_CRD_WATCH") != "true" { diff --git a/controllers/storagecluster/storagecluster_controller_test.go b/controllers/storagecluster/storagecluster_controller_test.go index d05307dda8..0a8283a1e6 100644 --- a/controllers/storagecluster/storagecluster_controller_test.go +++ b/controllers/storagecluster/storagecluster_controller_test.go @@ -1208,6 +1208,8 @@ func createFakeStorageClusterReconciler(t *testing.T, obj ...runtime.Object) Sto frecorder := record.NewFakeRecorder(1024) reporter := statusutil.NewEventReporter(frecorder) + availCrds := map[string]bool{} + return StorageClusterReconciler{ recorder: reporter, Client: client, @@ -1216,6 +1218,7 @@ func createFakeStorageClusterReconciler(t *testing.T, obj ...runtime.Object) Sto Log: logf.Log.WithName("controller_storagecluster_test"), clusters: clusters, OperatorNamespace: operatorNamespace, + AvailableCrds: availCrds, } } diff --git a/controllers/util/k8sutil.go b/controllers/util/k8sutil.go index 45c01e37ea..fb1e5504ea 100644 --- a/controllers/util/k8sutil.go +++ b/controllers/util/k8sutil.go @@ -6,12 +6,12 @@ import ( "os" "strings" + ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1" + "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" - ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -47,7 +47,8 @@ const ( // This is the name for the OwnerUID FieldIndex OwnerUIDIndexName = "ownerUID" - OdfInfoNamespacedNameClaimName = "odfinfo.odf.openshift.io" + OdfInfoNamespacedNameClaimName = "odfinfo.odf.openshift.io" + ExitCodeThatShouldRestartTheProcess = 42 ) var podNamespace = os.Getenv(PodNamespaceEnvVar) @@ -165,17 +166,3 @@ func GenerateNameForNonResilientCephBlockPoolSC(initData *ocsv1.StorageCluster) } return fmt.Sprintf("%s-ceph-non-resilient-rbd", initData.Name) } - -func MapCRDAvailability(ctx context.Context, clnt client.Client, log logr.Logger, crdNames ...string) (map[string]bool, error) { - crdExist := map[string]bool{} - for _, crdName := range crdNames { - crd := &apiextensionsv1.CustomResourceDefinition{} - crd.Name = crdName - if err := clnt.Get(ctx, client.ObjectKeyFromObject(crd), crd); client.IgnoreNotFound(err) != nil { - log.Error(err, fmt.Sprintf("Error getting CRD for %s", crdName)) - return nil, fmt.Errorf("error getting CRD, %v", err) - } - crdExist[crdName] = crd.UID != "" - } - return crdExist, nil -} diff --git a/controllers/util/predicates.go b/controllers/util/predicates.go index fa710922f2..804647b7b3 100644 --- a/controllers/util/predicates.go +++ b/controllers/util/predicates.go @@ -1,7 +1,9 @@ package util import ( + "github.com/go-logr/logr" "reflect" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -68,3 +70,34 @@ func (p MetadataChangedPredicate) Update(e event.UpdateEvent) bool { return metaChanged } + +// Name Predicate return a predicate the filter events produced +// by resources that matches the given name +func NamePredicate(name string) predicate.Predicate { + return predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetName() == name + }) +} + +func CrdCreateAndDeletePredicate(log *logr.Logger, crdName string, crdExists bool) predicate.Predicate { + return predicate.Funcs{ + CreateFunc: func(_ event.CreateEvent) bool { + if !crdExists { + log.Info("CustomResourceDefinition %s was Created.", crdName) + } + return !crdExists + }, + DeleteFunc: func(_ event.DeleteEvent) bool { + if crdExists { + log.Info("CustomResourceDefinition %s was Deleted.", crdName) + } + return crdExists + }, + UpdateFunc: func(_ event.UpdateEvent) bool { + return false + }, + GenericFunc: func(_ event.GenericEvent) bool { + return false + }, + } +} diff --git a/controllers/util/util.go b/controllers/util/util.go index 79e84c3022..4ed20767b7 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -5,8 +5,9 @@ import ( "encoding/hex" "encoding/json" "fmt" - ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1" + "os" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -96,3 +97,9 @@ func CalculateMD5Hash(value any) string { hash := md5.Sum(data) return hex.EncodeToString(hash[:]) } + +func AssertEqual[T comparable](actual T, expected T, exitCode int) { + if actual != expected { + os.Exit(exitCode) + } +} diff --git a/main.go b/main.go index aaa4154f57..37ea4d8d9e 100644 --- a/main.go +++ b/main.go @@ -168,6 +168,22 @@ func main() { setupLog.Error(err, "Unable to get OperatorCondition") os.Exit(1) } + // apiclient.New() returns a client without cache. + // cache is not initialized before mgr.Start() + // we need this because we need to interact with OperatorCondition + apiClient, err := apiclient.New(mgr.GetConfig(), apiclient.Options{ + Scheme: mgr.GetScheme(), + }) + if err != nil { + setupLog.Error(err, "Unable to get Client") + os.Exit(1) + } + + availCrds, err := getAvailableCRDNames(context.Background(), apiClient) + if err != nil { + setupLog.Error(err, "Unable get a list of available CRD names") + os.Exit(1) + } if err = (&ocsinitialization.OCSInitializationReconciler{ Client: mgr.GetClient(), @@ -175,6 +191,7 @@ func main() { Scheme: mgr.GetScheme(), SecurityClient: secv1client.NewForConfigOrDie(mgr.GetConfig()), OperatorNamespace: operatorNamespace, + AvailableCrds: availCrds, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "OCSInitialization") os.Exit(1) @@ -186,6 +203,7 @@ func main() { Scheme: mgr.GetScheme(), OperatorNamespace: operatorNamespace, OperatorCondition: condition, + AvailableCrds: availCrds, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "StorageCluster") os.Exit(1) @@ -243,17 +261,6 @@ func main() { os.Exit(1) } - // apiclient.New() returns a client without cache. - // cache is not initialized before mgr.Start() - // we need this because we need to interact with OperatorCondition - apiClient, err := apiclient.New(mgr.GetConfig(), apiclient.Options{ - Scheme: mgr.GetScheme(), - }) - if err != nil { - setupLog.Error(err, "Unable to get Client") - os.Exit(1) - } - // Set OperatorCondition Upgradeable to True // We have to at least default the condition to True or // OLM will use the Readiness condition via our readiness probe instead: @@ -282,3 +289,17 @@ func main() { os.Exit(1) } } + +func getAvailableCRDNames(ctx context.Context, cl apiclient.Client) (map[string]bool, error) { + crdExist := map[string]bool{} + crdList := &metav1.PartialObjectMetadataList{} + crdList.SetGroupVersionKind(extv1.SchemeGroupVersion.WithKind("CustomResourceDefinitionList")) + if err := cl.List(ctx, crdList); err != nil { + return nil, fmt.Errorf("error listing CRDs, %v", err) + } + // Iterate over the list and populate the map + for i := range crdList.Items { + crdExist[crdList.Items[i].Name] = true + } + return crdExist, nil +} From bdc45f8054e004d7130b020bc862822b6b6cb252 Mon Sep 17 00:00:00 2001 From: raaizik <132667934+raaizik@users.noreply.github.com> Date: Thu, 8 Aug 2024 17:23:07 +0300 Subject: [PATCH 2/2] Available CRDs check feature w script Adds a script that bypasses pod restarts Signed-off-by: raaizik <132667934+raaizik@users.noreply.github.com> Co-Authored-By: Rewant Soni --- Dockerfile | 5 +++-- config/manager/manager.yaml | 2 +- deploy/csv-templates/ocs-operator.csv.yaml.in | 2 +- .../manifests/ocs-operator.clusterserviceversion.yaml | 2 +- hack/entrypoint.sh | 11 +++++++++++ 5 files changed, 17 insertions(+), 5 deletions(-) create mode 100755 hack/entrypoint.sh diff --git a/Dockerfile b/Dockerfile index d43476fd75..7604a4474a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,9 +24,10 @@ COPY --from=builder workspace/provider-api /usr/local/bin/provider-api COPY --from=builder workspace/onboarding-validation-keys-gen /usr/local/bin/onboarding-validation-keys-gen COPY --from=builder workspace/metrics/deploy/*rules*.yaml /ocs-prometheus-rules/ COPY --from=builder workspace/ux-backend-server /usr/local/bin/ux-backend-server +COPY --from=builder workspace/hack/entrypoint.sh /usr/local/bin/entrypoint -RUN chmod +x /usr/local/bin/ocs-operator /usr/local/bin/provider-api +RUN chmod +x /usr/local/bin/ocs-operator /usr/local/bin/provider-api /usr/local/bin/entrypoint USER operator -ENTRYPOINT ["/usr/local/bin/ocs-operator"] +ENTRYPOINT ["/usr/local/bin/entrypoint"] diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 63babb6d03..0e945344f9 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -23,7 +23,7 @@ spec: serviceAccountName: ocs-operator containers: - command: - - ocs-operator + - entrypoint args: - --enable-leader-election - "--health-probe-bind-address=:8081" diff --git a/deploy/csv-templates/ocs-operator.csv.yaml.in b/deploy/csv-templates/ocs-operator.csv.yaml.in index f4ed25398a..a230eb4102 100644 --- a/deploy/csv-templates/ocs-operator.csv.yaml.in +++ b/deploy/csv-templates/ocs-operator.csv.yaml.in @@ -537,7 +537,7 @@ spec: - --enable-leader-election - --health-probe-bind-address=:8081 command: - - ocs-operator + - entrypoint env: - name: WATCH_NAMESPACE valueFrom: diff --git a/deploy/ocs-operator/manifests/ocs-operator.clusterserviceversion.yaml b/deploy/ocs-operator/manifests/ocs-operator.clusterserviceversion.yaml index 33d9798c30..ecb465f9b9 100644 --- a/deploy/ocs-operator/manifests/ocs-operator.clusterserviceversion.yaml +++ b/deploy/ocs-operator/manifests/ocs-operator.clusterserviceversion.yaml @@ -546,7 +546,7 @@ spec: - --enable-leader-election - --health-probe-bind-address=:8081 command: - - ocs-operator + - entrypoint env: - name: WATCH_NAMESPACE valueFrom: diff --git a/hack/entrypoint.sh b/hack/entrypoint.sh new file mode 100755 index 0000000000..bc58ad148c --- /dev/null +++ b/hack/entrypoint.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +RESTART_EXIT_CODE=42 + +while true; do + ./usr/local/bin/ocs-operator $@ + EXIT_CODE=$? + if [ $EXIT_CODE -ne $RESTART_EXIT_CODE ]; then + exit $EXIT_CODE + fi +done