increase code coverage by moving utility functions from api packages (#1479)

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

pkg/apis/core/v1alpha1/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "go_default_library",
@@ -25,22 +25,3 @@ go_library(
         "//vendor/kubevirt.io/controller-lifecycle-operator-sdk/pkg/sdk/api:go_default_library",
     ],
 )
-
-go_test(
-    name = "go_default_test",
-    srcs = [
-        "utils_test.go",
-        "v1alpha1_suite_test.go",
-    ],
-    embed = [":go_default_library"],
-    deps = [
-        "//tests/reporters:go_default_library",
-        "//vendor/github.com/onsi/ginkgo:go_default_library",
-        "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library",
-        "//vendor/github.com/onsi/gomega:go_default_library",
-        "//vendor/k8s.io/api/core/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
-    ],
-)

pkg/apis/core/v1alpha1/utils/BUILD.bazel

@@ -0,0 +1,33 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["utils.go"],
+    importpath = "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1/utils",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/apis/core/v1alpha1:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = [
+        "utils_test.go",
+        "v1alpha1_suite_test.go",
+    ],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/core/v1alpha1:go_default_library",
+        "//tests/reporters:go_default_library",
+        "//vendor/github.com/onsi/ginkgo:go_default_library",
+        "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library",
+        "//vendor/github.com/onsi/gomega:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
+    ],
+)

