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 new resource filters can separate cluster and namespace scope res… #5838

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

blackpiglet
Copy link
Contributor

…ources.

Signed-off-by: Xun Jiang [email protected]

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #5120

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@blackpiglet blackpiglet self-assigned this Feb 7, 2023
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch from 1858f26 to 3132d92 Compare February 7, 2023 08:10
@anshulahuja98
Copy link
Collaborator

Since this PR also involves CR changes will this also not go in v1.11?

@blackpiglet
Copy link
Contributor Author

Since this PR also involves CR changes will this also not go in v1.11?

Yes. This is planned to delivered in v1.11, then the CRD change is not also a blocker for your design proposal.

@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 2 times, most recently from ac4455a to 091b265 Compare February 8, 2023 12:45
@codecov-commenter
Copy link

Codecov Report

Merging #5838 (091b265) into main (5db9437) will increase coverage by 0.09%.
The diff coverage is 50.19%.

@@            Coverage Diff             @@
##             main    #5838      +/-   ##
==========================================
+ Coverage   40.73%   40.83%   +0.09%     
==========================================
  Files         246      247       +1     
  Lines       21383    21723     +340     
==========================================
+ Hits         8711     8871     +160     
- Misses      12041    12213     +172     
- Partials      631      639       +8     
Impacted Files Coverage Δ
pkg/backup/request.go 100.00% <ø> (ø)
pkg/builder/backup_builder.go 0.00% <0.00%> (ø)
pkg/cmd/util/output/backup_describer.go 0.00% <0.00%> (ø)
pkg/cmd/cli/backup/create.go 21.48% <15.15%> (-0.84%) ⬇️
pkg/util/collections/includes_excludes.go 60.40% <44.77%> (-8.06%) ⬇️
pkg/backup/item_collector.go 59.05% <69.76%> (-0.83%) ⬇️
pkg/controller/backup_controller.go 56.43% <77.77%> (+0.78%) ⬆️
pkg/backup/item_backupper.go 77.32% <79.48%> (+0.04%) ⬆️
pkg/backup/backup.go 79.57% <100.00%> (+0.69%) ⬆️
pkg/util/kube/predicate.go 49.09% <0.00%> (-16.77%) ⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 3 times, most recently from 7602577 to 7e936b4 Compare February 9, 2023 11:32
@blackpiglet
Copy link
Contributor Author

blackpiglet commented Feb 9, 2023

Tests done:

  • check velero backup create -h has the new filters
  • check velero schedule create -h has the new filters
  • velero schedule create test --schedule="1 1 * * *" --include-namespaced-resources=configmaps,services --include-cluster-scope-resources=storageclasses
  • Old and new filters cannot be used together in backup.
    velero backup create test --include-namespaced-resources=configmaps,services --include-cluster-scope-resources=storageclasses --include-cluster-resources=true An error occurred: include-resources, exclude-resources and include-cluster-resources are old filter parameters. include-cluster-scope-resources, exclude-cluster-scope-resources, include-namespaced-resources and exclude-namespaced-resources are new new filter parameters. They cannot be used together
apiVersion: velero.io/v1
kind: Backup
metadata:
  labels:
    velero.io/storage-location: default
  name: test9
  namespace: velero
