Removes GetOwnerReference() from the kclient

pkg/devfile/adapters/kubernetes/component/adapter.go

@@ -442,11 +442,11 @@ func (a Adapter) createOrUpdateComponent(componentExists bool, ei envinfo.EnvSpe
 	}
 
 	// update the owner reference of the PVCs with the deployment
-	for _, pvc := range pvcs {
-		if pvc.OwnerReferences != nil || pvc.DeletionTimestamp != nil {
+	for i := range pvcs {
+		if pvcs[i].OwnerReferences != nil || pvcs[i].DeletionTimestamp != nil {
 			continue
 		}
-		err = a.Client.UpdateStorageOwnerReference(&pvc, generator.GetOwnerReference(deployment))
+		err = a.Client.UpdateStorageOwnerReference(&pvcs[i], generator.GetOwnerReference(deployment))
 		if err != nil {
 			return err
 		}

pkg/devfile/adapters/kubernetes/storage/utils.go

@@ -18,8 +18,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
-const pvcNameMaxLen = 45
-
 // CreateComponentStorage creates PVCs with the given list of storages if it does not exist, else it uses the existing PVC
 func CreateComponentStorage(Client *kclient.Client, storages []common.Storage, componentName string) (err error) {
 

pkg/devfile/adapters/kubernetes/utils/utils.go

@@ -369,7 +369,7 @@ func HandleEphemeralStorage(client kclient.Client, storageClient storage.Client,
 			for _, pvc := range pvcs {
 				err := client.DeletePVC(pvc.Name)
 				if err != nil {
-
+					return err
 				}
 			}
 		}

pkg/kclient/kclient.go

@@ -228,19 +228,3 @@ func (c *Client) IsResourceSupported(apiGroup, apiVersion, resourceName string)
 	}
 	return supported, nil
 }
-
-// GenerateOwnerReference generates an ownerReference which can then be set as
-// owner for various Kubernetes objects and ensure that when the owner object is
-// deleted from the cluster, all other objects are automatically removed by
-// Kubernetes garbage collector
-func GenerateOwnerReference(deployment appsv1.Deployment) metav1.OwnerReference {
-
-	ownerReference := metav1.OwnerReference{
-		APIVersion: "apps.openshift.io/v1",
-		Kind:       "Deployment",
-		Name:       deployment.Name,
-		UID:        deployment.UID,
-	}
-
-	return ownerReference
-}

pkg/kclient/volumes_test.go

@@ -365,7 +365,7 @@ func TestUpdateStorageOwnerReference(t *testing.T) {
 			args: args{
 				pvc: testingutil.FakePVC("pvc-1", "1Gi", map[string]string{}),
 				ownerReference: []metav1.OwnerReference{
-					GenerateOwnerReference(*fakeDeployment),
+					generator.GetOwnerReference(fakeDeployment),
 				},
 			},
 			wantErr: false,

pkg/storage/kubernetes.go

@@ -32,7 +32,6 @@ func (k kubernetesClient) Create(storage Storage) error {
 	}
 
 	labels := storagelabels.GetLabels(storage.Name, k.componentName, k.appName, true)
-	labels[storagelabels.DevfileStorageLabel] = storage.Name
 
 	if strings.Contains(storage.Name, OdoSourceVolume) {
 		// Add label for source pvc
@@ -54,7 +53,7 @@ func (k kubernetesClient) Create(storage Storage) error {
 
 	// Create PVC
 	klog.V(2).Infof("Creating a PVC with name %v and labels %v", pvcName, labels)
-	pvc, err = k.client.GetKubeClient().CreatePVC(*pvc)
+	_, err = k.client.GetKubeClient().CreatePVC(*pvc)
 	if err != nil {
 		return errors.Wrap(err, "unable to create PVC")
 	}

pkg/storage/kubernetes_test.go

@@ -531,7 +531,6 @@ func Test_kubernetesClient_List(t *testing.T) {
 func Test_kubernetesClient_Create(t *testing.T) {
 	type fields struct {
 		generic generic
-		client  occlient.Client
 	}
 	type args struct {
 		storage Storage
@@ -612,7 +611,6 @@ func Test_kubernetesClient_Create(t *testing.T) {
 			}
 
 			wantLabels := storageLabels.GetLabels(tt.args.storage.Name, tt.fields.generic.componentName, tt.fields.generic.appName, true)
-			wantLabels[storageLabels.DevfileStorageLabel] = tt.args.storage.Name
 
 			if strings.Contains(tt.args.storage.Name, OdoSourceVolume) {
 				// Add label for source pvc
@@ -621,7 +619,7 @@ func Test_kubernetesClient_Create(t *testing.T) {
 
 			// created PVC should be labeled with labels passed to CreatePVC
 			if !reflect.DeepEqual(createdPVC.Labels, wantLabels) {
-				t.Errorf("labels in created route is not matching expected labels, expected: %v, got: %v", wantLabels, createdPVC.Labels)
+				t.Errorf("labels in created pvc is not matching expected labels, expected: %v, got: %v", wantLabels, createdPVC.Labels)
 			}
 			// name, size of createdPVC should be matching to size, name passed to CreatePVC
 			if !reflect.DeepEqual(createdPVC.Spec.Resources.Requests["storage"], quantity) {
@@ -641,7 +639,6 @@ func Test_kubernetesClient_Delete(t *testing.T) {
 
 	type fields struct {
 		generic generic
-		client  occlient.Client
 	}
 	type args struct {
 		name string

pkg/storage/storage.go

@@ -680,8 +680,8 @@ func Push(client Client, configProvider localConfigProvider.LocalConfigProvider)
 	}
 
 	// find storage to delete
-	for _, storage := range storageClusterList.Items {
-		val, ok := storageConfigNames[storage.Name]
+	for storageName, storage := range storageClusterNames {
+		val, ok := storageConfigNames[storageName]
 		if !ok {
 			// delete the pvc
 			err = client.Delete(storage.Name)
@@ -691,15 +691,15 @@ func Push(client Client, configProvider localConfigProvider.LocalConfigProvider)
 			log.Successf("Deleted storage %v from %v", storage.Name, configProvider.GetName())
 			continue
 		} else if storage.Name == val.Name {
-			if val.Spec.Size != storage.Spec.Size || val.Spec.Path != storage.Spec.Path {
+			if val.Spec.Size != storage.Spec.Size {
 				return errors.Errorf("config mismatch for storage with the same name %s", storage.Name)
 			}
 		}
 	}
 
 	// find storage to create
-	for _, storage := range ConvertListLocalToMachine(localStorage).Items {
-		_, ok := storageClusterNames[storage.Name]
+	for storageName, storage := range storageConfigNames {
+		_, ok := storageClusterNames[storageName]
 		if !ok {
 			err := client.Create(storage)
 			if err != nil {

pkg/storage/storage_test.go

@@ -1053,13 +1053,8 @@ func TestPush(t *testing.T) {
 	clusterStorage0 := GetMachineFormatWithContainer("storage-0", "1Gi", "/data", "runtime-0")
 	clusterStorage1 := GetMachineFormatWithContainer("storage-1", "5Gi", "/path", "runtime-1")
 
-	type args struct {
-		client         Client
-		configProvider localConfigProvider.LocalConfigProvider
-	}
 	tests := []struct {
 		name                string
-		args                args
 		returnedFromLocal   []localConfigProvider.LocalStorage
 		returnedFromCluster StorageList
 		createdItems        []localConfigProvider.LocalStorage
@@ -1136,7 +1131,7 @@ func TestPush(t *testing.T) {
 		},
 		{
 			name: "case 6: spec mismatch",
-			returnedFromLocal: []localConfigProvider.LocalStorage{localStorage0,
+			returnedFromLocal: []localConfigProvider.LocalStorage{
 				{
 					Name:      "storage-1",
 					Size:      "3Gi",
@@ -1146,14 +1141,66 @@ func TestPush(t *testing.T) {
 			},
 			returnedFromCluster: StorageList{
 				Items: []Storage{
-					clusterStorage0,
 					clusterStorage1,
 				},
 			},
 			createdItems: []localConfigProvider.LocalStorage{},
 			deletedItems: []string{},
 			wantErr:      true,
 		},
+		{
+			name: "case 7: only one PVC created for two storage with same name but on different containers",
+			returnedFromLocal: []localConfigProvider.LocalStorage{
+				{
+					Name:      "storage-0",
+					Size:      "1Gi",
+					Path:      "/data",
+					Container: "runtime-0",
+				},
+				{
+					Name:      "storage-0",
+					Size:      "1Gi",
+					Path:      "/path",
+					Container: "runtime-1",
+				},
+			},
+			returnedFromCluster: StorageList{},
+			createdItems: []localConfigProvider.LocalStorage{
+				{
+					Name:      "storage-0",
+					Size:      "1Gi",
+					Path:      "/path",
+					Container: "runtime-1",
+				},
+			},
+		},
+		{
+			name: "case 8: only path spec mismatch",
+			returnedFromLocal: []localConfigProvider.LocalStorage{
+				{
+					Name:      "storage-1",
+					Size:      "5Gi",
+					Path:      "/data",
+					Container: "runtime-1",
+				},
+			},
+			returnedFromCluster: StorageList{
+				Items: []Storage{
+					clusterStorage1,
+				},
+			},
+		},
+		{
+			name:              "case 9: only one PVC deleted for two storage with same name but on different containers",
+			returnedFromLocal: []localConfigProvider.LocalStorage{},
+			returnedFromCluster: StorageList{
+				Items: []Storage{
+					GetMachineFormatWithContainer("storage-0", "1Gi", "/data", "runtime-0"),
+					GetMachineFormatWithContainer("storage-0", "1Gi", "/data", "runtime-1"),
+				},
+			},
+			deletedItems: []string{"storage-0"},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

tests/helper/helper_kubectl.go

@@ -74,7 +74,7 @@ func (kubectl KubectlRunner) GetRunningPodNameByComponent(compName string, names
 
 // GetPVCSize executes kubectl command and returns the bound storage size
 func (kubectl KubectlRunner) GetPVCSize(compName, storageName, namespace string) string {
-	selector := fmt.Sprintf("--selector=storage-name=%s,component=%s", storageName, compName)
+	selector := fmt.Sprintf("--selector=app.kubernetes.io/storage-name=%s,app.kubernetes.io/instance=%s", storageName, compName)
 	stdOut := CmdShouldPass(kubectl.path, "get", "pvc", "--namespace", namespace, selector, "-o", "jsonpath={.items[*].spec.resources.requests.storage}")
 	return strings.TrimSpace(stdOut)
 }

tests/helper/helper_oc.go

@@ -383,7 +383,7 @@ func (oc OcRunner) GetRunningPodNameByComponent(compName string, namespace strin
 
 // GetPVCSize executes oc command and returns the bound storage size
 func (oc OcRunner) GetPVCSize(compName, storageName, namespace string) string {
-	selector := fmt.Sprintf("--selector=storage-name=%s,component=%s", storageName, compName)
+	selector := fmt.Sprintf("--selector=app.kubernetes.io/storage-name=%s,app.kubernetes.io/instance=%s", storageName, compName)
 	stdOut := CmdShouldPass(oc.path, "get", "pvc", "--namespace", namespace, selector, "-o", "jsonpath={.items[*].spec.resources.requests.storage}")
 	return strings.TrimSpace(stdOut)
 }