Repository: ceph/ceph-csi
Pull request #2216 (2021-06-28 11:54:18)

rbd: set scheduling interval on snapshot mirrored image
Mirror-snapshots can also be automatically created on a periodic basis if mirror-snapshot schedules are defined. The mirror snapshot can be scheduled globally, per-pool, or per-image levels. Multiple mirror-snapshot schedules can be defined at any level.
    
To create a mirror-snapshot schedule with rbd, specify the mirror snapshot schedule add command along with an  optional pool or image name; interval; and optional start time:
 The interval can be specified in days, hours, or minutes using d, h, m suffix respectively. The optional start-time can be specified using the ISO 8601 time format. For example:
    
```
$ rbd --cluster site-a mirror snapshot schedule add --pool image-pool --image image1 24h 14:00:00-05:00
```

fixes: #2119


**Note:-**  Documentation will be covered in #2117

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1: # Snapshot scheduling testing

```yaml=
apiVersion: replication.storage.openshift.io/v1alpha1
kind: VolumeReplicationClass
metadata:
  name: rbd-volumereplicationclass
spec:
  provisioner: rook-ceph.rbd.csi.ceph.com
  parameters:
    mirroringMode: snapshot
    schedulingInterval: "12"
    schedulingStartTime: "16:18:43"
    replication.storage.openshift.io/replication-secret-name: rook-csi-rbd-provisioner
    replication.storage.openshift.io/replication-secret-namespace: rook-ceph
```
## When invalid interval is provided

```
d7d5-11eb-a951-0242ac110006 GRPC error: rpc error: code = Internal desc = rados: ret=-22, Invalid argument: "Invalid interval (12ad)"
```
## when invalid startTime is provided

```
E0628 05:59:47.154659       1 utils.go:172] ID: 95 Req-ID: 0001-0009-rook-ceph-0000000000000001-5cf49c95-d7d5-11eb-a951-0242ac110006 GRPC error: rpc error: code = Internal desc = rados: ret=-22, Invalid argument: "Invalid start time 26:18:43: hour must be in 0..23"
```

## Only scheduling time is provided in volumereplicationclass

```
E0628 06:01:11.661248       1 utils.go:172] ID: 110 Req-ID: 0001-0009-rook-ceph-0000000000000001-3f70ca01-d4c4-11eb-a392-0242ac110005 GRPC error: rpc error: code = InvalidArgument desc = schedulingStartTime parameter is supported only with schedulingInterval
```

## When only interval is provided

```
sh-4.4# rbd mirror snapshot schedule ls --pool=replicapool --image=csi-vol-5cf49c95-d7d5-11eb-a951-0242ac110006 --format=json
[{"interval":"12m","start_time":""}]
```

## When both interval and scheduling start time is provided

```
sh-4.4# rbd mirror snapshot schedule ls --pool=replicapool --image=csi-vol-5cf49c95-d7d5-11eb-a951-0242ac110006 --format=json
[{"interval":"12m","start_time":"16:18:00"}]
```
Madhu-1: > Just a quick initial look, definitely needs an updated version in `go.mod` instead of hacking `vendor/`. Releases of ceph-csi should only refer to released versions of go-ceph, so there needs to be an other issue/PR that rebases go-ceph later.

This is not hacking anything in vendor, `rbd/admin` is already available in go-ceph 0.10.0 release. till now `rbd/admin` has not imported, This PR is importing it.
nixpanic: > > Just a quick initial look, definitely needs an updated version in `go.mod` instead of hacking `vendor/`. Releases of ceph-csi should only refer to released versions of go-ceph, so there needs to be an other issue/PR that rebases go-ceph later.
> 
> This is not hacking anything in vendor, `rbd/admin` is already available in go-ceph 0.10.0 release. till now `rbd/admin` has not imported, This PR is importing it.

