Improve TTL time format explanation and usage

[juanpsm]

Description

When creating a schedule with a TTL I noticed some inconsistencies. First, I couldn't find any resources where to check the accepted format for the TTL string. With kubectl explain schedule.spec.template.ttl I found that it is a time.Duration-parseable string.

GO's time.Duration ParseDuration function says valid time units are "ns", "us", "ms", "s", "m", "h", "d", "w", "y". But when I tried to use "d", "w" or "y" I got the following error:

❯ velero schedule create test --schedule "0 0 * * 0" --ttl 1w
An error occurred: invalid argument "1w" for "--ttl" flag: time: unknown unit "w" in duration "1w"

If instead of using velero cli, I use kubectl, it gets created but then I can't retrieve anything using velero schedule get:

❯ cat <<EOF | kubectl create -f -
apiVersion: velero.io/v1
kind: Schedule
metadata:
  name: test
  namespace: velero
spec:
  schedule: "0 0 * * 0"
  template:
    ttl: 1w
EOF
schedule.velero.io/test created
❯ velero schedule get
An error occurred: time: unknown unit "w" in duration "1w"

That persists until the schedule is deleted.

Solution I'd like

I think that days, weeks and years should be valid units for the TTL string. It's somewhat weird that units like µs (microseconds) are accepted:

❯ velero schedule create test --schedule "0 0 * * 0" --ttl 5µs
Schedule "test" created successfully.
❯ velero schedule get
NAME  STATUS   CREATED     SCHEDULE   BACKUP TTL  LAST BACKUP  SELECTOR  PAUSED
test  Enabled  2023-07-26  0 0 * * 0  5µs         n/a          <none>    false

I don`t see the use case of such a short lived backup. Weeks, months and years are much useful for this kind of application, and it would provide more readability to the schedule.

Also, it would be nice to have a resource where to check the accepted format, because even if it's briefly mentioned in How Velero Works it is not consistent with what the cli and kubectl shows.

There it says that the default value for the TTL is 30 days (I assume 720h), but when a schedule is created without the --ttl flag, the TTL is set to 0s.

# velero cli
❯ velero schedule create test --schedule "0 0 * * 0"
Schedule "test" created successfully.
# kubectl
❯ cat <<EOF | kubectl create -f -
apiVersion: velero.io/v1
kind: Schedule
metadata:
  name: test2
  namespace: velero
spec:
  schedule: "0 0 * * 0"
  template:
    snapshotVolumes: false
EOF
❯ velero schedule get
NAME   STATUS   CREATED     SCHEDULE   BACKUP TTL  LAST BACKUP  SELECTOR  PAUSED
test   Enabled  2023-07-26  0 0 * * 0  0s          n/a          <none>    false
test2  Enabled  2023-07-26  0 0 * * 0  0s          n/a          <none>    false

Additional comments

Missing other parameters, like --schedule, results in a validation error that prevents the schedule from being created:

# velero cli
❯ velero schedule create test
An error occurred: --schedule is required
# kubectl
cat <<EOF | kubectl create -f -
apiVersion: velero.io/v1
kind: Schedule
metadata:
  name: test
  namespace: velero
spec:
  template:
    ttl: 8h
EOF
error: error validating "STDIN": error validating data: ValidationError(Schedule.spec): missing required field "schedule" in io.velero.v1.Schedule.spec; if you choose to ignore these errors, turn validation off with --validate=false

Even passing an incorrect value results in a failed validation that is explained in the schedule's description:

# velero cli
❯ velero schedule create test --schedule foo
Schedule "test" created successfully.
# kubectl
❯ cat <<EOF | kubectl create -f -
apiVersion: velero.io/v1
kind: Schedule
metadata:
  name: test2
  namespace: velero
spec:
  schedule: foo
  template:
    ttl: 8h
EOF
schedule.velero.io/test created

❯ velero schedule get
NAME   STATUS            CREATED     SCHEDULE  BACKUP TTL  LAST BACKUP  SELECTOR  PAUSED
test   FailedValidation  2023-07-26  foo       0s          n/a          <none>    false
test2  FailedValidation  2023-07-26  foo       8h0m0s      n/a          <none>    false

❯ velero schedule describe test
Name:         test
Namespace:    velero
Labels:       <none>
Annotations:  <none>

Phase:  FailedValidation

Validation errors:  invalid schedule: Expected exactly 5 fields, found 1: foo

Paused:  false
...

Besides, schedule field even recognizes the (non-standard) @daily, @weekly, etc. expressions, which is great:

# velero cli
❯ velero schedule create test --schedule @yearly --ttl 8h
Schedule "test" created successfully.
# kubectl
❯ cat <<EOF | kubectl create -f -
apiVersion: velero.io/v1
kind: Schedule
metadata:
  name: test2
  namespace: velero
spec:
  schedule: "@weekly"
  template:
    ttl: 8h
EOF
schedule.velero.io/test2 created
❯ velero schedule get
NAME   STATUS   CREATED     SCHEDULE  BACKUP TTL  LAST BACKUP  SELECTOR  PAUSED
test   Enabled  2023-07-26  @yearly   8h0m0s      n/a          <none>    false
test2  Enabled  2023-07-26  @weekly   8h0m0s      n/a          <none>    false

Environment:

• Velero version: 1.11.0
• Velero features: <NOT SET>
• Kubernetes version: 1.24.0
• Kubernetes installer & version: OKD 4.11
• Cloud provider or hardware configuration: vSphere 7.0.3
• OS: Pop!_OS 22.04 LTS

[yanggangtony]

The api.Schedule struct 's ttl define is above.

[yanggangtony]

And the flag parse the ttl is:

The ttl define is

[yanggangtony]

There maybe a time cast in k8s api time type and the golang 's time .

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. If a Velero team member has requested log or more information, please provide the output of the shared commands.

[github-actions[bot]]

This issue was closed because it has been stalled for 14 days with no activity.

[pandvan]

This is still an issue

[blackpiglet]

There is a in-progress PR on this topic. https://github.com/vmware-tanzu/velero/pull/7793

[kaovilai]

@blackpiglet #7793 is only for timezone for cron for schedule. Not related to duration for TTL.

[blackpiglet]

@kaovilai Thanks, then I reopen this issue.

[joekhoobyar]

this is still an issue for me

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. If a Velero team member has requested log or more information, please provide the output of the shared commands.

[kaovilai]

unstale

[kaovilai]

After debugging, the unitMap used by time.ParseDuration is defined as

var unitMap = map[string]uint64{
    "ns": uint64(Nanosecond),
    "us": uint64(Microsecond),
    "µs": uint64(Microsecond), // U+00B5 = micro symbol
    "μs": uint64(Microsecond), // U+03BC = Greek letter mu
    "ms": uint64(Millisecond),
    "s":  uint64(Second),
    "m":  uint64(Minute),
    "h":  uint64(Hour),
}

from /go/1.23.2/libexec/src/time/format.go (https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/time/format.go;l=1601-1610)

Browsing go.dev, the time ParseDuration actualy do not support the "w" format indicated. https://pkg.go.dev/time#ParseDuration

What @juanpsm linked is from https://pkg.go.dev/maze.io/x/duration#ParseDuration which is an entirely different time package.

I think this issue can be considered invalid, unless we want to adopt a different time package and convert it to go's built-ins "time".