spec:
  csiSnapshotTimeout: 10m0s
  defaultVolumesToFsBackup: false
  excludedClusterScopeResources:
  - clusterroles
  - services
  hooks: {}
  includedResources:
  - configmaps
  includedClusterScopeResources:
  - services
  - pod
  includedNamespaces:
  - '*'
  metadata: {}
  storageLocation: default
  ttl: 720h0m0s
  volumeSnapshotLocations:
  - default
  • Old and new filters cannot be used together in schedule.
    velero schedule create test --schedule="1 1 * * *" --include-namespaced-resources=configmaps,services --include-cluster-scope-resources=storageclasses --include-cluster-resources=true An error occurred: include-resources, exclude-resources and include-cluster-resources are old filter parameters. include-cluster-scope-resources, exclude-cluster-scope-resources, include-namespaced-resources and exclude-namespaced-resources are new new filter parameters. They cannot be used together

  • Backup describe command should show the new filters.
    IncludedClusterScopeResources's default value is <none>.
    ExcludedClusterScopeResources's default value is <none>.
    IncludedNamespacedResources's default value is *.
    ExcludedNamespacedResourcess default value is <none>.

  • Create a backup, and set the four new filters with values. The filters should have the value set correctly, and the resources are also filtered correctly.
    velero backup create test1 --include-namespaced-resources="deployments.apps" --exclude-namespaced-resources="services" --include-cluster-scope-resources="persistentvolumes" --exclude-cluster-scope-resources="storageclasses"

  • namespaced resource filters contain cluster-scoped resources. They should be ignored.
    velero backup create test2 --include-namespaced-resources=storageclasses,deployments.apps --include-cluster-scope-resources=clusterroles

  • cluster-scoped resource filters contain namespace resources. They should be ignored.
    velero backup create test3 --include-namespaced-resources=pods --include-cluster-scope-resources=clusterroles,services

  • cluster-scoped resources include and exclude filters that have some overlap.

    • exclude and include filters are comma-separated strings. exclude contains the elements in the include. Validation fails.
    • exclude is an asterisk. include filter has value. Validation fails.
    • exclude is a comma-separated string. include filter is an asterisk. resources are all backed up, except the ones specified in the exclude filter.
      velero backup create test8 --include-cluster-scope-resources="*" --exclude-cluster-scope-resources="clusterroles"
  • namespace resources include and exclude filters that have some overlap.

    • exclude and include filters are comma-separated strings. exclude contains the elements in the include. Validation fails.
      velero backup create test1 --include-namespaced-resources="pods,services" --exclude-namespaced-resources="pods"
    • exclude is an asterisk. include filter has value. Validation fails.
      velero backup create test2 --include-namespaced-resources="*" --exclude-namespaced-resources="*"
    • exclude is a comma-separated string. include filter is an asterisk. resources are all backed up, except the ones specified in the exclude filter.
      velero backup create test3 --include-namespaced-resources="*" --exclude-namespaced-resources="pods,roles" --include-namespaces=default
  • Check all new filters' asterisk value function.

    • --include-namespaced-resources="*" is same as not setting value. It means including all namesapced resources
      velero backup create test3 --include-namespaced-resources="*" --exclude-namespaced-resources="pods,roles" --include-namespaces=default
      velero backup create test2 --include-namespaces=default
    • --exclude-namespaced-resources=* means all namespace resources are excluded.
      velero backup create test1 --exclude-namespaced-resources="*"
    • --include-cluster-scope-resources=* means including all cluster-scoped resources.
      velero backup create test4 --include-cluster-scope-resources="*" --exclude-namespaced-resources="*"
    • --exclude-cluster-scope-resources=* means excluding all cluster-scoped resources.
      velero backup create test6 --exclude-cluster-scope-resources="*" --include-namespaces=kube-public
    • both --exclude-cluster-scope-resources and --exclude-namespace-resources are set to asterisk. It means all resources are excluded, but Velero treats k8s namespace resources differently. The command will only include the namespaces.
      velero backup create test5 --exclude-cluster-scope-resources="*" --exclude-namespaced-resources="*"
    • if --exclude-namespaced-resources="", --exclude-cluster-scope-resources="" and --exclude-namespaces specified all namespaces, the backup will include nothing, and the backup is still successful.
  • Check missing namespace filter means including all namespace resources.
    velero backup create test3 --include-namespaced-resources="*" --exclude-namespaced-resources="pods,roles" --include-namespaces=default
    velero backup create test7 --exclude-namespaced-resources="pods,roles" --include-namespaces=default

  • Check cluster scope filters default value's functions.

    • --include-namespaced-resources is not set means including all namespace resource types.
    • --exclude-namespaced-resources is not set means no namespace resource type is excluded.
    • --include-cluster-scope-resources and --exclude-cluster-scope-resources are both not set. That means only related cluster-scoped resources are included.
    • --include-cluster-scope-resources is not set means no additional cluster-scoped resource is included.
    • --exclude-cluster-scope-resources is not set means no additional cluster-scoped resource is excluded.
  • Check no resource filters are included in the backup. That should not change the existing behavior.
    velero backup create test9
    velero backup create test10 --include-cluster-scope-resources="*"
    The resource lists of those two backups should be the same.

  • Using scenarios in the design document.

    • no namespace resource + some cluster-scoped resource: velero backup create no-some --exclude-namespaced-resources="*" --include-cluster-scope-resources=storageclass, only StorageClass and Namespace resources are included.
    • no namespace resource + all cluster-scoped resource: velero backup create no-all --exclude-namespaced-resources="*" --include-cluster-scope-resources="*". All cluster-scoped resources are included.
    • some namespace resource + no cluster-scoped resource first scenario: velero backup create some-no-1 --include-namespaces=kube-public --exclude-cluster-scope-resources="*". All namespace resources in namespace kube-public are included.
    • some namespace resource + no cluster-scoped resource second scenario: velero backup create some-no-2 --include-namespaced-resources=configmaps --exclude-cluster-scope-resources="*". All ConfigMap and Namespace resources are included.
    • some namespace resource + no cluster-scoped resource third scenario: velero backup create some-no-3 --include-namespaced-resources=configmaps --include-namespaces=kube-public --exclude-cluster-scope-resources="*". ConfigMap in namespace kube-public and the Namespace resources are included.
    • some namespace resource + no cluster-scoped resource fourth scenario: velero backup create some-no-4 --exclude-namespaced-resources=configmaps --include-namespaces=kube-public --exclude-cluster-scope-resources="*". Resources in namespace kube-public, except ConfigMap are the Namespace resources are included.
    • some namespace resource + some related cluster-scoped resource first scenario: velero backup create some-related-1 --include-namespaces=default. All namespace resource in namespace default, the Namespace and possible related PVs are included.
    • some namespace resource + some related cluster-scoped resource second scenario: velero backup create some-related-2 --include-namespaces=default --include-namespaced-resources=persistentvolumeclaim,deployment,service,endpoint,pod,replicaset. persistentvolumeclaim, deployment, service, endpoint, pod, replicaset in namespace default, the Namespace and possible related PVs are included.
    • some namespace resource + some related cluster-scoped resource third scenario: velero backup create some-related-3 --include-namespaces=default --exclude-namespaced-resources=configmaps,services. All namespace resources in namespace default, and the Namespace resource, except ConfigMap and Service are included.
    • some namespaced resources + some additional cluster resources first scenario: velero backup create some-additional-1 --include-namespaces=kube-public --include-cluster-scope-resources=storageclasses. All namespace resources in namespace kube-public, Namespace resource and the StorageClass resources are included.
    • some namespaced resources + some additional cluster resources second scenario: velero backup create some-additional-2 --include-namespaced-resources=configmaps --include-cluster-scope-resources=storageclasses. All ConfigMap and Namespace resources and the StorageClass resources are included.
    • some namespaced resources + some additional cluster resources third scenario: velero backup create some-additional-3 --include-namespaced-resources=persistentvolumeclaims --include-namespaces=default --include-cluster-scope-resources=storageclasses. All PVCs in namespace default, the Namespace resource, all StorageClasses, and possible related PVs are included.
    • some namespaced resources + some additional cluster resources fourth scenario: ``.
    • some namespaced resources + all cluster resources first scenario: velero backup create some-all-1 --include-namespaces=kube-public --include-cluster-scope-resources="*". All namespace resources in kube-public, the Namespace resource, and all the cluster-scoped resources are included.
    • some namespaced resources + all cluster resources second scenario: velero backup create some-all-2 --include-namespaced-resources=configmaps --include-cluster-scope-resources="*". All cluster-scoped resources, all Namespaces, and all ConfigMaps are included.
    • some namespaced resources + all cluster resources third scenario: velero backup create some-all-3 --include-namespaces=default --include-namespaced-resources=configmaps --include-cluster-scope-resources="*". All cluster-scoped resources, the ConfigMap in specified namespaces, and the Namespaces resources are included.
    • all namespaced resources + no cluster resources: velero backup create all-no --exclude-cluster-scope-resources="*". All namespace resources and no cluster-scoped resources are included.
    • all namespaced resources + some additional cluster resources:
    • all namespaced resources + all cluster resources:
  • Basic E2E test.
    https://gist.github.com/blackpiglet/6debcb5507122ad138a369dcba8d4ada