pkg/apis/core/v1alpha1/utils/utils.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2020 The CDI Authors.
+
+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 utils
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1"
+)
+
+// IsPopulated indicates if the persistent volume passed in has been fully populated. It follow the following logic
+// 1. If the PVC is not owned by a DataVolume, return true, we assume someone else has properly populated the image
+// 2. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase succeeded return true
+// 3. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase !succeeded return false
+func IsPopulated(pvc *corev1.PersistentVolumeClaim, getDvFunc func(name, namespace string) (*cdiv1.DataVolume, error)) (bool, error) {
+	pvcOwner := metav1.GetControllerOf(pvc)
+	if pvcOwner != nil && pvcOwner.Kind == "DataVolume" {
+		// Find the data volume:
+		dv, err := getDvFunc(pvcOwner.Name, pvc.Namespace)
+		if err != nil {
+			return false, err
+		}
+		if dv.Status.Phase != cdiv1.Succeeded {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+// IsWaitForFirstConsumerBeforePopulating indicates if the persistent volume passed in is in ClaimPending state and waiting for first consumer.
+// It follow the following logic
+// 1. If the PVC is not owned by a DataVolume, return false, we can not assume it will be populated
+// 2. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase WaitForFirstConsumer return true
+// 3. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase !WaitForFirstConsumer return false
+func IsWaitForFirstConsumerBeforePopulating(pvc *corev1.PersistentVolumeClaim, getDvFunc func(name, namespace string) (*cdiv1.DataVolume, error)) (bool, error) {
+	pvcOwner := metav1.GetControllerOf(pvc)
+	if pvc.Status.Phase == corev1.ClaimPending && pvcOwner != nil && pvcOwner.Kind == "DataVolume" {
+		// Find the data volume:
+		dv, err := getDvFunc(pvcOwner.Name, pvc.Namespace)
+		if err != nil {
+			return false, err
+		}
+		if dv.Status.Phase == cdiv1.WaitForFirstConsumer {
+			return true, nil
+		}
+	}
+	return false, nil
+}

pkg/apis/core/v1alpha1/utils/utils_test.go

@@ -17,7 +17,7 @@
  *
  */
 
-package v1alpha1
+package utils
 
 import (
 	. "github.com/onsi/ginkgo"
@@ -28,12 +28,14 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+
+	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1"
 )
 
 var _ = Describe("IsWaitForFirstConsumerBeforePopulating on Alpha1", func() {
 
 	DescribeTable("PVC with DV as owner",
-		func(volumeClaimPhase corev1.PersistentVolumeClaimPhase, dataVolumePhase DataVolumePhase, expectedResponse bool) {
+		func(volumeClaimPhase corev1.PersistentVolumeClaimPhase, dataVolumePhase cdiv1.DataVolumePhase, expectedResponse bool) {
 
 			dv := newCloneDataVolume("source-dv", "default")
 			dv.Status.Phase = dataVolumePhase
@@ -45,21 +47,21 @@ var _ = Describe("IsWaitForFirstConsumerBeforePopulating on Alpha1", func() {
 				Name:       "source-dv",
 			})
 			res, err := IsWaitForFirstConsumerBeforePopulating(sourcePvc,
-				func(name, namespace string) (*DataVolume, error) {
+				func(name, namespace string) (*cdiv1.DataVolume, error) {
 					return dv, nil
 				})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(res).To(BeEquivalentTo(expectedResponse))
 		},
-		Entry("PVC Pending, dv is in WFFC phase", corev1.ClaimPending, WaitForFirstConsumer, true),
+		Entry("PVC Pending, dv is in WFFC phase", corev1.ClaimPending, cdiv1.WaitForFirstConsumer, true),
-		Entry("PVC Pending, dv is NOT in WFFC phase", corev1.ClaimPending, Pending, false),
+		Entry("PVC Pending, dv is NOT in WFFC phase", corev1.ClaimPending, cdiv1.Pending, false),
-		Entry("PVC Bound, phase does not matter", corev1.ClaimBound, PhaseUnset, false),
+		Entry("PVC Bound, phase does not matter", corev1.ClaimBound, cdiv1.PhaseUnset, false),
 	)
 
 	It("Should return false if source has no ownerRef", func() {
 		sourcePvc := createPvc("test", "default", corev1.ClaimPending)
 		res, err := IsWaitForFirstConsumerBeforePopulating(sourcePvc,
-			func(name, namespace string) (*DataVolume, error) {
+			func(name, namespace string) (*cdiv1.DataVolume, error) {
 				Fail("getDv should never be executed")
 				return nil, nil
 			})
@@ -89,10 +91,10 @@ func createPvc(name, ns string, claimPhase corev1.PersistentVolumeClaimPhase) *c
 	}
 }
 
-func newCloneDataVolume(name string, pvcNamespace string) *DataVolume {
+func newCloneDataVolume(name string, pvcNamespace string) *cdiv1.DataVolume {
 	var annCloneToken = "cdi.kubevirt.io/storage.clone.token"
-	return &DataVolume{
+	return &cdiv1.DataVolume{
-		TypeMeta: metav1.TypeMeta{APIVersion: SchemeGroupVersion.String()},
+		TypeMeta: metav1.TypeMeta{APIVersion: cdiv1.SchemeGroupVersion.String()},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: metav1.NamespaceDefault,
@@ -100,9 +102,9 @@ func newCloneDataVolume(name string, pvcNamespace string) *DataVolume {
 				annCloneToken: "foobar",
 			},
 		},
-		Spec: DataVolumeSpec{
+		Spec: cdiv1.DataVolumeSpec{
-			Source: DataVolumeSource{
+			Source: cdiv1.DataVolumeSource{
-				PVC: &DataVolumeSourcePVC{
+				PVC: &cdiv1.DataVolumeSourcePVC{
 					Name:      "test",
 					Namespace: pvcNamespace,
 				},

pkg/apis/core/v1alpha1/utils/v1alpha1_suite_test.go

@@ -1,4 +1,4 @@
-package v1alpha1
+package utils
 
 import (
 	. "github.com/onsi/ginkgo"

pkg/apis/core/v1beta1/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "go_default_library",
@@ -25,22 +25,3 @@ go_library(
         "//vendor/kubevirt.io/controller-lifecycle-operator-sdk/pkg/sdk/api:go_default_library",
     ],
 )
-
-go_test(
-    name = "go_default_test",
-    srcs = [
-        "utils_test.go",
-        "v1beta1_suite_test.go",
-    ],
-    embed = [":go_default_library"],
-    deps = [
-        "//tests/reporters:go_default_library",
-        "//vendor/github.com/onsi/ginkgo:go_default_library",
-        "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library",
-        "//vendor/github.com/onsi/gomega:go_default_library",
-        "//vendor/k8s.io/api/core/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
-    ],
-)

pkg/apis/core/v1beta1/utils/BUILD.bazel

@@ -0,0 +1,33 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["utils.go"],
+    importpath = "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1/utils",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/apis/core/v1beta1:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = [
+        "utils_test.go",
+        "v1beta1_suite_test.go",
+    ],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/core/v1beta1:go_default_library",
+        "//tests/reporters:go_default_library",
+        "//vendor/github.com/onsi/ginkgo:go_default_library",
+        "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library",
+        "//vendor/github.com/onsi/gomega:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
+    ],
+)

pkg/apis/core/v1beta1/utils/utils.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2020 The CDI Authors.
+
+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 utils
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1"
+)
+
+// IsPopulated indicates if the persistent volume passed in has been fully populated. It follow the following logic
+// 1. If the PVC is not owned by a DataVolume, return true, we assume someone else has properly populated the image
+// 2. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase succeeded return true
+// 3. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase !succeeded return false
+func IsPopulated(pvc *corev1.PersistentVolumeClaim, getDvFunc func(name, namespace string) (*cdiv1.DataVolume, error)) (bool, error) {
+	pvcOwner := metav1.GetControllerOf(pvc)
+	if pvcOwner != nil && pvcOwner.Kind == "DataVolume" {
+		// Find the data volume:
+		dv, err := getDvFunc(pvcOwner.Name, pvc.Namespace)
+		if err != nil {
+			return false, err
+		}
+		if dv.Status.Phase != cdiv1.Succeeded {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+// IsWaitForFirstConsumerBeforePopulating indicates if the persistent volume passed in is in ClaimPending state and waiting for first consumer.
+// It follow the following logic
+// 1. If the PVC is not owned by a DataVolume, return false, we can not assume it will be populated
+// 2. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase WaitForFirstConsumer return true
+// 3. If the PVC is owned by a DataVolume, look up the DV and check the phase, if phase !WaitForFirstConsumer return false
+func IsWaitForFirstConsumerBeforePopulating(pvc *corev1.PersistentVolumeClaim, getDvFunc func(name, namespace string) (*cdiv1.DataVolume, error)) (bool, error) {
+	pvcOwner := metav1.GetControllerOf(pvc)
+	if pvc.Status.Phase == corev1.ClaimPending && pvcOwner != nil && pvcOwner.Kind == "DataVolume" {
+		// Find the data volume:
+		dv, err := getDvFunc(pvcOwner.Name, pvc.Namespace)
+		if err != nil {
+			return false, err
+		}
+		if dv.Status.Phase == cdiv1.WaitForFirstConsumer {
+			return true, nil
+		}
+	}
+	return false, nil
+}

pkg/apis/core/v1beta1/utils/utils_test.go

@@ -17,7 +17,7 @@
  *
  */
 
-package v1beta1
+package utils
 
 import (
 	. "github.com/onsi/ginkgo"
@@ -28,11 +28,13 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+
+	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1"
 )
 
 var _ = Describe("IsWaitForFirstConsumerBeforePopulating on Beta1", func() {
 	DescribeTable("PVC with DV as owner",
-		func(volumeClaimPhase corev1.PersistentVolumeClaimPhase, dataVolumePhase DataVolumePhase, expectedResponse bool) {
+		func(volumeClaimPhase corev1.PersistentVolumeClaimPhase, dataVolumePhase cdiv1.DataVolumePhase, expectedResponse bool) {
 
 			dv := newCloneDataVolume("source-dv", "default")
 			dv.Status.Phase = dataVolumePhase
@@ -44,21 +46,21 @@ var _ = Describe("IsWaitForFirstConsumerBeforePopulating on Beta1", func() {
 				Name:       "source-dv",
 			})
 			res, err := IsWaitForFirstConsumerBeforePopulating(sourcePvc,
-				func(name, namespace string) (*DataVolume, error) {
+				func(name, namespace string) (*cdiv1.DataVolume, error) {
 					return dv, nil
 				})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(res).To(BeEquivalentTo(expectedResponse))
 		},
-		Entry("PVC Pending, dv is in WFFC phase", corev1.ClaimPending, WaitForFirstConsumer, true),
+		Entry("PVC Pending, dv is in WFFC phase", corev1.ClaimPending, cdiv1.WaitForFirstConsumer, true),
-		Entry("PVC Pending, dv is NOT in WFFC phase", corev1.ClaimPending, Pending, false),
+		Entry("PVC Pending, dv is NOT in WFFC phase", corev1.ClaimPending, cdiv1.Pending, false),
-		Entry("PVC Bound, phase does not matter", corev1.ClaimBound, PhaseUnset, false),
+		Entry("PVC Bound, phase does not matter", corev1.ClaimBound, cdiv1.PhaseUnset, false),
 	)
 
 	It("Should return false if source has no ownerRef", func() {
 		sourcePvc := createPvc("test", "default", corev1.ClaimPending)
 		res, err := IsWaitForFirstConsumerBeforePopulating(sourcePvc,
-			func(name, namespace string) (*DataVolume, error) {
+			func(name, namespace string) (*cdiv1.DataVolume, error) {
 				Fail("getDv should never be executed")
 				return nil, nil
 			})
@@ -69,7 +71,7 @@ var _ = Describe("IsWaitForFirstConsumerBeforePopulating on Beta1", func() {
 	It("Should return false if source has no ownerRef", func() {
 		sourcePvc := createPvc("test", "default", corev1.ClaimPending)
 		res, err := IsWaitForFirstConsumerBeforePopulating(sourcePvc,
-			func(name, namespace string) (*DataVolume, error) {
+			func(name, namespace string) (*cdiv1.DataVolume, error) {
 				Fail("getDv should never be executed")
 				return nil, nil
 			})
@@ -99,10 +101,10 @@ func createPvc(name, ns string, claimPhase corev1.PersistentVolumeClaimPhase) *c
 	}
 }
 
-func newCloneDataVolume(name string, pvcNamespace string) *DataVolume {
+func newCloneDataVolume(name string, pvcNamespace string) *cdiv1.DataVolume {
 	var annCloneToken = "cdi.kubevirt.io/storage.clone.token"
-	return &DataVolume{
+	return &cdiv1.DataVolume{
-		TypeMeta: metav1.TypeMeta{APIVersion: SchemeGroupVersion.String()},
+		TypeMeta: metav1.TypeMeta{APIVersion: cdiv1.SchemeGroupVersion.String()},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: metav1.NamespaceDefault,
@@ -110,9 +112,9 @@ func newCloneDataVolume(name string, pvcNamespace string) *DataVolume {
 				annCloneToken: "foobar",
 			},
 		},
-		Spec: DataVolumeSpec{
+		Spec: cdiv1.DataVolumeSpec{
-			Source: DataVolumeSource{
+			Source: cdiv1.DataVolumeSource{
-				PVC: &DataVolumeSourcePVC{
+				PVC: &cdiv1.DataVolumeSourcePVC{
 					Name:      "test",
 					Namespace: pvcNamespace,
 				},

pkg/apis/core/v1beta1/utils/v1beta1_suite_test.go

@@ -1,4 +1,4 @@
-package v1beta1
+package utils
 
 import (
 	. "github.com/onsi/ginkgo"

pkg/controller/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/apis/core/v1beta1:go_default_library",
+        "//pkg/apis/core/v1beta1/utils:go_default_library",
         "//pkg/common:go_default_library",
         "//pkg/feature-gates:go_default_library",
         "//pkg/operator:go_default_library",

pkg/controller/util.go

@@ -20,6 +20,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1"
+	cdiv1utils "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1/utils"
 	"kubevirt.io/containerized-data-importer/pkg/common"
 	"kubevirt.io/containerized-data-importer/pkg/util/cert"
 	"kubevirt.io/containerized-data-importer/pkg/util/naming"
@@ -595,7 +596,7 @@ func GetActiveCDI(c client.Client) (*cdiv1.CDI, error) {
 
 // IsPopulated returns if the passed in PVC has been populated according to the rules outlined in pkg/apis/core/<version>/utils.go
 func IsPopulated(pvc *v1.PersistentVolumeClaim, c client.Client) (bool, error) {
-	return cdiv1.IsPopulated(pvc, func(name, namespace string) (*cdiv1.DataVolume, error) {
+	return cdiv1utils.IsPopulated(pvc, func(name, namespace string) (*cdiv1.DataVolume, error) {
 		dv := &cdiv1.DataVolume{}
 		err := c.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, dv)
 		return dv, err