Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --or-selector for backup and restore command #6475

Merged
merged 1 commit into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/6475-nilesh-akhade
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `orLabelSelectors` for backup, restore commands
7 changes: 7 additions & 0 deletions pkg/cmd/cli/backup/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ type CreateOptions struct {
ExcludeNamespaceScopedResources flag.StringArray
Labels flag.Map
Selector flag.LabelSelector
OrSelector flag.OrLabelSelector
IncludeClusterResources flag.OptionalBool
Wait bool
StorageLocation string
Expand Down Expand Up @@ -130,6 +131,7 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) {
flags.StringVar(&o.StorageLocation, "storage-location", "", "Location in which to store the backup.")
flags.StringSliceVar(&o.SnapshotLocations, "volume-snapshot-locations", o.SnapshotLocations, "List of locations (at most one per provider) where volume snapshots should be stored.")
flags.VarP(&o.Selector, "selector", "l", "Only back up resources matching this label selector.")
flags.Var(&o.OrSelector, "or-selector", "Backup resources matching at least one of the label selector from the list. Label selectors should be separated by ' or '. For example, foo=bar or app=nginx")
flags.StringVar(&o.OrderedResources, "ordered-resources", "", "Mapping Kinds to an ordered list of specific resources of that Kind. Resource names are separated by commas and their names are in format 'namespace/resourcename'. For cluster scope resource, simply use resource name. Key-value pairs in the mapping are separated by semi-colon. Example: 'pods=ns1/pod1,ns1/pod2;persistentvolumeclaims=ns1/pvc4,ns1/pvc8'. Optional.")
flags.DurationVar(&o.CSISnapshotTimeout, "csi-snapshot-timeout", o.CSISnapshotTimeout, "How long to wait for CSI snapshot creation before timeout.")
flags.DurationVar(&o.ItemOperationTimeout, "item-operation-timeout", o.ItemOperationTimeout, "How long to wait for async plugin operations before timeout.")
Expand Down Expand Up @@ -168,6 +170,10 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto
return err
}

if o.Selector.LabelSelector != nil && o.OrSelector.OrLabelSelectors != nil {
return fmt.Errorf("either a 'selector' or an 'or-selector' can be specified, but not both")
}

