diff --git a/.github/workflows/crds-verify-kind.yaml b/.github/workflows/crds-verify-kind.yaml index a12029fe0..f56675c97 100644 --- a/.github/workflows/crds-verify-kind.yaml +++ b/.github/workflows/crds-verify-kind.yaml @@ -59,6 +59,7 @@ jobs: - 1.19.7 - 1.20.2 - 1.21.1 + - 1.22.0 # 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: diff --git a/.github/workflows/e2e-test-kind.yaml b/.github/workflows/e2e-test-kind.yaml new file mode 100644 index 000000000..bca46b0f5 --- /dev/null +++ b/.github/workflows/e2e-test-kind.yaml @@ -0,0 +1,114 @@ +name: "Run the E2E test on kind" +on: + push: + pull_request: + # Do not run when the change only includes these directories. + paths-ignore: + - "site/**" + - "design/**" +jobs: + # Build the Velero CLI and image once for all Kubernetes versions, and cache it so the fan-out workers can get it. + build: + runs-on: ubuntu-latest + steps: + # Look for a CLI that's made for this PR + - name: Fetch built CLI + id: cli-cache + uses: actions/cache@v2 + with: + path: ./_output/bin/linux/amd64/velero + # The cache key a combination of the current PR number and the commit SHA + key: velero-cli-${{ github.event.pull_request.number }}-${{ github.sha }} + - name: Fetch built image + id: image-cache + uses: actions/cache@v2 + with: + path: ./velero.tar + # The cache key a combination of the current PR number and the commit SHA + key: velero-image-${{ github.event.pull_request.number }}-${{ github.sha }} + - name: Fetch cached go modules + uses: actions/cache@v2 + if: steps.cli-cache.outputs.cache-hit != 'true' + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Check out the code + uses: actions/checkout@v2 + if: steps.cli-cache.outputs.cache-hit != 'true' || steps.image-cache.outputs.cache-hit != 'true' + # If no binaries were built for this PR, build it now. + - name: Build Velero CLI + if: steps.cli-cache.outputs.cache-hit != 'true' + run: | + make local + # If no image were built for this PR, build it now. + - name: Build Velero Image + if: steps.image-cache.outputs.cache-hit != 'true' + run: | + IMAGE=velero VERSION=pr-test make container + docker save velero:pr-test -o ./velero.tar + # Run E2E test against all kubernetes versions on kind + run-e2e-test: + needs: build + runs-on: ubuntu-latest + strategy: + matrix: + k8s: + # doesn't cover 1.15 as 1.15 doesn't support "apiextensions.k8s.io/v1" that is needed for the case + #- 1.15.12 + - 1.16.15 + - 1.17.17 + - 1.18.15 + - 1.19.7 + - 1.20.2 + - 1.21.1 + - 1.22.0 + fail-fast: false + steps: + - name: Check out the code + uses: actions/checkout@v2 + - name: Install MinIO + run: + docker run -d --rm -p 9000:9000 -e "MINIO_ACCESS_KEY=minio" -e "MINIO_SECRET_KEY=minio123" -e "MINIO_DEFAULT_BUCKETS=bucket,additional-bucket" bitnami/minio:2021.6.17-debian-10-r7 + - uses: engineerd/setup-kind@v0.5.0 + with: + version: "v0.11.1" + image: "kindest/node:v${{ matrix.k8s }}" + - name: Fetch built CLI + id: cli-cache + uses: actions/cache@v2 + with: + path: ./_output/bin/linux/amd64/velero + key: velero-cli-${{ github.event.pull_request.number }}-${{ github.sha }} + - name: Fetch built Image + id: image-cache + uses: actions/cache@v2 + with: + path: ./velero.tar + key: velero-image-${{ github.event.pull_request.number }}-${{ github.sha }} + - name: Load Velero Image + run: + kind load image-archive velero.tar + # always try to fetch the cached go modules as the e2e test needs it either + - name: Fetch cached go modules + uses: actions/cache@v2 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Run E2E test + run: | + cat << EOF > /tmp/credential + [default] + aws_access_key_id=minio + aws_secret_access_key=minio123 + EOF + GOPATH=~/go CLOUD_PROVIDER=kind \ + OBJECT_STORE_PROVIDER=aws BSL_CONFIG=region=minio,s3ForcePathStyle="true",s3Url=http://$(hostname -i):9000 \ + CREDS_FILE=/tmp/credential BSL_BUCKET=bucket \ + ADDITIONAL_OBJECT_STORE_PROVIDER=aws ADDITIONAL_BSL_CONFIG=region=minio,s3ForcePathStyle="true",s3Url=http://$(hostname -i):9000 \ + ADDITIONAL_CREDS_FILE=/tmp/credential ADDITIONAL_BSL_BUCKET=additional-bucket \ + GINKGO_FOCUS=Basic VELERO_IMAGE=velero:pr-test \ + make -C test/e2e run \ No newline at end of file diff --git a/changelogs/CHANGELOG-1.6.md b/changelogs/CHANGELOG-1.6.md index ee713ad1a..eabef8867 100644 --- a/changelogs/CHANGELOG-1.6.md +++ b/changelogs/CHANGELOG-1.6.md @@ -33,6 +33,7 @@ This API version was introduced in Kubernetes v1.8. * enable e2e tests to choose crd apiVersion (#3941, @sseago) * Upgrade Velero ClusterRoleBinding to use v1 API (#3995, @jenting) * Install Kubernetes preferred CRDs API version (v1beta1/v1). (#3999, @jenting) + * Use the cluster preferred CRD API version when polling for Velero CRD readiness. (#4015, @zubron) ## v1.6.2 ### 2021-07-16 diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 63d334a18..351c60429 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +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. @@ -767,9 +767,9 @@ func TestCRDInclusion(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.CRDs( - builder.ForCustomResourceDefinition("backups.velero.io").Result(), - builder.ForCustomResourceDefinition("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinition("test.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), ), test.VSLs( builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), @@ -793,9 +793,9 @@ func TestCRDInclusion(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.CRDs( - builder.ForCustomResourceDefinition("backups.velero.io").Result(), - builder.ForCustomResourceDefinition("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinition("test.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), ), test.VSLs( builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), @@ -813,9 +813,9 @@ func TestCRDInclusion(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.CRDs( - builder.ForCustomResourceDefinition("backups.velero.io").Result(), - builder.ForCustomResourceDefinition("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinition("test.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), ), test.VSLs( builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), @@ -839,9 +839,9 @@ func TestCRDInclusion(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.CRDs( - builder.ForCustomResourceDefinition("backups.velero.io").Result(), - builder.ForCustomResourceDefinition("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinition("test.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), ), test.VSLs( builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), @@ -862,9 +862,9 @@ func TestCRDInclusion(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.CRDs( - builder.ForCustomResourceDefinition("backups.velero.io").Result(), - builder.ForCustomResourceDefinition("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinition("test.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), ), test.VSLs( builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), @@ -883,9 +883,9 @@ func TestCRDInclusion(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.CRDs( - builder.ForCustomResourceDefinition("backups.velero.io").Result(), - builder.ForCustomResourceDefinition("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinition("test.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), + builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), ), test.VSLs( builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), diff --git a/pkg/backup/remap_crd_version_action_test.go b/pkg/backup/remap_crd_version_action_test.go index 0a58fb08a..9308ea049 100644 --- a/pkg/backup/remap_crd_version_action_test.go +++ b/pkg/backup/remap_crd_version_action_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +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. @@ -44,7 +44,7 @@ func TestRemapCRDVersionAction(t *testing.T) { // build a v1beta1 CRD with the same name and add it to the fake client that the plugin is going to call. // keep the same one for all 3 tests, since there's little value in recreating it - b := builder.ForCustomResourceDefinition("test.velero.io") + b := builder.ForCustomResourceDefinitionV1Beta1("test.velero.io") c := b.Result() _, err := betaClient.Create(context.TODO(), c, metav1.CreateOptions{}) require.NoError(t, err) diff --git a/pkg/builder/customresourcedefinition_builder.go b/pkg/builder/customresourcedefinition_builder.go deleted file mode 100644 index 847492884..000000000 --- a/pkg/builder/customresourcedefinition_builder.go +++ /dev/null @@ -1,91 +0,0 @@ -/* -Copyright 2019 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 builder - -import ( - apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// CustomResourceDefinitionBuilder builds CustomResourceDefinition objects. -type CustomResourceDefinitionBuilder struct { - object *apiextv1beta1.CustomResourceDefinition -} - -// ForCustomResourceDefinition is the constructor for a CustomResourceDefinitionBuilder. -func ForCustomResourceDefinition(name string) *CustomResourceDefinitionBuilder { - return &CustomResourceDefinitionBuilder{ - object: &apiextv1beta1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: apiextv1beta1.SchemeGroupVersion.String(), - Kind: "CustomResourceDefinition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - }, - } -} - -// Condition adds a CustomResourceDefinitionCondition objects to a CustomResourceDefinitionBuilder. -func (c *CustomResourceDefinitionBuilder) Condition(cond apiextv1beta1.CustomResourceDefinitionCondition) *CustomResourceDefinitionBuilder { - c.object.Status.Conditions = append(c.object.Status.Conditions, cond) - return c -} - -// Result returns the built CustomResourceDefinition. -func (b *CustomResourceDefinitionBuilder) Result() *apiextv1beta1.CustomResourceDefinition { - return b.object -} - -// ObjectMeta applies functional options to the CustomResourceDefinition's ObjectMeta. -func (b *CustomResourceDefinitionBuilder) ObjectMeta(opts ...ObjectMetaOpt) *CustomResourceDefinitionBuilder { - for _, opt := range opts { - opt(b.object) - } - - return b -} - -// CustomResourceDefinitionConditionBuilder builds CustomResourceDefinitionCondition objects. -type CustomResourceDefinitionConditionBuilder struct { - object apiextv1beta1.CustomResourceDefinitionCondition -} - -// ForCustomResourceDefinitionConditionBuilder is the construction for a CustomResourceDefinitionConditionBuilder. -func ForCustomResourceDefinitionCondition() *CustomResourceDefinitionConditionBuilder { - return &CustomResourceDefinitionConditionBuilder{ - object: apiextv1beta1.CustomResourceDefinitionCondition{}, - } -} - -// Type sets the Condition's type. -func (c *CustomResourceDefinitionConditionBuilder) Type(t apiextv1beta1.CustomResourceDefinitionConditionType) *CustomResourceDefinitionConditionBuilder { - c.object.Type = t - return c -} - -// Status sets the Condition's status. -func (c *CustomResourceDefinitionConditionBuilder) Status(cs apiextv1beta1.ConditionStatus) *CustomResourceDefinitionConditionBuilder { - c.object.Status = cs - return c -} - -// Results returns the built CustomResourceDefinitionCondition. -func (b *CustomResourceDefinitionConditionBuilder) Result() apiextv1beta1.CustomResourceDefinitionCondition { - return b.object -} diff --git a/pkg/builder/customresourcedefinition_v1beta1_builder.go b/pkg/builder/customresourcedefinition_v1beta1_builder.go new file mode 100644 index 000000000..d5ab9b248 --- /dev/null +++ b/pkg/builder/customresourcedefinition_v1beta1_builder.go @@ -0,0 +1,91 @@ +/* +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 builder + +import ( + apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// CustomResourceDefinitionV1Beta1Builder builds v1beta1 CustomResourceDefinition objects. +type CustomResourceDefinitionV1Beta1Builder struct { + object *apiextv1beta1.CustomResourceDefinition +} + +// ForCustomResourceDefinitionV1Beta1 is the constructor for a CustomResourceDefinitionV1Beta1Builder. +func ForCustomResourceDefinitionV1Beta1(name string) *CustomResourceDefinitionV1Beta1Builder { + return &CustomResourceDefinitionV1Beta1Builder{ + object: &apiextv1beta1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: apiextv1beta1.SchemeGroupVersion.String(), + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + }, + } +} + +// Condition adds a CustomResourceDefinitionCondition objects to a CustomResourceDefinitionV1Beta1Builder. +func (c *CustomResourceDefinitionV1Beta1Builder) Condition(cond apiextv1beta1.CustomResourceDefinitionCondition) *CustomResourceDefinitionV1Beta1Builder { + c.object.Status.Conditions = append(c.object.Status.Conditions, cond) + return c +} + +// Result returns the built CustomResourceDefinition. +func (b *CustomResourceDefinitionV1Beta1Builder) Result() *apiextv1beta1.CustomResourceDefinition { + return b.object +} + +// ObjectMeta applies functional options to the CustomResourceDefinition's ObjectMeta. +func (b *CustomResourceDefinitionV1Beta1Builder) ObjectMeta(opts ...ObjectMetaOpt) *CustomResourceDefinitionV1Beta1Builder { + for _, opt := range opts { + opt(b.object) + } + + return b +} + +// CustomResourceDefinitionV1Beta1ConditionBuilder builds CustomResourceDefinitionV1Beta1Condition objects. +type CustomResourceDefinitionV1Beta1ConditionBuilder struct { + object apiextv1beta1.CustomResourceDefinitionCondition +} + +// ForCustomResourceDefinitionV1Beta1Condition is the construction for a CustomResourceDefinitionV1Beta1ConditionBuilder. +func ForCustomResourceDefinitionV1Beta1Condition() *CustomResourceDefinitionV1Beta1ConditionBuilder { + return &CustomResourceDefinitionV1Beta1ConditionBuilder{ + object: apiextv1beta1.CustomResourceDefinitionCondition{}, + } +} + +// Type sets the Condition's type. +func (c *CustomResourceDefinitionV1Beta1ConditionBuilder) Type(t apiextv1beta1.CustomResourceDefinitionConditionType) *CustomResourceDefinitionV1Beta1ConditionBuilder { + c.object.Type = t + return c +} + +// Status sets the Condition's status. +func (c *CustomResourceDefinitionV1Beta1ConditionBuilder) Status(cs apiextv1beta1.ConditionStatus) *CustomResourceDefinitionV1Beta1ConditionBuilder { + c.object.Status = cs + return c +} + +// Result returns the built v1beta1 CustomResourceDefinitionCondition. +func (c *CustomResourceDefinitionV1Beta1ConditionBuilder) Result() apiextv1beta1.CustomResourceDefinitionCondition { + return c.object +} diff --git a/pkg/cmd/cli/install/install.go b/pkg/cmd/cli/install/install.go index d07051f5e..f14c8725c 100644 --- a/pkg/cmd/cli/install/install.go +++ b/pkg/cmd/cli/install/install.go @@ -298,24 +298,28 @@ func (o *InstallOptions) Run(c *cobra.Command, f client.Factory) error { if err != nil { return err } - factory := client.NewDynamicFactory(dynamicClient) + dynamicFactory := client.NewDynamicFactory(dynamicClient) + kbClient, err := f.KubebuilderClient() + if err != nil { + return err + } errorMsg := fmt.Sprintf("\n\nError installing Velero. Use `kubectl logs deploy/velero -n %s` to check the deploy logs", o.Namespace) - err = install.Install(factory, resources, os.Stdout) + err = install.Install(dynamicFactory, kbClient, resources, os.Stdout) if err != nil { return errors.Wrap(err, errorMsg) } if o.Wait { fmt.Println("Waiting for Velero deployment to be ready.") - if _, err = install.DeploymentIsReady(factory, o.Namespace); err != nil { + if _, err = install.DeploymentIsReady(dynamicFactory, o.Namespace); err != nil { return errors.Wrap(err, errorMsg) } if o.UseRestic { fmt.Println("Waiting for Velero restic daemonset to be ready.") - if _, err = install.DaemonSetIsReady(factory, o.Namespace); err != nil { + if _, err = install.DaemonSetIsReady(dynamicFactory, o.Namespace); err != nil { return errors.Wrap(err, errorMsg) } } diff --git a/pkg/install/install.go b/pkg/install/install.go index 88e12a71a..85257f4ad 100644 --- a/pkg/install/install.go +++ b/pkg/install/install.go @@ -1,5 +1,5 @@ /* -Copyright 2019 the Velero contributors. +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. @@ -17,6 +17,7 @@ limitations under the License. package install import ( + "context" "fmt" "io" "strings" @@ -25,6 +26,7 @@ import ( "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -57,53 +60,95 @@ type ResourceGroup struct { OtherResources []*unstructured.Unstructured } -// crdsAreReady polls the API server to see if the BackupStorageLocation and VolumeSnapshotLocation CRDs are ready to create objects. -func crdsAreReady(factory client.DynamicFactory, crdKinds []string) (bool, error) { - gvk := schema.FromAPIVersionAndKind(apiextv1beta1.SchemeGroupVersion.String(), "CustomResourceDefinition") - apiResource := metav1.APIResource{ - Name: kindToResource["CustomResourceDefinition"], - Namespaced: false, - } - c, err := factory.ClientForGroupVersionResource(gvk.GroupVersion(), apiResource, "") - if err != nil { - return false, errors.Wrapf(err, "Error creating client for CustomResourceDefinition polling") - } - // Track all the CRDs that have been found and successfully marshalled. - // len should be equal to len(crdKinds) in the happy path. - foundCRDs := make([]*apiextv1beta1.CustomResourceDefinition, 0) - var areReady bool - err = wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { - for _, k := range crdKinds { - unstruct, err := c.Get(k, metav1.GetOptions{}) +// crdV1Beta1ReadinessFn returns a function that can be used for polling to check +// if the provided unstructured v1beta1 CRDs are ready for use in the cluster. +func crdV1Beta1ReadinessFn(kbClient kbclient.Client, unstructuredCrds []*unstructured.Unstructured) func() (bool, error) { + // Track all the CRDs that have been found and in ready state. + // len should be equal to len(unstructuredCrds) in the happy path. + return func() (bool, error) { + foundCRDs := make([]*apiextv1beta1.CustomResourceDefinition, 0) + for _, unstructuredCrd := range unstructuredCrds { + crd := &apiextv1beta1.CustomResourceDefinition{} + key := kbclient.ObjectKey{Name: unstructuredCrd.GetName()} + err := kbClient.Get(context.Background(), key, crd) if apierrors.IsNotFound(err) { return false, nil } else if err != nil { - return false, errors.Wrapf(err, "error waiting for %s to be ready", k) + return false, errors.Wrapf(err, "error waiting for %s to be ready", crd.GetName()) } - - crd := new(apiextv1beta1.CustomResourceDefinition) - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstruct.Object, crd); err != nil { - return false, errors.Wrapf(err, "error converting %s from unstructured", k) - } - foundCRDs = append(foundCRDs, crd) } - if len(foundCRDs) != len(crdKinds) { + if len(foundCRDs) != len(unstructuredCrds) { return false, nil } for _, crd := range foundCRDs { - if !kube.IsCRDReady(crd) { + ready := kube.IsV1Beta1CRDReady(crd) + if !ready { return false, nil } - } - areReady = true - return true, nil - }) - return areReady, nil + } +} + +// crdV1ReadinessFn returns a function that can be used for polling to check +// if the provided unstructured v1 CRDs are ready for use in the cluster. +func crdV1ReadinessFn(kbClient kbclient.Client, unstructuredCrds []*unstructured.Unstructured) func() (bool, error) { + return func() (bool, error) { + foundCRDs := make([]*apiextv1.CustomResourceDefinition, 0) + for _, unstructuredCrd := range unstructuredCrds { + crd := &apiextv1.CustomResourceDefinition{} + key := kbclient.ObjectKey{Name: unstructuredCrd.GetName()} + err := kbClient.Get(context.Background(), key, crd) + if apierrors.IsNotFound(err) { + return false, nil + } else if err != nil { + return false, errors.Wrapf(err, "error waiting for %s to be ready", crd.GetName()) + } + foundCRDs = append(foundCRDs, crd) + } + + if len(foundCRDs) != len(unstructuredCrds) { + return false, nil + } + + for _, crd := range foundCRDs { + ready := kube.IsV1CRDReady(crd) + if !ready { + return false, nil + } + } + return true, nil + } +} + +// crdsAreReady polls the API server to see if the Velero CRDs are ready to create objects. +func crdsAreReady(kbClient kbclient.Client, crds []*unstructured.Unstructured) (bool, error) { + if len(crds) == 0 { + // no CRDs to check so return + return true, nil + } + + // We assume that all Velero CRDs have the same GVK so we can use the GVK of the + // first CRD to determine whether to use the v1beta1 or v1 API during polling. + gvk := crds[0].GroupVersionKind() + + var crdReadinessFn func() (bool, error) + if gvk.Version == "v1beta1" { + crdReadinessFn = crdV1Beta1ReadinessFn(kbClient, crds) + } else if gvk.Version == "v1" { + crdReadinessFn = crdV1ReadinessFn(kbClient, crds) + } else { + return false, fmt.Errorf("unsupported CRD version %q", gvk.Version) + } + + err := wait.PollImmediate(time.Second, time.Minute, crdReadinessFn) + if err != nil { + return false, errors.Wrap(err, "Error polling for CRDs") + } + return true, nil } func isAvailable(c appsv1.DeploymentCondition) bool { @@ -282,19 +327,19 @@ func CreateClient(r *unstructured.Unstructured, factory client.DynamicFactory, w // Resources will be sorted into CustomResourceDefinitions and any other resource type, and the function will wait up to 1 minute // for CRDs to be ready before proceeding. // An io.Writer can be used to output to a log or the console. -func Install(factory client.DynamicFactory, resources *unstructured.UnstructuredList, w io.Writer) error { +func Install(dynamicFactory client.DynamicFactory, kbClient kbclient.Client, resources *unstructured.UnstructuredList, w io.Writer) error { rg := GroupResources(resources) //Install CRDs first for _, r := range rg.CRDResources { - if err := createResource(r, factory, w); err != nil { + if err := createResource(r, dynamicFactory, w); err != nil { return err } } // Wait for CRDs to be ready before proceeding fmt.Fprint(w, "Waiting for resources to be ready in cluster...\n") - _, err := crdsAreReady(factory, []string{"backupstoragelocations.velero.io", "volumesnapshotlocations.velero.io"}) + _, err := crdsAreReady(kbClient, rg.CRDResources) if err == wait.ErrWaitTimeout { return errors.Errorf("timeout reached, CRDs not ready") } else if err != nil { @@ -303,7 +348,7 @@ func Install(factory client.DynamicFactory, resources *unstructured.Unstructured // Install all other resources for _, r := range rg.OtherResources { - if err = createResource(r, factory, w); err != nil { + if err = createResource(r, dynamicFactory, w); err != nil { return err } } diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go index 994e8f370..a80f864de 100644 --- a/pkg/util/kube/utils.go +++ b/pkg/util/kube/utils.go @@ -1,5 +1,5 @@ /* -Copyright 2017, 2019 the Velero contributors. +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. @@ -23,6 +23,7 @@ import ( "github.com/pkg/errors" corev1api "k8s.io/api/core/v1" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -145,8 +146,23 @@ func GetVolumeDirectory(pod *corev1api.Pod, volumeName string, pvcLister corev1l return pvc.Spec.VolumeName, nil } -// IsCRDReady checks a CRD to see if it's ready, with both the Established and NamesAccepted conditions. -func IsCRDReady(crd *apiextv1beta1.CustomResourceDefinition) bool { +// IsV1CRDReady checks a v1 CRD to see if it's ready, with both the Established and NamesAccepted conditions. +func IsV1CRDReady(crd *apiextv1.CustomResourceDefinition) bool { + var isEstablished, namesAccepted bool + for _, cond := range crd.Status.Conditions { + if cond.Type == apiextv1.Established && cond.Status == apiextv1.ConditionTrue { + isEstablished = true + } + if cond.Type == apiextv1.NamesAccepted && cond.Status == apiextv1.ConditionTrue { + namesAccepted = true + } + } + + return (isEstablished && namesAccepted) +} + +// IsV1Beta1CRDReady checks a v1beta1 CRD to see if it's ready, with both the Established and NamesAccepted conditions. +func IsV1Beta1CRDReady(crd *apiextv1beta1.CustomResourceDefinition) bool { var isEstablished, namesAccepted bool for _, cond := range crd.Status.Conditions { if cond.Type == apiextv1beta1.Established && cond.Status == apiextv1beta1.ConditionTrue { @@ -161,8 +177,8 @@ func IsCRDReady(crd *apiextv1beta1.CustomResourceDefinition) bool { } // IsUnstructuredCRDReady checks an unstructured CRD to see if it's ready, with both the Established and NamesAccepted conditions. -// TODO: Delete this function and use IsCRDReady when the upstream runtime.FromUnstructured function properly handles int64 field conversions. -// Duplicated function because the velero install package uses IsCRDReady with the beta types. +// TODO: Delete this function and use IsV1CRDReady/IsV1Beta1CRDReady when the upstream runtime.FromUnstructured function properly handles int64 field conversions. +// Duplicated function because the velero install package uses IsV1CRDReady/IsV1Beta1CRDReady with instances of v1/v1beta1 types. // See https://github.com/kubernetes/kubernetes/issues/87675 // This is different from the fix for https://github.com/vmware-tanzu/velero/issues/2319 because here, // we need to account for *both* v1beta1 and v1 CRDs, so doing marshalling into JSON to convert to a Go type may not be as useful here, unless we do @@ -207,6 +223,7 @@ func IsUnstructuredCRDReady(crd *unstructured.Unstructured) (bool, error) { // Here is the actual logic of the function // Cast the API's types into strings since we're pulling strings out of the unstructured data. + // We are using the v1beta1 constants here but they are the same as the v1 constants. if conditionType == string(apiextv1beta1.Established) && status == string(apiextv1beta1.ConditionTrue) { isEstablished = true } diff --git a/pkg/util/kube/utils_test.go b/pkg/util/kube/utils_test.go index d72f99404..cb9d2c79c 100644 --- a/pkg/util/kube/utils_test.go +++ b/pkg/util/kube/utils_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +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. @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -210,7 +211,7 @@ func TestGetVolumeDirectorySuccess(t *testing.T) { } } -func TestIsCRDReady(t *testing.T) { +func TestIsV1Beta1CRDReady(t *testing.T) { tests := []struct { name string crd *apiextv1beta1.CustomResourceDefinition @@ -218,33 +219,72 @@ func TestIsCRDReady(t *testing.T) { }{ { name: "CRD is not established & not accepting names - not ready", - crd: builder.ForCustomResourceDefinition("MyCRD").Result(), + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").Result(), want: false, }, { name: "CRD is established & not accepting names - not ready", - crd: builder.ForCustomResourceDefinition("MyCRD"). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).Result(), + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD"). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).Result(), want: false, }, { name: "CRD is not established & accepting names - not ready", - crd: builder.ForCustomResourceDefinition("MyCRD"). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).Result(), + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD"). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).Result(), want: false, }, { name: "CRD is established & accepting names - ready", - crd: builder.ForCustomResourceDefinition("MyCRD"). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()). + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD"). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()). Result(), want: true, }, } for _, tc := range tests { - result := IsCRDReady(tc.crd) + result := IsV1Beta1CRDReady(tc.crd) + assert.Equal(t, tc.want, result) + } +} + +func TestIsV1CRDReady(t *testing.T) { + tests := []struct { + name string + crd *apiextv1.CustomResourceDefinition + want bool + }{ + { + name: "CRD is not established & not accepting names - not ready", + crd: builder.ForV1CustomResourceDefinition("MyCRD").Result(), + want: false, + }, + { + name: "CRD is established & not accepting names - not ready", + crd: builder.ForV1CustomResourceDefinition("MyCRD"). + Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.Established).Status(apiextv1.ConditionTrue).Result()).Result(), + want: false, + }, + { + name: "CRD is not established & accepting names - not ready", + crd: builder.ForV1CustomResourceDefinition("MyCRD"). + Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NamesAccepted).Status(apiextv1.ConditionTrue).Result()).Result(), + want: false, + }, + { + name: "CRD is established & accepting names - ready", + crd: builder.ForV1CustomResourceDefinition("MyCRD"). + Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.Established).Status(apiextv1.ConditionTrue).Result()). + Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NamesAccepted).Status(apiextv1.ConditionTrue).Result()). + Result(), + want: true, + }, + } + + for _, tc := range tests { + result := IsV1CRDReady(tc.crd) assert.Equal(t, tc.want, result) } } @@ -257,26 +297,26 @@ func TestIsUnstructuredCRDReady(t *testing.T) { }{ { name: "CRD is not established & not accepting names - not ready", - crd: builder.ForCustomResourceDefinition("MyCRD").Result(), + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").Result(), want: false, }, { name: "CRD is established & not accepting names - not ready", - crd: builder.ForCustomResourceDefinition("MyCRD"). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).Result(), + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD"). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).Result(), want: false, }, { name: "CRD is not established & accepting names - not ready", - crd: builder.ForCustomResourceDefinition("MyCRD"). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).Result(), + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD"). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).Result(), want: false, }, { name: "CRD is established & accepting names - ready", - crd: builder.ForCustomResourceDefinition("MyCRD"). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()). - Condition(builder.ForCustomResourceDefinitionCondition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()). + crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD"). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()). + Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()). Result(), want: true, }, diff --git a/test/e2e/velero_utils.go b/test/e2e/velero_utils.go index 72d7f58fe..6920e05d6 100644 --- a/test/e2e/velero_utils.go +++ b/test/e2e/velero_utils.go @@ -109,7 +109,7 @@ func installVeleroServer(io *cliinstall.InstallOptions) error { errorMsg := "\n\nError installing Velero. Use `kubectl logs deploy/velero -n velero` to check the deploy logs" resources := install.AllResources(vo) - err = install.Install(client.dynamicFactory, resources, os.Stdout) + err = install.Install(client.dynamicFactory, client.kubebuilder, resources, os.Stdout) if err != nil { return errors.Wrap(err, errorMsg) }