@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch from 7e936b4 to 0865822 Compare February 9, 2023 12:43
@blackpiglet blackpiglet marked this pull request as ready for review February 10, 2023 02:53
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch from 0865822 to c2a9cef Compare February 10, 2023 02:58
@github-actions github-actions bot added the Area/Design Design Documents label Feb 10, 2023
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 2 times, most recently from 5b37173 to ac0d70e Compare February 10, 2023 08:43
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch from ac0d70e to 57b5eda Compare February 10, 2023 08:58
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 4 times, most recently from 0eb4daa to 392dd8a Compare February 27, 2023 12:59
pkg/backup/backup.go Outdated Show resolved Hide resolved
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 3 times, most recently from cb9f25b to d433bf0 Compare February 28, 2023 14:59
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 2 times, most recently from 53147d1 to 59cd4ad Compare March 2, 2023 07:17
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 6 times, most recently from 1dba935 to e21d086 Compare March 10, 2023 02:31
pkg/backup/item_backupper.go Outdated Show resolved Hide resolved
pkg/util/collections/includes_excludes.go Outdated Show resolved Hide resolved
pkg/util/collections/includes_excludes.go Outdated Show resolved Hide resolved
@blackpiglet blackpiglet force-pushed the add_new_resource_filters branch 2 times, most recently from 9bc21dc to 350e93d Compare March 13, 2023 12:09
reasonerjt
reasonerjt previously approved these changes Mar 13, 2023
Copy link
Contributor

@reasonerjt reasonerjt left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support cluster resource Includes/Excludes
5 participants