client, err := f.KubebuilderWatchClient()
if err != nil {
return err
Expand Down Expand Up @@ -365,6 +371,7 @@ func (o *CreateOptions) BuildBackup(namespace string) (*velerov1api.Backup, erro
IncludedNamespaceScopedResources(o.IncludeNamespaceScopedResources...).
ExcludedNamespaceScopedResources(o.ExcludeNamespaceScopedResources...).
LabelSelector(o.Selector.LabelSelector).
OrLabelSelector(o.OrSelector.OrLabelSelectors).
TTL(o.TTL).
StorageLocation(o.StorageLocation).
VolumeSnapshotLocations(o.SnapshotLocations...).
Expand Down
10 changes: 10 additions & 0 deletions pkg/cmd/cli/backup/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ func TestCreateOptions_BuildBackup(t *testing.T) {
orders, err := ParseOrderedResources(o.OrderedResources)
o.CSISnapshotTimeout = 20 * time.Minute
o.ItemOperationTimeout = 20 * time.Minute
orLabelSelectors := []*metav1.LabelSelector{
{
MatchLabels: map[string]string{"k1": "v1", "k2": "v2"},
},
{
MatchLabels: map[string]string{"a1": "b1", "a2": "b2"},
},
}
o.OrSelector.OrLabelSelectors = orLabelSelectors
assert.NoError(t, err)

backup, err := o.BuildBackup(cmdtest.VeleroNameSpace)
Expand All @@ -58,6 +67,7 @@ func TestCreateOptions_BuildBackup(t *testing.T) {
SnapshotVolumes: o.SnapshotVolumes.Value,
IncludeClusterResources: o.IncludeClusterResources.Value,
OrderedResources: orders,
OrLabelSelectors: orLabelSelectors,
CSISnapshotTimeout: metav1.Duration{Duration: o.CSISnapshotTimeout},
ItemOperationTimeout: metav1.Duration{Duration: o.ItemOperationTimeout},
}, backup.Spec)
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/cli/backup/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func TestNewDescribeCommand(t *testing.T) {

if err == nil {
assert.Contains(t, stdout, "Velero-Native Snapshots: <none included>")
assert.Contains(t, stdout, "Or label selector: <none>")
assert.Contains(t, stdout, fmt.Sprintf("Name: %s", backupName))
return
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/cmd/cli/restore/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ type CreateOptions struct {
StatusExcludeResources flag.StringArray
NamespaceMappings flag.Map
Selector flag.LabelSelector
OrSelector flag.OrLabelSelector
IncludeClusterResources flag.OptionalBool
Wait bool
AllowPartiallyFailed flag.OptionalBool
Expand Down Expand Up @@ -124,6 +125,7 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) {
flags.Var(&o.StatusIncludeResources, "status-include-resources", "Resources to include in the restore status, formatted as resource.group, such as storageclasses.storage.k8s.io.")
flags.Var(&o.StatusExcludeResources, "status-exclude-resources", "Resources to exclude from the restore status, formatted as resource.group, such as storageclasses.storage.k8s.io.")
flags.VarP(&o.Selector, "selector", "l", "Only restore resources matching this label selector.")
flags.Var(&o.OrSelector, "or-selector", "Restore resources matching at least one of the label selector from the list. Label selectors should be separated by ' or '. For example, foo=bar or app=nginx")
flags.DurationVar(&o.ItemOperationTimeout, "item-operation-timeout", o.ItemOperationTimeout, "How long to wait for async plugin operations before timeout.")
f := flags.VarPF(&o.RestoreVolumes, "restore-volumes", "", "Whether to restore volumes from snapshots.")
// this allows the user to just specify "--restore-volumes" as shorthand for "--restore-volumes=true"
Expand Down Expand Up @@ -185,6 +187,10 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto
return errors.New("Velero client is not set; unable to proceed")
}

if o.Selector.LabelSelector != nil && o.OrSelector.OrLabelSelectors != nil {
return errors.New("either a 'selector' or an 'or-selector' can be specified, but not both")
}

if len(o.ExistingResourcePolicy) > 0 && !isResourcePolicyValid(o.ExistingResourcePolicy) {
return errors.New("existing-resource-policy has invalid value, it accepts only none, update as value")
}
Expand Down Expand Up @@ -302,6 +308,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
ExistingResourcePolicy: api.PolicyType(o.ExistingResourcePolicy),
NamespaceMapping: o.NamespaceMappings.Data(),
LabelSelector: o.Selector.LabelSelector,
OrLabelSelectors: o.OrSelector.OrLabelSelectors,
RestorePVs: o.RestoreVolumes.Value,
PreserveNodePorts: o.PreserveNodePorts.Value,
IncludeClusterResources: o.IncludeClusterResources.Value,
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/cli/schedule/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
ExcludedNamespaceScopedResources: o.BackupOptions.ExcludeNamespaceScopedResources,
IncludeClusterResources: o.BackupOptions.IncludeClusterResources.Value,
LabelSelector: o.BackupOptions.Selector.LabelSelector,
OrLabelSelectors: o.BackupOptions.OrSelector.OrLabelSelectors,
SnapshotVolumes: o.BackupOptions.SnapshotVolumes.Value,
TTL: metav1.Duration{Duration: o.BackupOptions.TTL},
StorageLocation: o.BackupOptions.StorageLocation,
Expand Down
61 changes: 61 additions & 0 deletions pkg/cmd/util/flag/orlabelselector.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
Copyright 2017 the Velero contributors.
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 flag

import (
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// OrLabelSelector is a Cobra-compatible wrapper for defining
// a Kubernetes or-label-selector flag.
type OrLabelSelector struct {
OrLabelSelectors []*metav1.LabelSelector
}

// String returns a string representation of the or-label
// selector flag.
func (ls *OrLabelSelector) String() string {
orLabels := []string{}
for _, v := range ls.OrLabelSelectors {
orLabels = append(orLabels, metav1.FormatLabelSelector(v))
}
return strings.Join(orLabels, " or ")
}

// Set parses the provided string and assigns the result
// to the or-label-selector receiver. It returns an error if
// the string is not parseable.
func (ls *OrLabelSelector) Set(s string) error {
orItems := strings.Split(s, " or ")
ls.OrLabelSelectors = make([]*metav1.LabelSelector, 0)
for _, orItem := range orItems {
parsed, err := metav1.ParseToLabelSelector(orItem)
if err != nil {
return err
}
ls.OrLabelSelectors = append(ls.OrLabelSelectors, parsed)
}
return nil
}

// Type returns a string representation of the
// OrLabelSelector type.
func (ls *OrLabelSelector) Type() string {
return "orLabelSelector"
}
102 changes: 102 additions & 0 deletions pkg/cmd/util/flag/orlabelselector_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package flag

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestStringOfOrLabelSelector(t *testing.T) {
tests := []struct {
name string
orLabelSelector *OrLabelSelector
expectedStr string
}{
{
name: "or between two labels",
orLabelSelector: &OrLabelSelector{
OrLabelSelectors: []*metav1.LabelSelector{
{
MatchLabels: map[string]string{"k1": "v1"},
},
{
MatchLabels: map[string]string{"k2": "v2"},
},
},
},
expectedStr: "k1=v1 or k2=v2",
},
{
name: "or between two label groups",
orLabelSelector: &OrLabelSelector{
OrLabelSelectors: []*metav1.LabelSelector{
{
MatchLabels: map[string]string{"k1": "v1", "k2": "v2"},
},
{
MatchLabels: map[string]string{"a1": "b1", "a2": "b2"},
},
},
},
expectedStr: "k1=v1,k2=v2 or a1=b1,a2=b2",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.expectedStr, test.orLabelSelector.String())
})
}
}

func TestSetOfOrLabelSelector(t *testing.T) {
tests := []struct {
name string
inputStr string
expectedSelector *OrLabelSelector
}{
{
name: "or between two labels",
inputStr: "k1=v1 or k2=v2",
expectedSelector: &OrLabelSelector{
OrLabelSelectors: []*metav1.LabelSelector{
{
MatchLabels: map[string]string{"k1": "v1"},
},
{
MatchLabels: map[string]string{"k2": "v2"},
},
},
},
},
{
name: "or between two label groups",
inputStr: "k1=v1,k2=v2 or a1=b1,a2=b2",
expectedSelector: &OrLabelSelector{
OrLabelSelectors: []*metav1.LabelSelector{
{
MatchLabels: map[string]string{"k1": "v1", "k2": "v2"},
},
{
MatchLabels: map[string]string{"a1": "b1", "a2": "b2"},
},
},
},
},
}
selector := &OrLabelSelector{}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require.Nil(t, selector.Set(test.inputStr))
assert.Equal(t, len(test.expectedSelector.OrLabelSelectors), len(selector.OrLabelSelectors))
assert.Equal(t, test.expectedSelector.String(), selector.String())
})
}
}

