Skip to content

Part 1: Add replication source and destination controllers#12

Merged
Madhu-1 merged 5 commits intoRamenDR:mainfrom
Rakshith-R:add-controllers
Mar 10, 2026
Merged

Part 1: Add replication source and destination controllers#12
Madhu-1 merged 5 commits intoRamenDR:mainfrom
Rakshith-R:add-controllers

Conversation

@Rakshith-R
Copy link
Collaborator

Part 1: Add replication source and destination controllers


Will be followed by or in parallel to the following parts in separate prs.

Part 2: Movers (reconcile job and associated resources)
Part 3: Workers (source client and destination server)
Part 4: e2e

@Rakshith-R Rakshith-R requested a review from Madhu-1 March 6, 2026 05:17
@@ -0,0 +1,39 @@
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need two mover_scc? One is here, and another is in cmd/manager. i assume its a mistake?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my mistake.
Removing this.

instance := &volsyncv1alpha1.ReplicationDestination{}
if err := r.Get(ctx, req.NamespacedName, instance); err != nil {
if !kerrors.IsNotFound(err) {
logger.Error(err, "Failed to get Destination")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return is missing here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a comment.

same return is used in line 95 below, client.IgnoreNotFound(err) will make the error nil.

destination *volsyncv1alpha1.ReplicationDestination, privileged bool) (mover.Mover, error) {
var dataMover mover.Mover
for _, builder := range Catalog {
candidate, err := builder.FromDestination(client, logger, eventRecorder, destination, privileged)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the error handling here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed same pattern as here
https://github.com/backube/volsync/blob/1a7616613085863d5fcc108d06cd47827d660f24/internal/controller/mover/builder.go#L102-L111

but will add a error log here now
We should not be hitting an error in our plugin.

// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=get;list;watch;create;update;patch;delete;escalate;bind
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete;escalate;bind
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;list;watch;create;update;patch;delete;escalate;bind
// +kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=volsync-privileged-mover,verbs=use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

volsync-privileged-mover is not the SCC we are creating, its ceph-volsync-plugin-privileged-mover

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed it

// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=get;list;watch;create;update;patch;delete;escalate;bind
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete;escalate;bind
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;list;watch;create;update;patch;delete;escalate;bind
// +kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=volsync-privileged-mover,verbs=use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong SCC name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made a generic scaffolding below.
forgot to remove this 😮‍💨

Will be removed.

// +kubebuilder:rbac:groups=volsync.backube,resources=replicationsources/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;update;patch;delete;deletecollection
// +kubebuilder:rbac:groups=core,resources=events,verbs=create;update;patch
// +kubebuilder:rbac:groups=apps,resources=nodes,verbs=get;list;watch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node group is "" not app?

setupLog = ctrl.Log.WithName("setup")

//go:embed openshift/mover_scc.yaml
CephVolsyncPluginMoverSCCYamlRaw []byte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to export this as its not used outside?

Comment on lines +31 to +32
ErrNoMoverFound = fmt.Errorf("a replication method must be specified")
ErrMultipleMoversFound = fmt.Errorf("only one replication method can be supplied")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use errors.New?

var dataMover mover.Mover
for _, builder := range Catalog {
candidate, err := builder.FromSource(client, logger, eventRecorder, source, privileged)
if err == nil && candidate != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the err!=nil case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed same pattern as here
https://github.com/backube/volsync/blob/1a7616613085863d5fcc108d06cd47827d660f24/internal/controller/mover/builder.go#L102-L111

but will add a error log here now
We should not be hitting an error in our plugin.


// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
// TODO(user): Modify the Reconcile function to compare the state specified by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update the comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the comment

Signed-off-by: Rakshith R <rar@redhat.com>
Signed-off-by: Rakshith R <rar@redhat.com>
Signed-off-by: Rakshith R <rar@redhat.com>
Signed-off-by: Rakshith R <rar@redhat.com>
Signed-off-by: Rakshith R <rar@redhat.com>
@Rakshith-R Rakshith-R requested a review from Madhu-1 March 9, 2026 10:59
@Madhu-1 Madhu-1 merged commit fafc6b7 into RamenDR:main Mar 10, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants