From feb411cc3f8cc49b9c1505e5ac625401e24f60b4 Mon Sep 17 00:00:00 2001 From: Ming Date: Tue, 14 Jun 2022 06:47:18 +0000 Subject: [PATCH 01/14] Add more explanation for gen-docs parameters Signed-off-by: Ming --- site/content/docs/main/release-instructions.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/site/content/docs/main/release-instructions.md b/site/content/docs/main/release-instructions.md index 608b44168..c8eb317dd 100644 --- a/site/content/docs/main/release-instructions.md +++ b/site/content/docs/main/release-instructions.md @@ -67,7 +67,9 @@ For each major or minor release, create and publish a blog post to let folks kno - Run `make gen-docs`, passing the appropriate variables. Examples: a) `VELERO_VERSION=v1.5.0-rc.1 NEW_DOCS_VERSION=v1.5.0-rc.1 make gen-docs`. b) `VELERO_VERSION=v1.5.0 NEW_DOCS_VERSION=v1.5 make gen-docs`). - - Note: `PREVIOUS_DOCS_VERSION=` is optional; when not set, it will default to the latest doc version. + - Note: + - `PREVIOUS_DOCS_VERSION=` is optional; when not set, it will default to the latest doc version. + - `VELERO_VERSION` and `NEW_DOCS_VERSION` are slightly different, the `VELERO_VERSION` may have lots of small release versions for one specific $major.minor, such as 'v1.5.0' and 'v1.5.1', but `NEW_DOCS_VERSION` may still be 'v1.5' for not document update. 1. Clean up when there is an existing set of pre-release versioned docs for the version you are releasing - Example: `site/content/docs/v1.5.0-beta.1` exists, and you're releasing `v1.5.0-rc.1` or `v1.5` - Remove the directory containing the pre-release docs, i.e. `site/content/docs/`. From 3bb6252d152097ac5e61edeb333385feb67bfaba Mon Sep 17 00:00:00 2001 From: Ming Date: Tue, 14 Jun 2022 06:08:56 +0000 Subject: [PATCH 02/14] Pin the base image and golang image for v1.9.0 release Signed-off-by: Ming --- Dockerfile | 4 ++-- hack/build-image/Dockerfile | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8ffe8abff..8e3b99fd8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,7 +11,7 @@ # 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. -FROM --platform=$BUILDPLATFORM golang:1.17 as builder-env +FROM --platform=$BUILDPLATFORM golang:1.17.11 as builder-env ARG GOPROXY ARG PKG @@ -50,7 +50,7 @@ RUN mkdir -p /output/usr/bin && \ go build -o /output/${BIN} \ -ldflags "${LDFLAGS}" ${PKG}/cmd/${BIN} -FROM gcr.io/distroless/base-debian11:nonroot +FROM gcr.io/distroless/base-debian11@sha256:e672eb713e56feb13e349773973b81b1b9284f70b15cf18d1a09ad31a03abe59 LABEL maintainer="Nolan Brubaker " diff --git a/hack/build-image/Dockerfile b/hack/build-image/Dockerfile index 5f3564631..8948080d0 100644 --- a/hack/build-image/Dockerfile +++ b/hack/build-image/Dockerfile @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM golang:1.17 +FROM golang:1.17.11 ARG GOPROXY From b9fe1539f076198588e13efbccc71192b998d072 Mon Sep 17 00:00:00 2001 From: danfengl Date: Tue, 21 Jun 2022 07:02:08 +0000 Subject: [PATCH 03/14] Wait for pod running status before timeout despite errors of accessing API server Signed-off-by: danfengl --- Dockerfile | 4 ++-- hack/build-image/Dockerfile | 2 +- test/e2e/util/k8s/common.go | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8e3b99fd8..8ffe8abff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,7 +11,7 @@ # 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. -FROM --platform=$BUILDPLATFORM golang:1.17.11 as builder-env +FROM --platform=$BUILDPLATFORM golang:1.17 as builder-env ARG GOPROXY ARG PKG @@ -50,7 +50,7 @@ RUN mkdir -p /output/usr/bin && \ go build -o /output/${BIN} \ -ldflags "${LDFLAGS}" ${PKG}/cmd/${BIN} -FROM gcr.io/distroless/base-debian11@sha256:e672eb713e56feb13e349773973b81b1b9284f70b15cf18d1a09ad31a03abe59 +FROM gcr.io/distroless/base-debian11:nonroot LABEL maintainer="Nolan Brubaker " diff --git a/hack/build-image/Dockerfile b/hack/build-image/Dockerfile index 8948080d0..5f3564631 100644 --- a/hack/build-image/Dockerfile +++ b/hack/build-image/Dockerfile @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM golang:1.17.11 +FROM golang:1.17 ARG GOPROXY diff --git a/test/e2e/util/k8s/common.go b/test/e2e/util/k8s/common.go index 43812a74a..d562c75b3 100644 --- a/test/e2e/util/k8s/common.go +++ b/test/e2e/util/k8s/common.go @@ -62,7 +62,9 @@ func WaitForPods(ctx context.Context, client TestClient, namespace string, pods for _, podName := range pods { checkPod, err := client.ClientGo.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{}) if err != nil { - return false, errors.WithMessage(err, fmt.Sprintf("Failed to verify pod %s/%s is %s", namespace, podName, corev1api.PodRunning)) + //Should ignore "etcdserver: request timed out" kind of errors, try to get pod status again before timeout. + fmt.Println(errors.Wrap(err, fmt.Sprintf("Failed to verify pod %s/%s is %s, try again...", namespace, podName, corev1api.PodRunning))) + return false, nil } // If any pod is still waiting we don't need to check any more so return and wait for next poll interval if checkPod.Status.Phase != corev1api.PodRunning { From cb273ae46991a72149efc34c0634a90072b0298b Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Wed, 22 Jun 2022 10:38:28 +0800 Subject: [PATCH 04/14] Delete opened issues triage action. Because the column and project specified by this action do not exist anymore, and Velero team doesn't use this action to assign issue and triage anymore, remove this action. Signed-off-by: Xun Jiang --- .github/workflows/opened-issues-triage.yml | 15 --------------- changelogs/unreleased/5041-jxun | 1 + 2 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 .github/workflows/opened-issues-triage.yml create mode 100644 changelogs/unreleased/5041-jxun diff --git a/.github/workflows/opened-issues-triage.yml b/.github/workflows/opened-issues-triage.yml deleted file mode 100644 index f81443a3a..000000000 --- a/.github/workflows/opened-issues-triage.yml +++ /dev/null @@ -1,15 +0,0 @@ -name: Move new issues into Triage - -on: - issues: - types: [opened] - -jobs: - automate-project-columns: - runs-on: ubuntu-latest - steps: - - uses: alex-page/github-project-automation-plus@v0.3.0 - with: - project: "Velero Support Board" - column: "New" - repo-token: ${{ secrets.GH_TOKEN }} diff --git a/changelogs/unreleased/5041-jxun b/changelogs/unreleased/5041-jxun new file mode 100644 index 000000000..eeb5524f7 --- /dev/null +++ b/changelogs/unreleased/5041-jxun @@ -0,0 +1 @@ +Delete opened issues triage action. \ No newline at end of file From 0470c961bf24f7b20c18c6fc36ec66aab8d79266 Mon Sep 17 00:00:00 2001 From: Niu Lechuan Date: Mon, 27 Jun 2022 10:11:47 +0800 Subject: [PATCH 05/14] Fix typo in doc. 'Mamespace' to 'Namespace' Signed-off-by: Niu Lechuan --- site/content/docs/main/restore-reference.md | 2 +- site/content/docs/v1.9/restore-reference.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/site/content/docs/main/restore-reference.md b/site/content/docs/main/restore-reference.md index 0511f6512..a91ef960d 100644 --- a/site/content/docs/main/restore-reference.md +++ b/site/content/docs/main/restore-reference.md @@ -73,7 +73,7 @@ The following is an overview of Velero's restore process that starts after you r By default, Velero will restore resources in the following order: * Custom Resource Definitions -* Mamespaces +* Namespaces * StorageClasses * VolumeSnapshotClass * VolumeSnapshotContents diff --git a/site/content/docs/v1.9/restore-reference.md b/site/content/docs/v1.9/restore-reference.md index 0511f6512..a91ef960d 100644 --- a/site/content/docs/v1.9/restore-reference.md +++ b/site/content/docs/v1.9/restore-reference.md @@ -73,7 +73,7 @@ The following is an overview of Velero's restore process that starts after you r By default, Velero will restore resources in the following order: * Custom Resource Definitions -* Mamespaces +* Namespaces * StorageClasses * VolumeSnapshotClass * VolumeSnapshotContents From 7b320e71c9a08b0759937852a201f7dd806e718a Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Mon, 27 Jun 2022 14:44:05 +0800 Subject: [PATCH 06/14] Modify Github actions. 1. remove go.sum file from code spell check action. 2. change go version to 1.17 in CRD verify action, and add k8s 1.23 and 1.24 in verification list. Signed-off-by: Xun Jiang --- .github/workflows/crds-verify-kind.yaml | 6 ++++-- .github/workflows/pr-codespell.yml | 2 +- changelogs/unreleased/5052-jxun | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/5052-jxun diff --git a/.github/workflows/crds-verify-kind.yaml b/.github/workflows/crds-verify-kind.yaml index 45b6d71a9..398039f38 100644 --- a/.github/workflows/crds-verify-kind.yaml +++ b/.github/workflows/crds-verify-kind.yaml @@ -14,7 +14,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v2 with: - go-version: 1.16 + go-version: 1.17 id: go # Look for a CLI that's made for this PR - name: Fetch built CLI @@ -64,6 +64,8 @@ jobs: - 1.20.2 - 1.21.1 - 1.22.0 + - 1.23.6 + - 1.24.2 # All steps run in parallel unless otherwise specified. # See https://docs.github.com/en/actions/learn-github-actions/managing-complex-workflows#creating-dependent-jobs steps: @@ -81,7 +83,7 @@ jobs: velero-${{ github.event.pull_request.number }}- - uses: engineerd/setup-kind@v0.5.0 with: - version: "v0.11.1" + version: "v0.14.0" image: "kindest/node:v${{ matrix.k8s }}" - name: Install CRDs run: | diff --git a/.github/workflows/pr-codespell.yml b/.github/workflows/pr-codespell.yml index 169535a07..22218f1f8 100644 --- a/.github/workflows/pr-codespell.yml +++ b/.github/workflows/pr-codespell.yml @@ -14,7 +14,7 @@ jobs: uses: codespell-project/actions-codespell@master with: # ignore the config/.../crd.go file as it's generated binary data that is edited elswhere. - skip: .git,*.png,*.jpg,*.woff,*.ttf,*.gif,*.ico,./config/crd/v1beta1/crds/crds.go,./config/crd/v1/crds/crds.go + skip: .git,*.png,*.jpg,*.woff,*.ttf,*.gif,*.ico,./config/crd/v1beta1/crds/crds.go,./config/crd/v1/crds/crds.go,./go.sum ignore_words_list: iam,aks,ist,bridget,ue check_filenames: true check_hidden: true diff --git a/changelogs/unreleased/5052-jxun b/changelogs/unreleased/5052-jxun new file mode 100644 index 000000000..8df837959 --- /dev/null +++ b/changelogs/unreleased/5052-jxun @@ -0,0 +1 @@ +Modify Github actions. \ No newline at end of file From c6625d1424524e358998c53c38c05aa2824147a2 Mon Sep 17 00:00:00 2001 From: Niu Lechuan Date: Thu, 30 Jun 2022 10:09:29 +0800 Subject: [PATCH 07/14] add changelog file for this PR Signed-off-by: Niu Lechuan --- changelogs/unreleased/5051-niulechuan | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/5051-niulechuan diff --git a/changelogs/unreleased/5051-niulechuan b/changelogs/unreleased/5051-niulechuan new file mode 100644 index 000000000..1cba2e248 --- /dev/null +++ b/changelogs/unreleased/5051-niulechuan @@ -0,0 +1 @@ +Fix typo in doc, in https://velero.io/docs/main/restore-reference/ "Restore order" section, "Mamespace" should be "Namespace". From c633f68ac036fb4d8ca3a138ce636de0b32c0d84 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Thu, 30 Jun 2022 14:00:19 +0800 Subject: [PATCH 08/14] Propose Yonghui as a maintainer Yonghui joined the velero team earlier this year. He has been leading the effort for kopia integration, and delivered the comprehensive comparison report for kopia .vs. restic https://docs.google.com/document/d/1BMLuRzEpYWYE-Ci_eLg8gWbjDv4DSyqj/edit and the detailed design for using kopia as the unified repository: https://github.com/vmware-tanzu/velero/pull/4926 Signed-off-by: Daniel Jiang --- .github/auto-assignees.yml | 1 + MAINTAINERS.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/auto-assignees.yml b/.github/auto-assignees.yml index 282591d72..8f5a75a57 100644 --- a/.github/auto-assignees.yml +++ b/.github/auto-assignees.yml @@ -16,6 +16,7 @@ reviewers: - blackpiglet - qiuming-best - shubham-pampattiwar + - Lyndon-Li tech-writer: - a-mccarthy diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 2819a77e8..f62eb0f94 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -13,6 +13,7 @@ | Xun Jiang | [blackpiglet](https://github.com/blackpiglet) | [VMware](https://www.github.com/vmware/) | | Ming Qiu | [qiuming-best](https://github.com/qiuming-best) | [VMware](https://www.github.com/vmware/) | | Shubham Pampattiwar | [shubham-pampattiwar](https://github.com/shubham-pampattiwar) | [OpenShift](https://github.com/openshift) +| Yonghui Li | [Lyndon-Li](https://github.com/Lyndon-Li) | [VMware](https://www.github.com/vmware/) | ## Emeritus Maintainers * Adnan Abdulhussein ([prydonius](https://github.com/prydonius)) From fb897471c01fc6623e3ed46e752eb6a04b4d98b4 Mon Sep 17 00:00:00 2001 From: niulechuan <81207605+niulechuan@users.noreply.github.com> Date: Mon, 4 Jul 2022 14:31:20 +0800 Subject: [PATCH 09/14] Move 'velero.io/exclude-from-backup' label string to const (#5053) * move 'velero.io/exclude-from-backup' label name to const Signed-off-by: Niu Lechuan * add changelog file (in changelogs/unreleased) of this PR Signed-off-by: Niu Lechuan --- changelogs/unreleased/5053-niulechuan | 1 + pkg/backup/item_backupper.go | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/5053-niulechuan diff --git a/changelogs/unreleased/5053-niulechuan b/changelogs/unreleased/5053-niulechuan new file mode 100644 index 000000000..f44c46a69 --- /dev/null +++ b/changelogs/unreleased/5053-niulechuan @@ -0,0 +1 @@ +Move 'velero.io/exclude-from-backup' label string to const diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index fd4b16028..bd40c3bdb 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -61,6 +61,11 @@ type itemBackupper struct { snapshotLocationVolumeSnapshotters map[string]velero.VolumeSnapshotter } +const ( + // veleroExcludeFromBackupLabel labeled item should be exclude by velero in backup job. + veleroExcludeFromBackupLabel = "velero.io/exclude-from-backup" +) + // backupItem backs up an individual item to tarWriter. The item may be excluded based on the // namespaces IncludesExcludes list. // In addition to the error return, backupItem also returns a bool indicating whether the item @@ -78,8 +83,8 @@ func (ib *itemBackupper) backupItem(logger logrus.FieldLogger, obj runtime.Unstr log = log.WithField("resource", groupResource.String()) log = log.WithField("namespace", namespace) - if metadata.GetLabels()["velero.io/exclude-from-backup"] == "true" { - log.Info("Excluding item because it has label velero.io/exclude-from-backup=true") + if metadata.GetLabels()[veleroExcludeFromBackupLabel] == "true" { + log.Infof("Excluding item because it has label %s=true", veleroExcludeFromBackupLabel) return false, nil } From f550f8e3cda9b6cfaad065525ccc679703622000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Tue, 28 Jun 2022 18:14:40 +0800 Subject: [PATCH 10/14] Fix bsl validation bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix bsl validation bug: the BSL is validated continually and doesn't respect the validation period configured Fixes #5056 Signed-off-by: Wenkai Yin(尹文开) --- changelogs/unreleased/5101-ywk253100 | 1 + .../backup_storage_location_controller.go | 29 +-- pkg/util/kube/predicate.go | 47 +++++ pkg/util/kube/predicate_test.go | 180 ++++++++++++++++++ 4 files changed, 236 insertions(+), 21 deletions(-) create mode 100644 changelogs/unreleased/5101-ywk253100 create mode 100644 pkg/util/kube/predicate.go create mode 100644 pkg/util/kube/predicate_test.go diff --git a/changelogs/unreleased/5101-ywk253100 b/changelogs/unreleased/5101-ywk253100 new file mode 100644 index 000000000..ade00f2a9 --- /dev/null +++ b/changelogs/unreleased/5101-ywk253100 @@ -0,0 +1 @@ + Fix bsl validation bug: the BSL is validated continually and doesn't respect the validation period configured \ No newline at end of file diff --git a/pkg/controller/backup_storage_location_controller.go b/pkg/controller/backup_storage_location_controller.go index ec35a8916..1b08da897 100644 --- a/pkg/controller/backup_storage_location_controller.go +++ b/pkg/controller/backup_storage_location_controller.go @@ -24,12 +24,10 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/vmware-tanzu/velero/internal/storage" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -39,7 +37,10 @@ import ( ) const ( - backupStorageLocationSyncPeriod = 1 * time.Minute + // keep the enqueue period a smaller value to make sure the BSL can be validated as expected. + // The BSL validation frequency is 1 minute by default, if we set the enqueue period as 1 minute, + // this will cause the actual validation interval for each BSL to be 2 minutes + bslValidationEnqueuePeriod = 10 * time.Second ) // BackupStorageLocationReconciler reconciles a BackupStorageLocation object @@ -185,7 +186,7 @@ func (r *BackupStorageLocationReconciler) SetupWithManager(mgr ctrl.Manager) err r.Log, mgr.GetClient(), &velerov1api.BackupStorageLocationList{}, - backupStorageLocationSyncPeriod, + bslValidationEnqueuePeriod, // Add filter function to enqueue BSL per ValidationFrequency setting. func(object client.Object) bool { location := object.(*velerov1api.BackupStorageLocation) @@ -193,22 +194,8 @@ func (r *BackupStorageLocationReconciler) SetupWithManager(mgr ctrl.Manager) err }, ) return ctrl.NewControllerManagedBy(mgr). - For(&velerov1api.BackupStorageLocation{}). - // Handle BSL's creation event and spec update event to let changed BSL got validation immediately. - WithEventFilter(predicate.Funcs{ - CreateFunc: func(ce event.CreateEvent) bool { - return true - }, - UpdateFunc: func(ue event.UpdateEvent) bool { - return ue.ObjectNew.GetGeneration() != ue.ObjectOld.GetGeneration() - }, - DeleteFunc: func(de event.DeleteEvent) bool { - return false - }, - GenericFunc: func(ge event.GenericEvent) bool { - return false - }, - }). + // As the "status.LastValidationTime" field is always updated, this triggers new reconciling process, skip the update event that include no spec change to avoid the reconcile loop + For(&velerov1api.BackupStorageLocation{}, builder.WithPredicates(kube.SpecChangePredicate{})). Watches(g, nil). Complete(r) } diff --git a/pkg/util/kube/predicate.go b/pkg/util/kube/predicate.go new file mode 100644 index 000000000..3073ef881 --- /dev/null +++ b/pkg/util/kube/predicate.go @@ -0,0 +1,47 @@ +/* +Copyright 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 kube + +import ( + "reflect" + + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// SpecChangePredicate implements a default update predicate function on Spec change +// As Velero doesn't enable subresource in CRDs, we cannot use the object's metadata.generation field to check the spec change +// More details about the generation field refer to https://github.com/kubernetes-sigs/controller-runtime/blob/v0.12.2/pkg/predicate/predicate.go#L156 +type SpecChangePredicate struct { + predicate.Funcs +} + +func (SpecChangePredicate) Update(e event.UpdateEvent) bool { + if e.ObjectOld == nil { + return false + } + if e.ObjectNew == nil { + return false + } + oldSpec := reflect.ValueOf(e.ObjectOld).Elem().FieldByName("Spec") + // contains no field named "Spec", return false directly + if oldSpec.IsZero() { + return false + } + newSpec := reflect.ValueOf(e.ObjectNew).Elem().FieldByName("Spec") + return !reflect.DeepEqual(oldSpec.Interface(), newSpec.Interface()) +} diff --git a/pkg/util/kube/predicate_test.go b/pkg/util/kube/predicate_test.go new file mode 100644 index 000000000..d1c3be8df --- /dev/null +++ b/pkg/util/kube/predicate_test.go @@ -0,0 +1,180 @@ +/* +Copyright 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 kube + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + corev1api "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func TestSpecChangePredicate(t *testing.T) { + cases := []struct { + name string + oldObj client.Object + newObj client.Object + changed bool + }{ + { + name: "Contains no spec field", + oldObj: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bsl01", + }, + }, + newObj: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bsl01", + }, + }, + changed: false, + }, + { + name: "ObjectMetas are different, Specs are same", + oldObj: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bsl01", + Annotations: map[string]string{"key1": "value1"}, + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + }, + }, + newObj: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bsl01", + Annotations: map[string]string{"key2": "value2"}, + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + }, + }, + changed: false, + }, + { + name: "Statuses are different, Specs are same", + oldObj: &velerov1.BackupStorageLocation{ + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + }, + Status: velerov1.BackupStorageLocationStatus{ + Phase: velerov1.BackupStorageLocationPhaseAvailable, + }, + }, + newObj: &velerov1.BackupStorageLocation{ + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + }, + Status: velerov1.BackupStorageLocationStatus{ + Phase: velerov1.BackupStorageLocationPhaseUnavailable, + }, + }, + changed: false, + }, + { + name: "Specs are different", + oldObj: &velerov1.BackupStorageLocation{ + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + }, + }, + newObj: &velerov1.BackupStorageLocation{ + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "aws", + }, + }, + changed: true, + }, + { + name: "Specs are same", + oldObj: &velerov1.BackupStorageLocation{ + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + Config: map[string]string{"key": "value"}, + Credential: &corev1api.SecretKeySelector{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: "secret", + }, + Key: "credential", + }, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "bucket1", + Prefix: "prefix", + CACert: []byte{'a'}, + }, + }, + Default: true, + AccessMode: velerov1.BackupStorageLocationAccessModeReadWrite, + BackupSyncPeriod: &metav1.Duration{ + Duration: 1 * time.Minute, + }, + ValidationFrequency: &metav1.Duration{ + Duration: 1 * time.Minute, + }, + }, + }, + newObj: &velerov1.BackupStorageLocation{ + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "azure", + Config: map[string]string{"key": "value"}, + Credential: &corev1api.SecretKeySelector{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: "secret", + }, + Key: "credential", + }, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "bucket1", + Prefix: "prefix", + CACert: []byte{'a'}, + }, + }, + Default: true, + AccessMode: velerov1.BackupStorageLocationAccessModeReadWrite, + BackupSyncPeriod: &metav1.Duration{ + Duration: 1 * time.Minute, + }, + ValidationFrequency: &metav1.Duration{ + Duration: 1 * time.Minute, + }, + }, + }, + changed: false, + }, + } + + predicate := SpecChangePredicate{} + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + changed := predicate.Update(event.UpdateEvent{ + ObjectOld: c.oldObj, + ObjectNew: c.newObj, + }) + assert.Equal(t, c.changed, changed) + }) + } +} From bf8d135876e9bba6c3676f613da0f4e7664678ed Mon Sep 17 00:00:00 2001 From: danfengl Date: Fri, 8 Jul 2022 09:35:07 +0000 Subject: [PATCH 11/14] Fix resource filtering command issue in Doc Signed-off-by: danfengl --- site/content/docs/v1.4/resource-filtering.md | 8 ++++---- site/content/docs/v1.5/resource-filtering.md | 8 ++++---- site/content/docs/v1.6/resource-filtering.md | 8 ++++---- site/content/docs/v1.7/resource-filtering.md | 8 ++++---- site/content/docs/v1.8/resource-filtering.md | 8 ++++---- site/content/docs/v1.9/resource-filtering.md | 8 ++++---- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/site/content/docs/v1.4/resource-filtering.md b/site/content/docs/v1.4/resource-filtering.md index 6c1470e04..0b893c49f 100644 --- a/site/content/docs/v1.4/resource-filtering.md +++ b/site/content/docs/v1.4/resource-filtering.md @@ -24,7 +24,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore two namespaces and their objects. ```bash - velero restore create --include-namespaces , + velero restore create --include-namespaces , --from-backup ``` ### --include-resources @@ -38,7 +38,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore all deployments and configmaps in the cluster. ```bash - velero restore create --include-resources deployments,configmaps + velero restore create --include-resources deployments,configmaps --from-backup ``` * Backup the deployments in a namespace. @@ -72,7 +72,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore only namespaced resources in the cluster. ```bash - velero restore create --include-cluster-resources=false + velero restore create --include-cluster-resources=false --from-backup ``` * Backup a namespace and include cluster-scoped resources. @@ -107,7 +107,7 @@ Wildcard excludes are ignored. * Exclude two namespaces during a restore. ```bash - velero restore create --exclude-namespaces , + velero restore create --exclude-namespaces , --from-backup ``` ### --exclude-resources diff --git a/site/content/docs/v1.5/resource-filtering.md b/site/content/docs/v1.5/resource-filtering.md index 6c1470e04..0b893c49f 100644 --- a/site/content/docs/v1.5/resource-filtering.md +++ b/site/content/docs/v1.5/resource-filtering.md @@ -24,7 +24,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore two namespaces and their objects. ```bash - velero restore create --include-namespaces , + velero restore create --include-namespaces , --from-backup ``` ### --include-resources @@ -38,7 +38,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore all deployments and configmaps in the cluster. ```bash - velero restore create --include-resources deployments,configmaps + velero restore create --include-resources deployments,configmaps --from-backup ``` * Backup the deployments in a namespace. @@ -72,7 +72,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore only namespaced resources in the cluster. ```bash - velero restore create --include-cluster-resources=false + velero restore create --include-cluster-resources=false --from-backup ``` * Backup a namespace and include cluster-scoped resources. @@ -107,7 +107,7 @@ Wildcard excludes are ignored. * Exclude two namespaces during a restore. ```bash - velero restore create --exclude-namespaces , + velero restore create --exclude-namespaces , --from-backup ``` ### --exclude-resources diff --git a/site/content/docs/v1.6/resource-filtering.md b/site/content/docs/v1.6/resource-filtering.md index 6c1470e04..0b893c49f 100644 --- a/site/content/docs/v1.6/resource-filtering.md +++ b/site/content/docs/v1.6/resource-filtering.md @@ -24,7 +24,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore two namespaces and their objects. ```bash - velero restore create --include-namespaces , + velero restore create --include-namespaces , --from-backup ``` ### --include-resources @@ -38,7 +38,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore all deployments and configmaps in the cluster. ```bash - velero restore create --include-resources deployments,configmaps + velero restore create --include-resources deployments,configmaps --from-backup ``` * Backup the deployments in a namespace. @@ -72,7 +72,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore only namespaced resources in the cluster. ```bash - velero restore create --include-cluster-resources=false + velero restore create --include-cluster-resources=false --from-backup ``` * Backup a namespace and include cluster-scoped resources. @@ -107,7 +107,7 @@ Wildcard excludes are ignored. * Exclude two namespaces during a restore. ```bash - velero restore create --exclude-namespaces , + velero restore create --exclude-namespaces , --from-backup ``` ### --exclude-resources diff --git a/site/content/docs/v1.7/resource-filtering.md b/site/content/docs/v1.7/resource-filtering.md index 6c1470e04..0b893c49f 100644 --- a/site/content/docs/v1.7/resource-filtering.md +++ b/site/content/docs/v1.7/resource-filtering.md @@ -24,7 +24,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore two namespaces and their objects. ```bash - velero restore create --include-namespaces , + velero restore create --include-namespaces , --from-backup ``` ### --include-resources @@ -38,7 +38,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore all deployments and configmaps in the cluster. ```bash - velero restore create --include-resources deployments,configmaps + velero restore create --include-resources deployments,configmaps --from-backup ``` * Backup the deployments in a namespace. @@ -72,7 +72,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore only namespaced resources in the cluster. ```bash - velero restore create --include-cluster-resources=false + velero restore create --include-cluster-resources=false --from-backup ``` * Backup a namespace and include cluster-scoped resources. @@ -107,7 +107,7 @@ Wildcard excludes are ignored. * Exclude two namespaces during a restore. ```bash - velero restore create --exclude-namespaces , + velero restore create --exclude-namespaces , --from-backup ``` ### --exclude-resources diff --git a/site/content/docs/v1.8/resource-filtering.md b/site/content/docs/v1.8/resource-filtering.md index ddcd97e4a..1f2d2133d 100644 --- a/site/content/docs/v1.8/resource-filtering.md +++ b/site/content/docs/v1.8/resource-filtering.md @@ -24,7 +24,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore two namespaces and their objects. ```bash - velero restore create --include-namespaces , + velero restore create --include-namespaces , --from-backup ``` ### --include-resources @@ -38,7 +38,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore all deployments and configmaps in the cluster. ```bash - velero restore create --include-resources deployments,configmaps + velero restore create --include-resources deployments,configmaps --from-backup ``` * Backup the deployments in a namespace. @@ -72,7 +72,7 @@ Wildcard takes precedence when both a wildcard and specific resource are include * Restore only namespaced resources in the cluster. ```bash - velero restore create --include-cluster-resources=false + velero restore create --include-cluster-resources=false --from-backup ``` * Backup a namespace and include cluster-scoped resources. @@ -113,7 +113,7 @@ Wildcard excludes are ignored. * Exclude two namespaces during a restore. ```bash - velero restore create --exclude-namespaces , + velero restore create --exclude-namespaces , --from-backup ``` ### --exclude-resources diff --git a/site/content/docs/v1.9/resource-filtering.md b/site/content/docs/v1.9/resource-filtering.md index 02ae3d68d..5c0b34c73 100644 --- a/site/content/docs/v1.9/resource-filtering.md +++ b/site/content/docs/v1.9/resource-filtering.md @@ -26,7 +26,7 @@ Namespaces to include. Default is `*`, all namespaces. * Restore two namespaces and their objects. ```bash - velero restore create --include-namespaces , + velero restore create --include-namespaces , --from-backup ``` ### --include-resources @@ -42,7 +42,7 @@ Kubernetes resources to include in the backup, formatted as resource.group, such * Restore all deployments and configmaps in the cluster. ```bash - velero restore create --include-resources deployments,configmaps + velero restore create --include-resources deployments,configmaps --from-backup ``` * Backup the deployments in a namespace. @@ -76,7 +76,7 @@ Includes cluster-scoped resources. This option can have three possible values: * Restore only namespaced resources in the cluster. ```bash - velero restore create --include-cluster-resources=false + velero restore create --include-cluster-resources=false --from-backup ``` * Backup a namespace and include cluster-scoped resources. @@ -119,7 +119,7 @@ Namespaces to exclude. * Exclude two namespaces during a restore. ```bash - velero restore create --exclude-namespaces , + velero restore create --exclude-namespaces , --from-backup ``` ### --exclude-resources From 9102f53131670ba002b41ab9f2170f4212d2a759 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Mon, 11 Jul 2022 01:41:00 +0800 Subject: [PATCH 12/14] Dump stack trace when the plugin server handles panic Mitigate the issue mentioned in #4782 When there's a bug or misconfiguration that causes nil pointer there will be more stack trace information to help us debug. Signed-off-by: Daniel Jiang --- changelogs/unreleased/5110-reasonerjt | 1 + pkg/plugin/framework/handle_panic.go | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/5110-reasonerjt diff --git a/changelogs/unreleased/5110-reasonerjt b/changelogs/unreleased/5110-reasonerjt new file mode 100644 index 000000000..350f91fa1 --- /dev/null +++ b/changelogs/unreleased/5110-reasonerjt @@ -0,0 +1 @@ +Dump stack trace when the plugin server handles panic \ No newline at end of file diff --git a/pkg/plugin/framework/handle_panic.go b/pkg/plugin/framework/handle_panic.go index 10eb1d2b9..4ea0ec2b5 100644 --- a/pkg/plugin/framework/handle_panic.go +++ b/pkg/plugin/framework/handle_panic.go @@ -17,6 +17,8 @@ limitations under the License. package framework import ( + "runtime/debug" + "github.com/pkg/errors" "google.golang.org/grpc/codes" ) @@ -38,7 +40,8 @@ func handlePanic(p interface{}) error { if _, ok := panicErr.(stackTracer); ok { err = panicErr } else { - err = errors.Wrap(panicErr, "plugin panicked") + errWithStacktrace := errors.Errorf("%v, stack trace: %s", panicErr, debug.Stack()) + err = errors.Wrap(errWithStacktrace, "plugin panicked") } } From 9173ac117e271bbdd56200c9504b8eb581a45053 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Mon, 18 Jul 2022 19:11:53 +0800 Subject: [PATCH 13/14] Let "make shell xxx" respect GOPROXY This commit mitigates the issue for running "make update" locally when the network is not friendly for accessing the default "proxy.golang.org" Signed-off-by: Daniel Jiang --- Makefile | 1 + changelogs/unreleased/5128-reasonerjt | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelogs/unreleased/5128-reasonerjt diff --git a/Makefile b/Makefile index 8dcf6ee50..b3a6a32f2 100644 --- a/Makefile +++ b/Makefile @@ -163,6 +163,7 @@ shell: build-dirs build-env @# under $GOPATH). @docker run \ -e GOFLAGS \ + -e GOPROXY \ -i $(TTY) \ --rm \ -u $$(id -u):$$(id -g) \ diff --git a/changelogs/unreleased/5128-reasonerjt b/changelogs/unreleased/5128-reasonerjt new file mode 100644 index 000000000..3ba53b059 --- /dev/null +++ b/changelogs/unreleased/5128-reasonerjt @@ -0,0 +1 @@ +Let "make shell xxx" respect GOPROXY \ No newline at end of file From 64a8c44104b28d2ed94a9ebc6497d4e539904b98 Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Thu, 14 Jul 2022 16:25:25 -0400 Subject: [PATCH 14/14] Modify BackupStoreGetter to avoid BSL spec changes Pass in a new copy of the map of config values rather than modifying the BSL Spec.Config and then pass in that field. Signed-off-by: Scott Seago --- changelogs/unreleased/5122-sseago | 1 + pkg/persistence/object_store.go | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) create mode 100644 changelogs/unreleased/5122-sseago diff --git a/changelogs/unreleased/5122-sseago b/changelogs/unreleased/5122-sseago new file mode 100644 index 000000000..ec8dc473e --- /dev/null +++ b/changelogs/unreleased/5122-sseago @@ -0,0 +1 @@ +Modify BackupStoreGetter to avoid BSL spec changes diff --git a/pkg/persistence/object_store.go b/pkg/persistence/object_store.go index 1c0b619e1..acda15323 100644 --- a/pkg/persistence/object_store.go +++ b/pkg/persistence/object_store.go @@ -131,19 +131,25 @@ func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocatio return nil, errors.Errorf("backup storage location's bucket name %q must not contain a '/' (if using a prefix, put it in the 'Prefix' field instead)", location.Spec.ObjectStorage.Bucket) } + // Pass a new map into the object store rather than modifying the passed-in + // location. This prevents Velero controllers from accidentally modifying + // the in-cluster BSL with data which doesn't belong in Spec.Config + objectStoreConfig := make(map[string]string) + if location.Spec.Config != nil { + for key, val := range location.Spec.Config { + objectStoreConfig[key] = val + } + } + // add the bucket name and prefix to the config map so that object stores // can use them when initializing. The AWS object store uses the bucket // name to determine the bucket's region when setting up its client. - if location.Spec.Config == nil { - location.Spec.Config = make(map[string]string) - } - - location.Spec.Config["bucket"] = bucket - location.Spec.Config["prefix"] = prefix + objectStoreConfig["bucket"] = bucket + objectStoreConfig["prefix"] = prefix // Only include a CACert if it's specified in order to maintain compatibility with plugins that don't expect it. if location.Spec.ObjectStorage.CACert != nil { - location.Spec.Config["caCert"] = string(location.Spec.ObjectStorage.CACert) + objectStoreConfig["caCert"] = string(location.Spec.ObjectStorage.CACert) } // If the BSL specifies a credential, fetch its path on disk and pass to @@ -154,7 +160,7 @@ func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocatio return nil, errors.Wrap(err, "unable to get credentials") } - location.Spec.Config["credentialsFile"] = credsFile + objectStoreConfig["credentialsFile"] = credsFile } objectStore, err := objectStoreGetter.GetObjectStore(location.Spec.Provider) @@ -162,7 +168,7 @@ func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocatio return nil, err } - if err := objectStore.Init(location.Spec.Config); err != nil { + if err := objectStore.Init(objectStoreConfig); err != nil { return nil, err }