func TestTypeOfOrLabelSelector(t *testing.T) {
selector := &OrLabelSelector{}
assert.Equal(t, "orLabelSelector", selector.Type())
}
12 changes: 12 additions & 0 deletions pkg/cmd/util/output/backup_describer.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,18 @@ func DescribeBackupSpec(d *Describer, spec velerov1api.BackupSpec) {
}
d.Printf("Label selector:\t%s\n", s)

d.Println()
if len(spec.OrLabelSelectors) == 0 {
s = emptyDisplay
} else {
orLabelSelectors := []string{}
for _, v := range spec.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, metav1.FormatLabelSelector(v))
}
s = strings.Join(orLabelSelectors, " or ")
}
d.Printf("Or label selector:\t%s\n", s)

d.Println()
d.Printf("Storage Location:\t%s\n", spec.StorageLocation)

Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/util/output/backup_describer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ Resources:

Label selector: <none>

Or label selector: <none>

Storage Location: backup-location

Velero-Native Snapshot PVs: auto
Expand Down Expand Up @@ -153,6 +155,8 @@ Resources:

Label selector: <none>

Or label selector: <none>

Storage Location: backup-location

Velero-Native Snapshot PVs: auto
Expand Down Expand Up @@ -208,6 +212,8 @@ Resources:

