Update common controller tests after split controller #220

ggriffiths · 2019-12-23T06:52:49Z

Signed-off-by: Grant Griffiths grant@portworx.com

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
Adds more tests to common controller pkg

Which issue(s) this PR fixes:
Part of #196

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

ggriffiths · 2019-12-23T06:54:39Z

Main tests left to cover:

fix/update a few create snapshot tests
update delete snapshot tests

/assign @xing-yang

xing-yang · 2019-12-23T15:57:40Z

Please change from "Fixes #196" to "Part of #196" so that the issue won't be closed automatically when this PR is merged.

xing-yang · 2019-12-23T16:25:15Z

pkg/common-controller/framework_test.go

@@ -770,6 +761,7 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
 	ctrl.snapshotListerSynced = alwaysReady
 	ctrl.classListerSynced = alwaysReady
 	ctrl.pvcListerSynced = alwaysReady
+	ctrl.createSnapshotContentInterval = time.Millisecond * 10


Set to 5 * time.Millisecond.

Also add a ctrl.createSnapshotContentRetryCount = 3

xing-yang · 2019-12-23T16:27:43Z

pkg/common-controller/framework_test.go

+				if test.initialSnapshots[0].Name == snapshot.Name {
+					if slice.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer, nil) && !slice.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer, nil) &&
+						slice.ContainsString(test.initialSnapshots[0].ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) && !slice.ContainsString(snapshot.ObjectMeta.Finalizers, utils.VolumeSnapshotAsSourceFinalizer, nil) {
+						klog.V(4).Infof("test %q succeeded. SnapshotFinalizer is removed from PVC %s", test.name, snapshot.Name)


s/SnapshotFinalizer is removed from PVC/SnapshotFinalizer is removed from snapshot

xing-yang · 2019-12-23T16:28:39Z

pkg/common-controller/framework_test.go

+
+		ctrl, err := newTestController(kubeClient, client, nil, t, test)
+		if err != nil {
+			t.Fatalf("Test %q construct persistent content failed: %v", test.name, err)


s/construct persistent content failed/construct test controller failed

xing-yang · 2019-12-23T16:29:50Z

pkg/common-controller/framework_test.go

+			t.Errorf("Test %q failed: %v", test.name, err)
+		}
+
+		// Verify Finalizer tests results


Wrong comments?

xing-yang · 2019-12-23T16:32:01Z

pkg/common-controller/snapshot_create_test.go

 		{
+			name:              "7-2 - fail to update snapshot reports warning sevent",


s/warning sevent/warning event

xing-yang · 2019-12-23T16:35:56Z

pkg/common-controller/snapshot_create_test.go

-					parameters:   map[string]string{"param1": "value1"},
-					// information to return
-					err: errors.New("mock create snapshot error"),
+					name:              "7-7 - fail create snapshot due to csi driver error",


This test case does not seem relevant any more as the common controller does not communicate with CSI driver directly.

ggriffiths · 2019-12-23T17:21:26Z

Addressed feedback @xing-yang

xing-yang · 2019-12-23T17:45:33Z

pkg/common-controller/snapshot_create_test.go

-			// TODO(xiangqian): this test case needs to be revisited the scenario
-			// of VolumeSnapshotContent saving failure. Since there will be no content object
-			// in API server, it could potentially cause leaking issue
-			name:              "7-9 - fail create snapshot due to cannot save snapshot content",


7-9 should still be kept and we'll revisit it later.

xing-yang · 2019-12-23T17:45:56Z

pkg/common-controller/snapshot_create_test.go

-			test:           testSyncSnapshot,
-		},*/
-		/*{
-		name:              "7-8 - fail create snapshot due to cannot update snapshot status",


7-8 should also be kept so we remember to revisit.

Signed-off-by: Grant Griffiths <grant@portworx.com>

xing-yang · 2019-12-23T18:07:42Z

/lgtm
/approve

k8s-ci-robot · 2019-12-23T18:07:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ggriffiths, xing-yang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [xing-yang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update 0e7ae99 Update k8s image repo url 77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version 155854b Fix dep version mismatch 8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update 1d3f94d Update go version to 1.20 to match k/k v1.27 e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error b74a512 test: fix golint error aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver 7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0 a2171be Merge pull request kubernetes-csi#216 from msau42/process cb98782 Merge pull request kubernetes-csi#217 from msau42/owners a11216e add new reviewers and remove inactive reviewers dd98675 Add step for checking builds b66c082 Merge pull request kubernetes-csi#214 from pohly/junit-fixes b9b6763 filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit d427783 filter-junit.go: preserve system error log 38e1146 prow.sh: publish individual JUnit files as separate artifacts git-subtree-dir: release-tools git-subtree-split: 6613c39

670bb0e Merge pull request kubernetes-csi#229 from marosset/fix-codespell-errors 35d5e78 Merge pull request kubernetes-csi#219 from yashsingh74/update-registry 63473cc Merge pull request kubernetes-csi#231 from coulof/bump-go-version-1.20.5 29a5c76 Merge pull request kubernetes-csi#228 from mowangdk/chore/adopt_kubernetes_recommand_labels 8dd2821 Update cloudbuild image with go 1.20.5 1df23db Merge pull request kubernetes-csi#230 from msau42/prow 1f92b7e Add ginkgo timeout to e2e tests to help catch any stuck tests 2b8b80e fixing some codespell errors c10b678 Merge pull request kubernetes-csi#227 from coulof/check-sidecar-supported-versions 72984ec chore: adopt kubernetes recommand label b055535 Header bd0a10b typo c39d73c Add comments f6491af Script to verify EOL sidecar version 4133d1d Merge pull request kubernetes-csi#226 from msau42/cloudbuild 8d519d2 Pin buildkit to v0.10.6 to workaround v0.11 bug with docker manifest 6e04a03 Merge pull request kubernetes-csi#224 from msau42/cloudbuild 26fdfff Update cloudbuild image 6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update 0e7ae99 Update k8s image repo url 77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version 155854b Fix dep version mismatch 8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update 1d3f94d Update go version to 1.20 to match k/k v1.27 e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error b74a512 test: fix golint error 901bcb5 Update registry k8s.gcr.io -> registry.k8s.io aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver 7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0 a2171be Merge pull request kubernetes-csi#216 from msau42/process cb98782 Merge pull request kubernetes-csi#217 from msau42/owners a11216e add new reviewers and remove inactive reviewers dd98675 Add step for checking builds b66c082 Merge pull request kubernetes-csi#214 from pohly/junit-fixes b9b6763 filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit d427783 filter-junit.go: preserve system error log 38e1146 prow.sh: publish individual JUnit files as separate artifacts 78c0fb7 Merge pull request kubernetes-csi#208 from jsafrane/skip-selinux 36e433e Skip SELinux tests in CI by default 348d4a9 Merge pull request kubernetes-csi#207 from RaunakShah/reviewers 1efc272 Merge pull request kubernetes-csi#206 from RaunakShah/update-prow 7d410d8 Changes to csi prow to run e2e tests in sidecars cfa5a75 Merge pull request kubernetes-csi#203 from humblec/test-vendor 4edd1d8 Add RaunakShah to CSI reviewers group 7ccc959 release tools update to 1.19 d24254f Merge pull request kubernetes-csi#202 from xing-yang/kind_0.14.0 0faa3fc Update to Kind v0.14.0 images ef4e1b2 Merge pull request kubernetes-csi#201 from xing-yang/add_1.24_image 4ddce25 Add 1.24 Kind image 7fe5149 Merge pull request kubernetes-csi#200 from pohly/bump-kubernetes-version 70915a8 prow.sh: update snapshotter version 31a3f38 Merge pull request kubernetes-csi#199 from pohly/bump-kubernetes-version 7577454 prow.sh: bump Kubernetes to v1.22.0 d29a2e7 Merge pull request kubernetes-csi#198 from pohly/csi-test-5.0.0 41cb70d prow.sh: sanity testing with csi-test v5.0.0 c85a63f Merge pull request kubernetes-csi#197 from pohly/fix-alpha-testing b86d8e9 support Kubernetes 1.25 + Ginkgo v2 ab0b0a3 Merge pull request kubernetes-csi#192 from andyzhangx/patch-1 7bbab24 Merge pull request kubernetes-csi#196 from humblec/non-alpha e51ff2c introduce control variable for non alpha feature gate configuration ca19ef5 Merge pull request kubernetes-csi#195 from pohly/fix-alpha-testing 3948331 fix testing with latest Kubernetes e4dab7f Merge pull request kubernetes-csi#194 from yselkowitz/registry-k8s-io 84a4d5a Move from k8s.gcr.io to registry.k8s.io 9a0260c fix boilerplate header 37d1104 Merge pull request kubernetes-csi#191 from pohly/go-1.18 db917f5 update to Go 1.18 335339f Merge pull request kubernetes-csi#187 from mauriciopoppe/remove-eol-windows-versions 890b87a Merge pull request kubernetes-csi#188 from pwschuurman/update-release-notes-docs 274bc9b Update Sidecar Release Process documentation to reference latest syntax for release-notes tool 87b6c37 Merge pull request kubernetes-csi#185 from Garima-Negi/fix-OWNERS-files f1de2c6 Fix OWNERS file - squashed commits 59ae38b Remove EOL windows versions from BUILD_PLATFORMS 5d66471 Merge pull request kubernetes-csi#186 from humblec/sp d066f1b Correct prow.sh typo and make codespell linter pass 762e22d Merge pull request kubernetes-csi#184 from pohly/image-publishing-troubleshooting 81e26c3 SIDECAR_RELEASE_PROCESS.md: add troubleshooting for image publishing 31aa44d Merge pull request kubernetes-csi#182 from chrishenzie/csi-sanity-version f49e141 Update csi-sanity test suite to v4.3.0 d9815c2 Merge pull request kubernetes-csi#181 from mauriciopoppe/armv7-support 05c1801 Add support to build arm/v7 images 4aedf35 Merge pull request kubernetes-csi#178 from xing-yang/timeout 2b9897e Increase build timeout 51d3702 Merge pull request kubernetes-csi#177 from mauriciopoppe/kind-image-1.23 a30efea Add kind image for 1.23 a6a1a79 Merge pull request kubernetes-csi#176 from pohly/go-1.17.3 0a2cf63 prow.sh: bump Go to 1.17.3 fc29fdd Merge pull request kubernetes-csi#141 from pohly/prune-replace-optional 5b9a1e0 Merge pull request kubernetes-csi#175 from jimdaga/patch-1 5eeb602 images: use k8s-staging-test-infra/gcb-docker-gcloud b46691a go-get-kubernetes.sh: make replace statement pruning optional git-subtree-dir: release-tools git-subtree-split: 670bb0e

test: fix golint error

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 23, 2019

k8s-ci-robot requested review from jingxu97 and saad-ali December 23, 2019 06:52

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 23, 2019

k8s-ci-robot assigned xing-yang Dec 23, 2019

ggriffiths force-pushed the common_controller_uts branch from 8c64461 to 217d406 Compare December 23, 2019 07:03

ggriffiths force-pushed the common_controller_uts branch from 217d406 to b21dd35 Compare December 23, 2019 16:21

xing-yang reviewed Dec 23, 2019

View reviewed changes

ggriffiths force-pushed the common_controller_uts branch from b21dd35 to 470b180 Compare December 23, 2019 17:20

ggriffiths force-pushed the common_controller_uts branch from 470b180 to aace3e4 Compare December 23, 2019 17:30

xing-yang reviewed Dec 23, 2019

View reviewed changes

Update common controller tests after split controller

33c896f

Signed-off-by: Grant Griffiths <grant@portworx.com>

ggriffiths force-pushed the common_controller_uts branch from aace3e4 to 33c896f Compare December 23, 2019 18:00

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 23, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 23, 2019

k8s-ci-robot merged commit 09a7c03 into kubernetes-csi:master Dec 23, 2019

andyzhangx pushed a commit to andyzhangx/external-snapshotter that referenced this pull request Jun 5, 2024

Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error

e322ce5

test: fix golint error

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update common controller tests after split controller #220

Update common controller tests after split controller #220

ggriffiths commented Dec 23, 2019 •

edited

Loading

ggriffiths commented Dec 23, 2019 •

edited

Loading

xing-yang commented Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang Dec 23, 2019

ggriffiths commented Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang Dec 23, 2019

xing-yang commented Dec 23, 2019

k8s-ci-robot commented Dec 23, 2019

		{
		name: "7-2 - fail to update snapshot reports warning sevent",

Update common controller tests after split controller #220

Update common controller tests after split controller #220

Conversation

ggriffiths commented Dec 23, 2019 • edited Loading

ggriffiths commented Dec 23, 2019 • edited Loading

xing-yang commented Dec 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggriffiths commented Dec 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xing-yang commented Dec 23, 2019

k8s-ci-robot commented Dec 23, 2019

ggriffiths commented Dec 23, 2019 •

edited

Loading

ggriffiths commented Dec 23, 2019 •

edited

Loading