Oh, really?! Sorry about the confusion then :see_no_evil: 
Yuggupta27: /test ci/centos/mini-e2e-helm-sc/k8s-1.20
Yuggupta27: > /test ci/centos/mini-e2e-helm-sc/k8s-1.20

Kindly ignore, want to test something
Madhu-1: /retest ci/centos/mini-e2e-helm/k8s-1.20
Madhu-1: > /retest ci/centos/mini-e2e-helm/k8s-1.20

` etcdserver: request timed out`
Madhu-1: /retest ci/centos/mini-e2e-helm/k8s-1.21
Madhu-1: > /retest ci/centos/mini-e2e-helm/k8s-1.21

`Jul  5 12:13:11.973: INFO: Error getting pvc in namespace: '': etcdserver: request timed out
Jul  5 12:13:11.973: INFO: failed to create PVC (rbd-13180): failed to get pvc: etcdserver: request timed out`
Madhu-1: /retest ci/centos/mini-e2e/k8s-1.21
humblec: Apart from https://github.com/ceph/ceph-csi/pull/2216#discussion_r664360483, this looks good to me :+1: 
Madhu-1: > Apart from [#2216 (comment)](https://github.com/ceph/ceph-csi/pull/2216#discussion_r664360483), this looks good to me

This is already answered at https://github.com/ceph/ceph-csi/pull/2216#discussion_r664364858 @humblec anything else is required?
Madhu-1: @mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 

Madhu-1: /retest ci/centos/mini-e2e-helm/k8s-1.19
Madhu-1: /retest ci/centos/mini-e2e-helm/k8s-1.20
Madhu-1: > /retest ci/centos/mini-e2e-helm/k8s-1.20

`X Exiting due to GUEST_PROVISION: Failed to start host: creating host: create: Error creating machine: Error in driver during machine creation: IP not available after waiting: machine minikube didn't return IP after 1 minute
`
Madhu-1: > /retest ci/centos/mini-e2e-helm/k8s-1.20

`failed to create PVC with error failed to get pvc: etcdserver: request timed out`

Patch:
diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go
index e5608b3e2..987191a94 100644
--- a/internal/rbd/rbd_util.go
+++ b/internal/rbd/rbd_util.go
@@ -32,6 +32,7 @@ import (
 
 	"github.com/ceph/go-ceph/rados"
 	librbd "github.com/ceph/go-ceph/rbd"
+	"github.com/ceph/go-ceph/rbd/admin"
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/golang/protobuf/ptypes"
 	"github.com/golang/protobuf/ptypes/timestamp"
@@ -1733,3 +1734,41 @@ func (rs *rbdSnapshot) isCompatibleThickProvision(dst *rbdVolume) error {
 
 	return nil
 }
+
+func (ri *rbdImage) addSnapshotScheduling(
+	interval admin.Interval,
+	startTime admin.StartTime) error {
+	ls := admin.NewLevelSpec(ri.Pool, ri.RadosNamespace, ri.RbdImageName)
+	ra, err := ri.conn.GetRBDAdmin()
+	if err != nil {
+		return err
+	}
+	adminConn := ra.MirrorSnashotSchedule()
+	// list all the snapshot scheduling and check at least one image scheduling
+	// exists with specified interval.
+	ssList, err := adminConn.List(ls)
+	if err != nil {
+		return err
+	}
+
+	for _, ss := range ssList {
+		// make sure we are matching image level scheduling. The
+		// `adminConn.List` lists the global level scheduling also.
+		if ss.Name == ri.String() {
+			for _, s := range ss.Schedule {
+				// TODO: Add support to check start time also.
+				// The start time is currently stored with different format
+				// in ceph. Comparison is not possible unless we know in
+				// which format ceph is storing it.
+				if s.Interval == interval {
+					return err
+				}
+			}
+		}
+	}
+	err = adminConn.Add(ls, interval, startTime)
+	if err != nil {
+		return err
+	}
+	return nil
+}
diff --git a/internal/rbd/replicationcontrollerserver.go b/internal/rbd/replicationcontrollerserver.go
index 8ce695056..294e0e897 100644
--- a/internal/rbd/replicationcontrollerserver.go
+++ b/internal/rbd/replicationcontrollerserver.go
@@ -19,12 +19,14 @@ package rbd
 import (
 	"context"
 	"errors"
+	"regexp"
 	"strconv"
 	"strings"
 
 	"github.com/ceph/ceph-csi/internal/util"
 
 	librbd "github.com/ceph/go-ceph/rbd"
+	"github.com/ceph/go-ceph/rbd/admin"
 	"github.com/csi-addons/spec/lib/go/replication"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -56,6 +58,18 @@ const (
 	imageMirroringKey = "mirroringMode"
 	// forceKey + key to get the force option from parameters.
 	forceKey = "force"
+
+	// schedulingIntervalKey to get the schedulingInterval from the
+	// parameters.
+	// Interval of time between scheduled snapshots. Typically in the form
+	// <num><m,h,d>.
+	schedulingIntervalKey = "schedulingInterval"
+
+	// schedulingStartTimeKey to get the schedulingStartTime from the
+	// parameters.
+	// (optional) StartTime is the time the snapshot schedule
+	// begins, can be specified using the ISO 8601 time format.
+	schedulingStartTimeKey = "schedulingStartTime"
 )
 
 // ReplicationServer struct of rbd CSI driver with supported methods of Replication
@@ -108,6 +122,56 @@ func getMirroringMode(ctx context.Context, parameters map[string]string) (librbd
 	return mirroringMode, nil
 }
 
+// getSchedulingDetails gets the mirroring mode and scheduling details from the
+// input GRPC request parameters and validates the scheduling is only supported
+// for mirroring mode.
+func getSchedulingDetails(parameters map[string]string) (admin.Interval, admin.StartTime, error) {
+	admInt := admin.NoInterval
+	adminStartTime := admin.NoStartTime
+	var err error
+
+	val := parameters[imageMirroringKey]
+
+	switch imageMirroringMode(val) {
+	case imageMirrorModeSnapshot:
+	default:
+		return admInt, adminStartTime, status.Error(codes.InvalidArgument, "scheduling is only supported for snapshot mode")
+	}
+
+	// validate mandatory interval field
+	interval, ok := parameters[schedulingIntervalKey]
+	if ok && interval == "" {
+		return admInt, adminStartTime, status.Error(codes.InvalidArgument, "scheduling interval cannot be empty")
+	}
+	adminStartTime = admin.StartTime(parameters[schedulingStartTimeKey])
+	if !ok {
+		// startTime is alone not supported it has to be present with interval
+		if adminStartTime != "" {
+			return admInt, admin.NoStartTime, status.Errorf(codes.InvalidArgument,
+				"%q parameter is supported only with %q",
+				schedulingStartTimeKey,
+				schedulingIntervalKey)
+		}
+	}
+	if interval != "" {
+		admInt, err = validateSchedulingInterval(interval)
+		if err != nil {
+			return admInt, admin.NoStartTime, status.Error(codes.InvalidArgument, err.Error())
+		}
+	}
+	return admInt, adminStartTime, nil
+}
+
+// validateSchedulingInterval return the interval as it is if its ending with
+// `m|h|d` or else it will return error.
+func validateSchedulingInterval(interval string) (admin.Interval, error) {
+	var re = regexp.MustCompile(`^[0-9]+[mhd]$`)
+	if re.MatchString(interval) {
+		return admin.Interval(interval), nil
+	}
+	return "", errors.New("interval specified without d, h, m suffix")
+}
+
 // EnableVolumeReplication extracts the RBD volume information from the
 // volumeID, If the image is present it will enable the mirroring based on the
 // user provided information.
@@ -124,6 +188,11 @@ func (rs *ReplicationServer) EnableVolumeReplication(ctx context.Context,
 	}
 	defer cr.DeleteCredentials()
 
+	interval, startTime, err := getSchedulingDetails(req.GetParameters())
+	if err != nil {
+		return nil, err
+	}
+
 	if acquired := rs.VolumeLocks.TryAcquire(volumeID); !acquired {
 		util.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, volumeID)
 		return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID)
@@ -162,6 +231,20 @@ func (rs *ReplicationServer) EnableVolumeReplication(ctx context.Context,
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 	}
+
+	if interval != "" {
+		err = rbdVol.addSnapshotScheduling(interval, startTime)
+		if err != nil {
+			return nil, err
+		}
+		util.DebugLog(
+			ctx,
+			"Added scheduling at interval %s, start time %s for volume %s",
+			interval,
+			startTime,
+			rbdVol)
+	}
+
 	return &replication.EnableVolumeReplicationResponse{}, nil
 }
 
diff --git a/internal/util/connection.go b/internal/util/connection.go
index b25e35a6d..6d17697d9 100644
--- a/internal/util/connection.go
+++ b/internal/util/connection.go
@@ -23,6 +23,7 @@ import (
 
 	ca "github.com/ceph/go-ceph/cephfs/admin"
 	"github.com/ceph/go-ceph/rados"
+	ra "github.com/ceph/go-ceph/rbd/admin"
 )
 
 type ClusterConnection struct {
@@ -111,6 +112,15 @@ func (cc *ClusterConnection) GetFSAdmin() (*ca.FSAdmin, error) {
 	return ca.NewFromConn(cc.conn), nil
 }
 
+// GetRBDAdmin get RBDAdmin to administrate rbd volumes.
+func (cc *ClusterConnection) GetRBDAdmin() (*ra.RBDAdmin, error) {
+	if cc.conn == nil {
+		return nil, errors.New("cluster is not connected yet")
+	}
+
+	return ra.NewFromConn(cc.conn), nil
+}
+
 // DisableDiscardOnZeroedWriteSame enables the
 // `rbd_discard_on_zeroed_write_same` option in the cluster connection, so that
 // writing zero blocks of data are actual writes on the OSDs (doing
diff --git a/vendor/github.com/ceph/go-ceph/rbd/admin/admin.go b/vendor/github.com/ceph/go-ceph/rbd/admin/admin.go
new file mode 100644
index 000000000..955b3ef7d
--- /dev/null
+++ b/vendor/github.com/ceph/go-ceph/rbd/admin/admin.go
@@ -0,0 +1,51 @@
+// +build !nautilus
+
+package admin
+
+import (
+	"fmt"
+
+	ccom "github.com/ceph/go-ceph/common/commands"
+)
+
+// RBDAdmin is used to administrate rbd volumes and pools.
+type RBDAdmin struct {
+	conn ccom.RadosCommander
+}
+
+// NewFromConn creates an new management object from a preexisting
+// rados connection. The existing connection can be rados.Conn or any
+// type implementing the RadosCommander interface.
+func NewFromConn(conn ccom.RadosCommander) *RBDAdmin {
+	return &RBDAdmin{conn}
+}
+
+// LevelSpec values are used to identify RBD objects wherever Ceph APIs
+// require a levelspec to select an image, pool, or namespace.
+type LevelSpec struct {
+	spec string
+}
+
+// NewLevelSpec is used to construct a LevelSpec given a pool and
+// optional namespace and image names.
+func NewLevelSpec(pool, namespace, image string) LevelSpec {
+	var s string
+	if image != "" && namespace != "" {
+		s = fmt.Sprintf("%s/%s/%s", pool, namespace, image)
+	} else if image != "" {
+		s = fmt.Sprintf("%s/%s", pool, image)
+	} else if namespace != "" {
+		s = fmt.Sprintf("%s/%s/", pool, namespace)
+	} else {
+		s = fmt.Sprintf("%s/", pool)
+	}
+	return LevelSpec{s}
+}
+
+// NewRawLevelSpec returns a LevelSpec directly based on the spec string
+// argument without constructing it from component values. This should only be
+// used if NewLevelSpec can not create the levelspec value you want to pass to
+// ceph.
+func NewRawLevelSpec(spec string) LevelSpec {
+	return LevelSpec{spec}
+}
diff --git a/vendor/github.com/ceph/go-ceph/rbd/admin/doc.go b/vendor/github.com/ceph/go-ceph/rbd/admin/doc.go
new file mode 100644
index 000000000..44d4fab6a
--- /dev/null
+++ b/vendor/github.com/ceph/go-ceph/rbd/admin/doc.go
@@ -0,0 +1,10 @@
+/*
+Package admin is a convenience layer to support the administration of
+rbd volumes, snapshots, scheduling etc that is outside of the C api
+for librbd.
+
+Unlike the rbd package this API does not map to APIs provided by
+ceph libraries themselves. This API is not yet stable and is subject
+to change.
+*/
+package admin
diff --git a/vendor/github.com/ceph/go-ceph/rbd/admin/msschedule.go b/vendor/github.com/ceph/go-ceph/rbd/admin/msschedule.go
new file mode 100644
index 000000000..bbb7f0c91
--- /dev/null
+++ b/vendor/github.com/ceph/go-ceph/rbd/admin/msschedule.go
@@ -0,0 +1,170 @@
+// +build !nautilus
+
+package admin
+
+import (
+	ccom "github.com/ceph/go-ceph/common/commands"
+	"github.com/ceph/go-ceph/internal/commands"
+)
+
+// Interval of time between scheduled snapshots. Typically in the form
+// <num><m,h,d>. Exact content supported is defined internally on the ceph mgr.
+type Interval string
+
+// StartTime is the time the snapshot schedule begins. Exact content supported
+// is defined internally on the ceph mgr.
+type StartTime string
+
+var (
+	// NoInterval indicates no specific interval.
+	NoInterval = Interval("")
+
+	// NoStartTime indicates no specific start time.
+	NoStartTime = StartTime("")
+)
+
+// MirrorSnashotScheduleAdmin encapsulates management functions for
+// ceph rbd mirror snapshot schedules.
+type MirrorSnashotScheduleAdmin struct {
+	conn ccom.MgrCommander
+}
+
+// MirrorSnashotSchedule returns a MirrorSnashotScheduleAdmin type for
+// managing ceph rbd mirror snapshot schedules.
+func (ra *RBDAdmin) MirrorSnashotSchedule() *MirrorSnashotScheduleAdmin {
+	return &MirrorSnashotScheduleAdmin{conn: ra.conn}
+}
+
+// Add a new snapshot schedule to the given pool/image based on the supplied
+// level spec.
+//
+// Similar To:
+//  rbd mirror snapshot schedule add <level_spec> <interval> <start_time>
+func (mss *MirrorSnashotScheduleAdmin) Add(l LevelSpec, i Interval, s StartTime) error {
+	m := map[string]string{
+		"prefix":     "rbd mirror snapshot schedule add",
+		"level_spec": l.spec,
+		"format":     "json",
+	}
+	if i != NoInterval {
+		m["interval"] = string(i)
+	}
+	if s != NoStartTime {
+		m["start_time"] = string(s)
+	}
+	return commands.MarshalMgrCommand(mss.conn, m).NoData().End()
+}
+
+// List the snapshot schedules based on the supplied level spec.
+//
+// Similar To:
+//  rbd mirror snapshot schedule list <level_spec>
+func (mss *MirrorSnashotScheduleAdmin) List(l LevelSpec) ([]SnapshotSchedule, error) {
+	m := map[string]string{
+		"prefix":     "rbd mirror snapshot schedule list",
+		"level_spec": l.spec,
+		"format":     "json",
+	}
+	return parseMirrorSnapshotScheduleList(
+		commands.MarshalMgrCommand(mss.conn, m))
+}
+
+type snapshotScheduleMap map[string]snapshotScheduleSubsection
+
+type snapshotScheduleSubsection struct {
+	Name     string         `json:"name"`
+	Schedule []ScheduleTerm `json:"schedule"`
+}
+
+// ScheduleTerm represents the interval and start time component of
+// a snapshot schedule.
+type ScheduleTerm struct {
+	Interval  Interval  `json:"interval"`
+	StartTime StartTime `json:"start_time"`
+}
+
+// SnapshotSchedule contains values representing an entire snapshot schedule
+// for an image or pool.
+type SnapshotSchedule struct {
+	Name        string
+	LevelSpecID string
+	Schedule    []ScheduleTerm
+}
+
+func parseMirrorSnapshotScheduleList(res commands.Response) (
+	[]SnapshotSchedule, error) {
+
+	var ss snapshotScheduleMap
+	if err := res.NoStatus().Unmarshal(&ss).End(); err != nil {
+		return nil, err
+	}
+
+	var sched []SnapshotSchedule
+	for k, v := range ss {
+		sched = append(sched, SnapshotSchedule{
+			Name:        v.Name,
+			LevelSpecID: k,
+			Schedule:    v.Schedule,
+		})
+	}
+	return sched, nil
+}
+
+// Remove a snapshot schedule matching the supplied arguments.
+//
+// Similar To:
+//  rbd mirror snapshot schedule remove <level_spec> <interval> <start_time>
+func (mss *MirrorSnashotScheduleAdmin) Remove(
+	l LevelSpec, i Interval, s StartTime) error {
+
+	m := map[string]string{
+		"prefix":     "rbd mirror snapshot schedule remove",
+		"level_spec": l.spec,
+		"format":     "json",
+	}
+	if i != NoInterval {
+		m["interval"] = string(i)
+	}
+	if s != NoStartTime {
+		m["start_time"] = string(s)
+	}
+	return commands.MarshalMgrCommand(mss.conn, m).NoData().End()
+}
+
+// Status returns the status of the snapshot (eg. when it will next take place)
+// matching the supplied level spec.
+//
+// Similar To:
+//  rbd mirror snapshot schedule status <level_spec>
+func (mss *MirrorSnashotScheduleAdmin) Status(l LevelSpec) ([]ScheduledImage, error) {
+	m := map[string]string{
+		"prefix":     "rbd mirror snapshot schedule status",
+		"level_spec": l.spec,
+		"format":     "json",
+	}
+	return parseMirrorSnapshotScheduleStatus(
+		commands.MarshalMgrCommand(mss.conn, m))
+}
+
+// ScheduleTime is the time a snapshot will occur.
+type ScheduleTime string
+
+// ScheduledImage contains the item scheduled and when it will next occur.
+type ScheduledImage struct {
+	Image        string       `json:"image"`
+	ScheduleTime ScheduleTime `json:"schedule_time"`
+}
+
+type scheduledImageWrapper struct {
+	ScheduledImages []ScheduledImage `json:"scheduled_images"`
+}
+
+func parseMirrorSnapshotScheduleStatus(res commands.Response) (
+	[]ScheduledImage, error) {
+
+	var siw scheduledImageWrapper
+	if err := res.NoStatus().Unmarshal(&siw).End(); err != nil {
+		return nil, err
+	}
+	return siw.ScheduledImages, nil
+}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 9b4ac7c3d..375d76323 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -59,6 +59,7 @@ github.com/ceph/go-ceph/internal/retry
 github.com/ceph/go-ceph/internal/timespec
 github.com/ceph/go-ceph/rados
 github.com/ceph/go-ceph/rbd
+github.com/ceph/go-ceph/rbd/admin
 # github.com/cespare/xxhash/v2 v2.1.1
 github.com/cespare/xxhash/v2
 # github.com/container-storage-interface/spec v1.5.0


Test patch:
diff --git a/internal/rbd/replicationcontrollerserver_test.go b/internal/rbd/replicationcontrollerserver_test.go
new file mode 100644
index 000000000..e5b2aada3
--- /dev/null
+++ b/internal/rbd/replicationcontrollerserver_test.go
@@ -0,0 +1,155 @@
+/*
+Copyright 2021 The Ceph-CSI Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rbd
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/ceph/go-ceph/rbd/admin"
+)
+
+func TestValidateSchedulingInterval(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		interval string
+		want     admin.Interval
+		wantErr  bool
+	}{
+		{
+			"valid interval in minutes",
+			"3m",
+			admin.Interval("3m"),
+			false,
+		},
+		{
+			"valid interval in hour",
+			"22h",
+			admin.Interval("22h"),
+			false,
+		},
+		{
+			"valid interval in days",
+			"13d",
+			admin.Interval("13d"),
+			false,
+		},
+		{
+			"invalid interval without number",
+			"d",
+			admin.Interval(""),
+			true,
+		},
+		{
+			"invalid interval without (m|h|d) suffix",
+			"12",
+			admin.Interval(""),
+			true,
+		},
+	}
+	for _, tt := range tests {
+		st := tt
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := validateSchedulingInterval(st.interval)
+			if (err != nil) != st.wantErr {
+				t.Errorf("validateSchedulingInterval() error = %v, wantErr %v", err, st.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, st.want) {
+				t.Errorf("validateSchedulingInterval() = %v, want %v", got, st.want)
+			}
+		})
+	}
+}
+
+func TestGetSchedulingDetails(t *testing.T) {
+	tests := []struct {
+		name          string
+		parameters    map[string]string
+		wantInterval  admin.Interval
+		wantStartTime admin.StartTime
+		wantErr       bool
+	}{
+		{
+			"valid parameters",
+			map[string]string{
+				imageMirroringKey:      string(imageMirrorModeSnapshot),
+				schedulingIntervalKey:  "1h",
+				schedulingStartTimeKey: "14:00:00-05:00",
+			},
+			admin.Interval("1h"),
+			admin.StartTime("14:00:00-05:00"),
+			false,
+		},
+		{
+			"valid parameters when optional startTime is missing",
+			map[string]string{
+				imageMirroringKey:     string(imageMirrorModeSnapshot),
+				schedulingIntervalKey: "1h",
+			},
+			admin.Interval("1h"),
+			admin.NoStartTime,
+			false,
+		},
+		{
+			"when mirroring mode is journal",
+			map[string]string{
+				imageMirroringKey:     "journal",
+				schedulingIntervalKey: "1h",
+			},
+			admin.NoInterval,
+			admin.NoStartTime,
+			true,
+		},
+		{
+			"when startTime is specified without interval",
+			map[string]string{
+				imageMirroringKey:      string(imageMirrorModeSnapshot),
+				schedulingStartTimeKey: "14:00:00-05:00",
+			},
+			admin.NoInterval,
+			admin.NoStartTime,
+			true,
+		},
+		{
+			"when no scheduling is specified",
+			map[string]string{
+				imageMirroringKey: string(imageMirrorModeSnapshot),
+			},
+			admin.NoInterval,
+			admin.NoStartTime,
+			false,
+		},
+	}
+	for _, tt := range tests {
+		st := tt
+		t.Run(tt.name, func(t *testing.T) {
+			interval, startTime, err := getSchedulingDetails(st.parameters)
+			if (err != nil) != st.wantErr {
+				t.Errorf("getSchedulingDetails() error = %v, wantErr %v", err, st.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(interval, st.wantInterval) {
+				t.Errorf("getSchedulingDetails() interval = %v, want %v", interval, st.wantInterval)
+			}
+			if !reflect.DeepEqual(startTime, st.wantStartTime) {
+				t.Errorf("getSchedulingDetails() startTime = %v, want %v", startTime, st.wantStartTime)
+			}
+		})
+	}
+}