Skip to content

Commit ab42435

Browse files
allmightyspiffallmightyspiff
committed
#730 Updated block|file snapshot-enable to support INTERVAL snapshots, and updated the help messages a bit
1 parent 638d14e commit ab42435

3 files changed

Lines changed: 67 additions & 70 deletions

File tree

‎plugin/commands/block/snapshot_enable.go‎

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package block
22

33
import (
4-
"strconv"
5-
64
"github.com/spf13/cobra"
5+
"slices"
6+
"strconv"
7+
"strings"
78

89
slErr "github.ibm.com/SoftLayer/softlayer-cli/plugin/errors"
910
. "github.ibm.com/SoftLayer/softlayer-cli/plugin/i18n"
@@ -21,6 +22,8 @@ var DAY_OF_WEEK = map[int]string{
2122
6: "SATURDAY",
2223
}
2324

25+
var SCHEDULES = []string{"INTERVAL", "HOURLY", "DAILY", "WEEKLY"}
26+
2427
type SnapshotEnableCommand struct {
2528
*metadata.SoftlayerStorageCommand
2629
Command *cobra.Command
@@ -40,8 +43,7 @@ func NewSnapshotEnableCommand(sl *metadata.SoftlayerStorageCommand) *SnapshotEna
4043
cobraCmd := &cobra.Command{
4144
Use: "snapshot-enable " + T("IDENTIFIER"),
4245
Short: T("Enable snapshots for a given volume on the specified schedule"),
43-
Long: T(`${COMMAND_NAME} sl {{.storageType}} snapshot-enable VOLUME_ID [OPTIONS]
44-
46+
Long: T(`See https://sldn.softlayer.com/reference/services/SoftLayer_Network_Storage/enableSnapshots/ for more details about these options.
4547
EXAMPLE:
4648
${COMMAND_NAME} sl {{.storageType}} snapshot-enable 12345678 -s WEEKLY -c 5 -m 0 --hour 2 -d 0
4749
This command enables snapshot for volume with ID 12345678, snapshot is taken weekly on every Sunday at 2:00, and up to 5 snapshots are retained.`, sl.StorageI18n),
@@ -50,11 +52,20 @@ EXAMPLE:
5052
return thisCmd.Run(args)
5153
},
5254
}
53-
cobraCmd.Flags().StringVarP(&thisCmd.ScheduleType, "schedule-type", "s", "", T("Snapshot schedule [required], options are: HOURLY,DAILY,WEEKLY"))
54-
cobraCmd.Flags().IntVarP(&thisCmd.RetentionCount, "retention-count", "c", 0, T("Number of snapshots to retain [required]"))
55-
cobraCmd.Flags().IntVarP(&thisCmd.Minute, "minute", "m", 0, T("Minute of the hour when snapshots should be taken, integer between 0 to 59"))
56-
cobraCmd.Flags().IntVarP(&thisCmd.Hour, "hour", "r", 0, T("Hour of the day when snapshots should be taken, integer between 0 to 23"))
57-
cobraCmd.Flags().IntVarP(&thisCmd.DayOfWeek, "day-of-week", "d", 0, T("Day of the week when snapshots should be taken, integer between 0 to 6. \n 0 means Sunday,1 means Monday,2 means Tuesday,3 means Wendesday,4 means Thursday,5 means Friday,6 means Saturday"))
55+
subs := map[string]interface{}{"ScheduleTypes": strings.Join(SCHEDULES, ", ")}
56+
cobraCmd.Flags().StringVarP(&thisCmd.ScheduleType, "schedule-type", "s", "",
57+
T("Snapshot schedule, options are: {{.ScheduleTypes}}", subs))
58+
cobraCmd.Flags().IntVarP(&thisCmd.RetentionCount, "retention-count", "c", 0, T("Number of snapshots to retain"))
59+
cobraCmd.Flags().IntVarP(&thisCmd.Minute, "minute", "m", 0,
60+
T("Minute of the hour when snapshots should be taken, integer between 0 to 59"))
61+
cobraCmd.Flags().IntVarP(&thisCmd.Hour, "hour", "r", 0,
62+
T("Hour of the day when snapshots should be taken, integer between 0 to 23"))
63+
cobraCmd.Flags().IntVarP(&thisCmd.DayOfWeek, "day-of-week", "d", 0,
64+
T(`Day of the week when snapshots should be taken, integer between 0 to 6.
65+
0 means Sunday,1 means Monday,2 means Tuesday,3 means Wendesday,4 means Thursday,5 means Friday,6 means Saturday`))
66+
cobraCmd.MarkFlagRequired("schedule-type") //#nosec G104 -- Doesn't matter if this errors
67+
cobraCmd.MarkFlagRequired("retention-count") //#nosec G104 -- Doesn't matter if this errors
68+
5869
thisCmd.Command = cobraCmd
5970
return thisCmd
6071
}
@@ -68,32 +79,36 @@ func (cmd *SnapshotEnableCommand) Run(args []string) error {
6879
if cmd.ScheduleType == "" {
6980
return slErr.NewInvalidUsageError(T("[-s|--schedule-type] is required, options are: HOURLY, DAILY, WEEKLY."))
7081
}
71-
scheduleType := cmd.ScheduleType
72-
if scheduleType != "HOURLY" && scheduleType != "DAILY" && scheduleType != "WEEKLY" {
73-
return slErr.NewInvalidUsageError(T("[-s|--schedule-type] must be HOURLY, DAILY, or WEEKLY."))
82+
83+
if !slices.Contains(SCHEDULES, cmd.ScheduleType) {
84+
subs := map[string]interface{}{"ScheduleTypes": strings.Join(SCHEDULES, ", "), "Selected": cmd.ScheduleType}
85+
return slErr.NewInvalidUsageError(
86+
T("[-s|--schedule-type] needs to be one of {{.ScheduleTypes}}, not {{.Selected}}.", subs))
7487
}
7588
retentionCount := cmd.RetentionCount
7689

7790
if retentionCount == 0 {
7891
return slErr.NewMissingInputError("-c|--retention-count")
7992
}
8093

81-
minute := cmd.Minute
82-
if minute < 0 || minute > 59 {
94+
if cmd.Minute < 0 || cmd.Minute > 59 {
8395
return slErr.NewInvalidUsageError(T("[-m|--minute] value must be between 0 and 59."))
8496
}
85-
hour := cmd.Hour
86-
if hour < 0 || hour > 23 {
97+
98+
if cmd.Hour < 0 || cmd.Hour > 23 {
8799
return slErr.NewInvalidUsageError(T("[-r|--hour] value must be between 0 and 23."))
88100
}
89-
dayOfWeek := cmd.DayOfWeek
90-
if dayOfWeek < 0 || dayOfWeek > 6 {
101+
102+
if cmd.DayOfWeek < 0 || cmd.DayOfWeek > 6 {
91103
return slErr.NewInvalidUsageError(T("[-d|--day-of-week] value must be between 0 and 6."))
92104
}
93-
err = cmd.StorageManager.EnableSnapshot(volumeID, scheduleType, retentionCount, minute, hour, DAY_OF_WEEK[dayOfWeek])
94-
subs := map[string]interface{}{"ScheduleType": scheduleType, "VolumeID": volumeID}
105+
106+
err = cmd.StorageManager.EnableSnapshot(
107+
volumeID, cmd.ScheduleType, retentionCount, cmd.Minute, cmd.Hour, DAY_OF_WEEK[cmd.DayOfWeek])
108+
subs := map[string]interface{}{"ScheduleType": cmd.ScheduleType, "VolumeID": volumeID}
95109
if err != nil {
96-
return slErr.NewAPIError(T("Failed to enable {{.ScheduleType}} snapshot for volume {{.VolumeID}}.\n", subs), err.Error(), 2)
110+
return slErr.NewAPIError(
111+
T("Failed to enable {{.ScheduleType}} snapshot for volume {{.VolumeID}}.\n", subs), err.Error(), 2)
97112
}
98113
cmd.UI.Ok()
99114
cmd.UI.Print(T("{{.ScheduleType}} snapshots have been enabled for volume {{.VolumeID}}.", subs))

‎plugin/commands/block/snapshot_enable_test.go‎

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -32,63 +32,38 @@ var _ = Describe("Snapshot enable", func() {
3232
})
3333

3434
Describe("Snapshot enable", func() {
35-
Context("Snapshot enable without volume id", func() {
36-
It("return error", func() {
35+
Context("Incorrect Usage Tests", func() {
36+
It("No arguments", func() {
3737
err := testhelpers.RunCobraCommand(cliCommand.Command)
3838
Expect(err).To(HaveOccurred())
3939
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: This command requires one argument"))
4040
})
41-
})
42-
Context("Snapshot enable with wrong volume id", func() {
43-
It("return error", func() {
44-
err := testhelpers.RunCobraCommand(cliCommand.Command, "abc")
41+
It("Bad Volume ID", func() {
42+
err := testhelpers.RunCobraCommand(cliCommand.Command, "a1234", "-c=100", "-s=INTERVAL")
4543
Expect(err).To(HaveOccurred())
4644
Expect(err.Error()).To(ContainSubstring("Invalid input for 'Volume ID'. It must be a positive integer."))
4745
})
48-
})
49-
50-
Context("Snapshot enable without -s", func() {
51-
It("return error", func() {
52-
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234")
46+
It("Bad Interval", func() {
47+
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "--schedule-type=FAKE", "-c=100")
5348
Expect(err).To(HaveOccurred())
54-
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: [-s|--schedule-type] is required, options are: HOURLY, DAILY, WEEKLY."))
49+
Expect(err.Error()).To(ContainSubstring("needs to be one of INTERVAL, HOURLY, DAILY, WEEKLY, not FAKE."))
5550
})
56-
})
57-
58-
Context("Snapshot enable with wrong -s", func() {
59-
It("return error", func() {
60-
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "MONTHLY")
61-
Expect(err).To(HaveOccurred())
62-
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: [-s|--schedule-type] must be HOURLY, DAILY, or WEEKLY."))
63-
})
64-
})
65-
66-
Context("Snapshot enable without -c", func() {
67-
It("return error", func() {
51+
It("No Retention Count", func() {
6852
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "HOURLY")
6953
Expect(err).To(HaveOccurred())
70-
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: '-c|--retention-count' is required"))
54+
Expect(err.Error()).To(ContainSubstring(`required flag(s) "retention-count" not set`))
7155
})
72-
})
73-
74-
Context("Snapshot enable with wrong -m", func() {
75-
It("return error", func() {
56+
It("Bad Minutes Value", func() {
7657
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "HOURLY", "-c", "3", "-m", "100")
7758
Expect(err).To(HaveOccurred())
7859
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: [-m|--minute] value must be between 0 and 59."))
7960
})
80-
})
81-
82-
Context("Snapshot enable with wrong --hour", func() {
83-
It("return error", func() {
61+
It("Wrong Hour", func() {
8462
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "HOURLY", "-c", "3", "-m", "20", "--hour", "50")
8563
Expect(err).To(HaveOccurred())
8664
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: [-r|--hour] value must be between 0 and 23."))
8765
})
88-
})
89-
90-
Context("Snapshot enable with wrong -d", func() {
91-
It("return error", func() {
66+
It("Wrong Days of the Week", func() {
9267
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "HOURLY", "-c", "3", "-m", "20", "--hour", "10", "-d", "10")
9368
Expect(err).To(HaveOccurred())
9469
Expect(err.Error()).To(ContainSubstring("Incorrect Usage: [-d|--day-of-week] value must be between 0 and 6."))
@@ -99,22 +74,26 @@ var _ = Describe("Snapshot enable", func() {
9974
BeforeEach(func() {
10075
FakeStorageManager.EnableSnapshotReturns(nil)
10176
})
102-
It("return no error", func() {
77+
It("Happy Path All Options", func() {
10378
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "HOURLY", "-c", "3", "-m", "20", "--hour", "10", "-d", "0")
10479
Expect(err).NotTo(HaveOccurred())
10580
Expect(fakeUI.Outputs()).To(ContainSubstring("HOURLY snapshots have been enabled for volume 1234."))
10681
})
82+
It("Happy Path min Options", func() {
83+
err := testhelpers.RunCobraCommand(cliCommand.Command, "9999", "-s", "INTERVAL", "-c", "500")
84+
Expect(err).NotTo(HaveOccurred())
85+
Expect(fakeUI.Outputs()).To(ContainSubstring("INTERVAL snapshots have been enabled for volume 9999."))
86+
})
10787
})
10888

10989
Context("Snapshot enable with correct parameters but server API call fails", func() {
11090
BeforeEach(func() {
11191
FakeStorageManager.EnableSnapshotReturns(errors.New("Internal Server Error"))
11292
})
113-
It("return error", func() {
93+
It("API error", func() {
11494
err := testhelpers.RunCobraCommand(cliCommand.Command, "1234", "-s", "HOURLY", "-c", "3", "-m", "20", "--hour", "10", "-d", "0")
11595
Expect(err).To(HaveOccurred())
11696
Expect(err.Error()).To(ContainSubstring("Failed to enable HOURLY snapshot for volume 1234."))
117-
11897
})
11998
})
12099
})

‎plugin/i18n/v2Resources/active.en-US.json‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,6 @@
374374
"${COMMAND_NAME} sl {{.storageType}} snapshot-disable VOLUME_ID [OPTIONS]\n\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-disable 12345678 -s DAILY\n This command disables daily snapshot for volume with ID 12345678.": {
375375
"other": "${COMMAND_NAME} sl {{.storageType}} snapshot-disable VOLUME_ID [OPTIONS]\n\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-disable 12345678 -s DAILY\n This command disables daily snapshot for volume with ID 12345678."
376376
},
377-
"${COMMAND_NAME} sl {{.storageType}} snapshot-enable VOLUME_ID [OPTIONS]\n\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-enable 12345678 -s WEEKLY -c 5 -m 0 --hour 2 -d 0\n This command enables snapshot for volume with ID 12345678, snapshot is taken weekly on every Sunday at 2:00, and up to 5 snapshots are retained.": {
378-
"other": "${COMMAND_NAME} sl {{.storageType}} snapshot-enable VOLUME_ID [OPTIONS]\n\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-enable 12345678 -s WEEKLY -c 5 -m 0 --hour 2 -d 0\n This command enables snapshot for volume with ID 12345678, snapshot is taken weekly on every Sunday at 2:00, and up to 5 snapshots are retained."
379-
},
380377
"${COMMAND_NAME} sl {{.storageType}} snapshot-list VOLUME_ID [OPTIONS]\n\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-list 12345678 --sortby id \n This command lists all snapshots of volume with ID 12345678 and sorts them by ID.": {
381378
"other": "${COMMAND_NAME} sl {{.storageType}} snapshot-list VOLUME_ID [OPTIONS]\n\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-list 12345678 --sortby id \n This command lists all snapshots of volume with ID 12345678 and sorts them by ID."
382379
},
@@ -1640,8 +1637,8 @@
16401637
"Date time": {
16411638
"other": "Date time"
16421639
},
1643-
"Day of the week when snapshots should be taken, integer between 0 to 6. \n 0 means Sunday,1 means Monday,2 means Tuesday,3 means Wendesday,4 means Thursday,5 means Friday,6 means Saturday": {
1644-
"other": "Day of the week when snapshots should be taken, integer between 0 to 6. \n 0 means Sunday,1 means Monday,2 means Tuesday,3 means Wendesday,4 means Thursday,5 means Friday,6 means Saturday"
1640+
"Day of the week when snapshots should be taken, integer between 0 to 6. \n\t 0 means Sunday,1 means Monday,2 means Tuesday,3 means Wendesday,4 means Thursday,5 means Friday,6 means Saturday": {
1641+
"other": "Day of the week when snapshots should be taken, integer between 0 to 6. \n\t 0 means Sunday,1 means Monday,2 means Tuesday,3 means Wendesday,4 means Thursday,5 means Friday,6 means Saturday"
16451642
},
16461643
"Dedicated Access": {
16471644
"other": "Dedicated Access"
@@ -4751,8 +4748,8 @@
47514748
"Number of seconds to report as one data point. 300, 600, 1800, 3600 (default), 43200 or 86400 seconds": {
47524749
"other": "Number of seconds to report as one data point. 300, 600, 1800, 3600 (default), 43200 or 86400 seconds"
47534750
},
4754-
"Number of snapshots to retain [required]": {
4755-
"other": "Number of snapshots to retain [required]"
4751+
"Number of snapshots to retain": {
4752+
"other": "Number of snapshots to retain"
47564753
},
47574754
"Number of times before marking as DOWN. [1-10]": {
47584755
"other": "Number of times before marking as DOWN. [1-10]"
@@ -5786,6 +5783,9 @@
57865783
"Security group {{.ID}} is updated.": {
57875784
"other": "Security group {{.ID}} is updated."
57885785
},
5786+
"See https://sldn.softlayer.com/reference/services/SoftLayer_Network_Storage/enableSnapshots/ for more details about these options.\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-enable 12345678 -s WEEKLY -c 5 -m 0 --hour 2 -d 0\n This command enables snapshot for volume with ID 12345678, snapshot is taken weekly on every Sunday at 2:00, and up to 5 snapshots are retained.": {
5787+
"other": "See https://sldn.softlayer.com/reference/services/SoftLayer_Network_Storage/enableSnapshots/ for more details about these options.\nEXAMPLE:\n ${COMMAND_NAME} sl {{.storageType}} snapshot-enable 12345678 -s WEEKLY -c 5 -m 0 --hour 2 -d 0\n This command enables snapshot for volume with ID 12345678, snapshot is taken weekly on every Sunday at 2:00, and up to 5 snapshots are retained."
5788+
},
57895789
"Serial #": {
57905790
"other": "Serial #"
57915791
},
@@ -5993,6 +5993,9 @@
59935993
"Snapshot schedule to use for replication. Options are: HOURLY,DAILY,WEEKLY [required]": {
59945994
"other": "Snapshot schedule to use for replication. Options are: HOURLY,DAILY,WEEKLY [required]"
59955995
},
5996+
"Snapshot schedule, options are: {{.ScheduleTypes}}": {
5997+
"other": "Snapshot schedule, options are: {{.ScheduleTypes}}"
5998+
},
59965999
"Snapshot space not found for original volume, origin snapshot space is needed for duplication": {
59976000
"other": "Snapshot space not found for original volume, origin snapshot space is needed for duplication"
59986001
},
@@ -7361,8 +7364,8 @@
73617364
"[-s|--schedule-type] is required, options are: HOURLY, DAILY, WEEKLY.": {
73627365
"other": "[-s|--schedule-type] is required, options are: HOURLY, DAILY, WEEKLY."
73637366
},
7364-
"[-s|--schedule-type] must be HOURLY, DAILY, or WEEKLY.": {
7365-
"other": "[-s|--schedule-type] must be HOURLY, DAILY, or WEEKLY."
7367+
"[-s|--schedule-type] needs to be one of {{.ScheduleTypes}}, not {{.Selected}}.": {
7368+
"other": "[-s|--schedule-type] needs to be one of {{.ScheduleTypes}}, not {{.Selected}}."
73667369
},
73677370
"[-s|--size] is required.\nRun '{{.CommandName}} sl block volume-options' to get available options.": {
73687371
"other": "[-s|--size] is required.\nRun '{{.CommandName}} sl block volume-options' to get available options."

0 commit comments

Comments
 (0)