Label selector: <none>

Or label selector: <none>

Storage Location: backup-location

Velero-Native Snapshot PVs: auto
Expand Down
12 changes: 12 additions & 0 deletions pkg/cmd/util/output/restore_describer.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,18 @@ func DescribeRestore(ctx context.Context, kbClient kbclient.Client, restore *vel
}
d.Printf("Label selector:\t%s\n", s)

d.Println()
if len(restore.Spec.OrLabelSelectors) == 0 {
s = emptyDisplay
} else {
orLabelSelectors := []string{}
for _, v := range restore.Spec.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, metav1.FormatLabelSelector(v))
}
s = strings.Join(orLabelSelectors, " or ")
}
d.Printf("Or label selector:\t%s\n", s)

d.Println()
d.Printf("Restore PVs:\t%s\n", BoolPointerString(restore.Spec.RestorePVs, "false", "true", "auto"))

Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/util/output/schedule_describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ Backup Template:

Label selector: <none>

Or label selector: <none>

Storage Location:

Velero-Native Snapshot PVs: auto
Expand Down Expand Up @@ -82,6 +84,8 @@ Backup Template:

Label selector: <none>

Or label selector: <none>

Storage Location:

Velero-Native Snapshot PVs: auto
Expand Down
18 changes: 18 additions & 0 deletions site/content/docs/main/resource-filtering.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,24 @@ Includes cluster-scoped resources. Cannot work with `--include-cluster-scoped-re

For more information read the [Kubernetes label selector documentation](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking this docs, couldn't we achieve the same result using the --selector flag with in?

Ex.: Do these commands produce the same result?

  • velero backup create nginx-backup --selector "app in (nginx,my-nginx,nginx-example)"
  • velero backup create nginx-backup --or-selector "app=nginx|app=my-nginx|app=nginx-example"

Did not try every possibility, but maybe this PR can be achieve only adding examples to the docs with different scenarios (and updating help description of --selector flag in CLI)

Copy link
Contributor

Choose a reason for hiding this comment

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

But I think the usage from the docs is limited. For example, if I have 2 (or more) different labels (KEYs), I can't do an OR with them ("Similarly the comma separator acts as an AND operator.")

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mateusoliveira43 yes -- that's the sort of scenario the or selector was added for:
velero backup create nginx-backup --or-selector "app=nginx|foo=bar"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If resources to be backed up are labeled either env=prod or environment=production, then we cannot use --selector env=prod,environment=production. But we can use --or-selector "env=prod|environment=production"

I have updated the PR to include examples and explanations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have modified the syntax a little bit. The or keyword can be used in place of | pipe symbol.


### --or-selector

To include the resources that match at least one of the label selectors from the list. Separate the selectors with ` or `. The ` or ` is used as a separator to split label selectors, and it is not an operator.

This option cannot be used together with `--selector`.

* Include resources matching any one of the label selector, `foo=bar` or `baz=qux`

```bash
velero backup create backup1 --or-selector "foo=bar or baz=qux"
```

* Include resources that are labeled `environment=production` or `env=prod` or `env=production` or `environment=prod`.

```bash
velero restore create restore-prod --from-backup=prod-backup --or-selector "env in (prod,production) or environment in (prod, production)"
```

### --include-cluster-scoped-resources
Kubernetes cluster-scoped resources to include in the backup, formatted as resource.group, such as `storageclasses.storage.k8s.io`(use '*' for all resources). Cannot work with `--include-resources`, `--exclude-resources` and `--include-cluster-resources`. This parameter only works for backup, not for restore.

Expand Down
Loading