Create or update StorageClusterPeer #221

vbnrh · 2024-07-12T07:09:48Z

Updated MirrorPeerReconciler to handle StorageClientType by adding conditions in Reconcile method.
Introduced createStorageClusterPeer function for creating StorageClusterPeer objects.
Added utility functions fetchClientInfoConfigMap and getClientInfoFromConfigMap for handling client info.
Modified processManagedClusterAddon to utilize new config structure.
Enhanced DRPolicyReconciler to skip certain operations based on StorageClientType.

openshift-ci · 2024-07-12T07:09:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vbnrh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [vbnrh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

vbnrh · 2024-07-12T07:10:26Z

Commits contain changes from #220 and can be ignored

vbnrh · 2024-07-12T11:04:41Z

/test integration-test

vbnrh · 2024-07-17T04:38:02Z

go.sum

@@ -711,8 +710,8 @@ github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3c
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
 github.com/ramendr/ramen/api v0.0.0-20240409201920-10024cae3bfd h1:TsDaQqfb1BcR78JWXBmUyj6Qx4By5loUZ95CxmA/6zo=
 github.com/ramendr/ramen/api v0.0.0-20240409201920-10024cae3bfd/go.mod h1:PCb0ODjdi4eYuxY/nSw+/rQqmzkmBVqGNoDr2JXdlKE=
-github.com/red-hat-storage/ocs-operator/api/v4 v4.0.0-20240327160100-bbe9d9d49462 h1:84M7EBnmBISt2LcoyYPWsw+A3/7BGXp6Mh3sjUH5vIg=
-github.com/red-hat-storage/ocs-operator/api/v4 v4.0.0-20240327160100-bbe9d9d49462/go.mod h1:uySjux/lY0DpC+VXof4ly2SlS7QUocTm2CH4sU8ICeg=
+github.com/rewantsoni/ocs-operator/api/v4 v4.0.0-20240701052137-de69df292a5d h1:zXBbu5hpZmW4i3heppui4pqbvubZF/WsBiuP6ZekNKE=


Temporarily till this merges red-hat-storage/ocs-operator#2466

red-hat-storage/ocs-operator#2677 is merged. We can update this.

vbnrh · 2024-07-17T04:38:51Z

go.mod

@@ -133,6 +133,7 @@ replace (
 	github.com/openshift/hive => github.com/openshift/hive v1.1.17-0.20220223000051-b1c8fa5853b1
 	github.com/openshift/hive/apis => github.com/openshift/hive/apis v0.0.0-20220221165319-b389a65758da
 	github.com/portworx/sched-ops => github.com/portworx/sched-ops v0.20.4-openstorage-rc3
+	github.com/red-hat-storage/ocs-operator/api/v4 => github.com/rewantsoni/ocs-operator/api/v4 v4.0.0-20240701052137-de69df292a5d


To be removed as soon as this merges red-hat-storage/ocs-operator#2466

Update: red-hat-storage/ocs-operator#2677 is the PR, the older PR was before the design changes

rewantsoni · 2024-07-17T06:30:51Z

controllers/utils/peer_ref.go

+	if peerRefs[0].StorageClusterRef.Namespace == "" && peerRefs[1].StorageClusterRef.Namespace == "" {
+		return true
+	}
+	return false


Should we use Namespace empty as the condition to verify if it is a StorageClient or check for its existence?
We could simplify this to

return peerRefs[0].StorageClusterRef.Namespace == "" && peerRefs[1].StorageClusterRef.Namespace == ""

This is just a MirrorPeer check to see if its client pairing . Later more changes will made to API to make it more explicit.

rewantsoni · 2024-07-17T06:32:19Z

controllers/mirrorpeer_controller.go

+func getClientInfoFromConfigMap(clientInfoMap map[string]string, clientName string) (ClientInfo, error) {
+	clientInfoJSON, ok := clientInfoMap[clientName]


This might change after https://github.com/red-hat-storage/odf-multicluster-orchestrator/pull/219/files#r1680476919

vbnrh · 2024-08-13T08:11:46Z

/test integration-test

umangachapagain · 2024-08-19T11:10:33Z

api/v1alpha1/mirrorpeer_types.go

-	Name      string `json:"name"`
+	Name string `json:"name"`
+
+	// +kubebuilder:validation:Optional
 	Namespace string `json:"namespace"`


Suggested change

Namespace string `json:"namespace"`

Namespace string `json:"namespace,omitempty"`

umangachapagain · 2024-08-19T11:12:04Z

addons/agent_mirrorpeer_controller.go

-				r.Logger.Error("Failed to create ODR ObjectBucketClaim", "error", err, "MirrorPeer", mirrorPeer.Name, "namespace", scNamespace)
-				return err
-			}
+	logger := r.Logger.With("MirrorPeer", mirrorPeer.Name)


This is already set by the main Reconcile() function.

umangachapagain · 2024-08-19T11:15:05Z

controllers/drpolicy_controller.go

@@ -113,6 +113,11 @@ func (r *DRPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		return ctrl.Result{}, err
 	}

+	if utils.IsStorageClientType(mirrorPeer.Spec.Items) {
+		logger.Info("MirrorPeer contains StorageClient reference. Skipping creation of VolumeReplicationClasses", "MirrorPeer", mirrorPeer.Name)


Suggested change

logger.Info("MirrorPeer contains StorageClient reference. Skipping creation of VolumeReplicationClasses", "MirrorPeer", mirrorPeer.Name)

logger.Info("MirrorPeer contains StorageClient reference. Skipping creation of VolumeReplicationClasses")

Isn't MirrorPeer name already injected in the main reconcile function?

This is DRPolicy controller, only the req is injected with the logger. MirrorPeer is fetched separately

umangachapagain · 2024-08-19T11:15:59Z

controllers/mirrorpeer_controller.go

+const (
+	mirrorPeerFinalizer         = "hub.multicluster.odf.openshift.io"
+	spokeClusterRoleBindingName = "spoke-clusterrole-bindings"
+	ClientConfigMapKeyTemplate  = "%s/%s"


This is not a constant.

umangachapagain · 2024-08-19T11:17:22Z

controllers/mirrorpeer_controller.go

+func getKey(clusterName, clientName string) string {
+	return fmt.Sprintf(ClientConfigMapKeyTemplate, clusterName, clientName)
+}


Do we really need this? I think Sprintf is good enough.

I prefer it this way as it is more verbally descriptive. I will remove the clientconfigmapkey and use a format string

umangachapagain · 2024-08-19T11:18:04Z

controllers/mirrorpeer_controller.go

+		// Provider B's onboarding token will be used for Provider A's StorageClusterPeer
+		onboardingToken, err := fetchOnboardingTicket(ctx, client, oppositeClient, mirrorPeer)
+		if err != nil {
+			return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. err %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)


Suggested change

return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. err %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)

return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)

umangachapagain · 2024-08-19T11:21:44Z

controllers/utils/peer_ref.go

@@ -33,3 +33,7 @@ func GetPeerRefForSpokeCluster(mp *multiclusterv1alpha1.MirrorPeer, spokeCluster
 	}
 	return nil, fmt.Errorf("PeerRef for cluster %s under mirrorpeer %s not found", spokeClusterName, mp.Name)
 }
+
+func IsStorageClientType(peerRefs []multiclusterv1alpha1.PeerRef) bool {
+	return peerRefs[0].StorageClusterRef.Namespace == "" && peerRefs[1].StorageClusterRef.Namespace == ""


This isn't the right way to check this. IMO we need to cross-reference with our configmap.

…pe handling - Updated `MirrorPeerReconciler` to handle `StorageClientType` by adding conditions in `Reconcile` method. - Introduced `createStorageClusterPeer` function for creating `StorageClusterPeer` objects. - Added utility functions `fetchClientInfoConfigMap` and `getClientInfoFromConfigMap` for handling client info. - Modified `processManagedClusterAddon` to utilize new config structure. - Enhanced `DRPolicyReconciler` to skip certain operations based on `StorageClientType`. Signed-off-by: vbadrina <[email protected]>

Signed-off-by: vbadrina <[email protected]>

- New utility functions for checking if MirrorPeer is having StorageClient ref - Functions cross check with the configmap to see if client - Functions will take a parameter for managedCluster and hub cluster separately to pick the configmap properly Signed-off-by: vbadrina <[email protected]>

openshift-ci bot added the approved label Jul 12, 2024

openshift-merge-robot added the needs-rebase label Jul 12, 2024

vbnrh force-pushed the mirror-api-changes branch from a2f2376 to b912827 Compare July 12, 2024 13:22

openshift-merge-robot removed the needs-rebase label Jul 12, 2024

vbnrh force-pushed the mirror-api-changes branch from b912827 to 686de01 Compare July 12, 2024 13:50

vbnrh changed the title ~~Refactor MirrorPeerReconciler, API changes for StorageClient and deps updates~~ API changes for StorageClient Jul 12, 2024

vbnrh force-pushed the mirror-api-changes branch 3 times, most recently from d5eb008 to 66aa1cd Compare July 17, 2024 04:22

vbnrh marked this pull request as draft July 17, 2024 04:23

openshift-ci bot added the do-not-merge/work-in-progress label Jul 17, 2024

vbnrh changed the title ~~API changes for StorageClient~~ Create or update StorageClusterPeer Jul 17, 2024

vbnrh force-pushed the mirror-api-changes branch from 66aa1cd to e7b374d Compare July 17, 2024 04:34

vbnrh commented Jul 17, 2024

View reviewed changes

rewantsoni suggested changes Jul 17, 2024

View reviewed changes

openshift-ci bot assigned rewantsoni Jul 17, 2024

vbnrh force-pushed the mirror-api-changes branch 3 times, most recently from e43ec63 to 62202a2 Compare July 17, 2024 13:08

openshift-merge-robot added the needs-rebase label Jul 21, 2024

vbnrh force-pushed the mirror-api-changes branch from 8f86941 to 31b9f0e Compare July 22, 2024 11:25

openshift-merge-robot removed the needs-rebase label Jul 22, 2024

vbnrh force-pushed the mirror-api-changes branch 3 times, most recently from 1aafe87 to 1a5c378 Compare July 24, 2024 11:16

vbnrh force-pushed the mirror-api-changes branch from 1a5c378 to 832ce81 Compare August 8, 2024 08:42

vbnrh marked this pull request as ready for review August 12, 2024 05:24

openshift-ci bot removed the do-not-merge/work-in-progress label Aug 12, 2024

umangachapagain requested changes Aug 19, 2024

View reviewed changes

openshift-ci bot assigned umangachapagain Aug 19, 2024

vbnrh requested a review from umangachapagain August 21, 2024 10:28

vbnrh force-pushed the mirror-api-changes branch from a196623 to 52a9939 Compare August 21, 2024 10:31

openshift-merge-robot added the needs-rebase label Sep 1, 2024

vbnrh added 3 commits September 9, 2024 14:09

Create objectbucketclaim for each client

24a3637

Signed-off-by: vbadrina <[email protected]>

vbnrh force-pushed the mirror-api-changes branch from 52a9939 to ebdb0b3 Compare September 10, 2024 09:37

openshift-merge-robot removed the needs-rebase label Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create or update StorageClusterPeer #221

Create or update StorageClusterPeer #221

vbnrh commented Jul 12, 2024 •

edited

Loading

openshift-ci bot commented Jul 12, 2024

vbnrh commented Jul 12, 2024

vbnrh commented Jul 12, 2024

vbnrh Jul 17, 2024

umangachapagain Sep 24, 2024

vbnrh Jul 17, 2024

rewantsoni Jul 17, 2024

rewantsoni Jul 17, 2024

vbnrh Jul 17, 2024

rewantsoni Jul 17, 2024

vbnrh commented Aug 13, 2024

umangachapagain Aug 19, 2024

vbnrh Aug 21, 2024

umangachapagain Aug 19, 2024

vbnrh Aug 21, 2024

umangachapagain Aug 19, 2024

vbnrh Aug 21, 2024

umangachapagain Aug 19, 2024

umangachapagain Aug 19, 2024

vbnrh Aug 21, 2024

umangachapagain Aug 19, 2024

vbnrh Aug 21, 2024

umangachapagain Aug 19, 2024

vbnrh Aug 21, 2024

		func getClientInfoFromConfigMap(clientInfoMap map[string]string, clientName string) (ClientInfo, error) {
		clientInfoJSON, ok := clientInfoMap[clientName]

	Namespace string `json:"namespace"`
	Namespace string `json:"namespace,omitempty"`

	logger.Info("MirrorPeer contains StorageClient reference. Skipping creation of VolumeReplicationClasses", "MirrorPeer", mirrorPeer.Name)
	logger.Info("MirrorPeer contains StorageClient reference. Skipping creation of VolumeReplicationClasses")

	return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. err %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)
	return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)

Create or update StorageClusterPeer #221

Are you sure you want to change the base?

Create or update StorageClusterPeer #221

Conversation

vbnrh commented Jul 12, 2024 • edited Loading

openshift-ci bot commented Jul 12, 2024

vbnrh commented Jul 12, 2024

vbnrh commented Jul 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbnrh commented Aug 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbnrh commented Jul 12, 2024 •

edited

Loading