From 01ad37fc40489d9d241e79dc97779d1e91936910 Mon Sep 17 00:00:00 2001 From: Arik Hadas Date: Mon, 18 Dec 2023 23:46:00 +0200 Subject: [PATCH 1/2] Do not set owner for PV of OVA provider There is no point in setting the OVA provider as the owner of the PV we create for it since the PV is not namespace-scoped and thus, it is not removed when the OVA provider is removed. Signed-off-by: Arik Hadas --- pkg/controller/provider/ova-setup.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/controller/provider/ova-setup.go b/pkg/controller/provider/ova-setup.go index 705078e1b..2f445052b 100644 --- a/pkg/controller/provider/ova-setup.go +++ b/pkg/controller/provider/ova-setup.go @@ -27,19 +27,19 @@ const ( ) func (r Reconciler) CreateOVAServerDeployment(provider *api.Provider, ctx context.Context) { - ownerReference := metav1.OwnerReference{ - APIVersion: "forklift.konveyor.io/v1beta1", - Kind: "Provider", - Name: provider.Name, - UID: provider.UID, - } pvName := fmt.Sprintf("%s-pv-%s-%s", ovaServer, provider.Name, provider.Namespace) - err := r.createPvForNfs(provider, ctx, ownerReference, pvName) + err := r.createPvForNfs(provider, ctx, pvName) if err != nil { r.Log.Error(err, "Failed to create PV for the OVA server") return } + ownerReference := metav1.OwnerReference{ + APIVersion: "forklift.konveyor.io/v1beta1", + Kind: "Provider", + Name: provider.Name, + UID: provider.UID, + } pvcName := fmt.Sprintf("%s-pvc-%s", ovaServer, provider.Name) err = r.createPvcForNfs(provider, ctx, ownerReference, pvName, pvcName) if err != nil { @@ -61,7 +61,7 @@ func (r Reconciler) CreateOVAServerDeployment(provider *api.Provider, ctx contex } } -func (r *Reconciler) createPvForNfs(provider *api.Provider, ctx context.Context, ownerReference metav1.OwnerReference, pvName string) (err error) { +func (r *Reconciler) createPvForNfs(provider *api.Provider, ctx context.Context, pvName string) (err error) { splitted := strings.Split(provider.Spec.URL, ":") nfsServer := splitted[0] nfsPath := splitted[1] @@ -69,9 +69,8 @@ func (r *Reconciler) createPvForNfs(provider *api.Provider, ctx context.Context, pv := &core.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ - Name: pvName, - OwnerReferences: []metav1.OwnerReference{ownerReference}, - Labels: labels, + Name: pvName, + Labels: labels, }, Spec: core.PersistentVolumeSpec{ Capacity: core.ResourceList{ From 1a48c6b2c72380f82133de060022367853ff68e2 Mon Sep 17 00:00:00 2001 From: Arik Hadas Date: Mon, 18 Dec 2023 23:49:00 +0200 Subject: [PATCH 2/2] Set reclaim policy of PV of OVA provider to Delete The PV is currently not set with a reclaim policy and therefore it defaults to 'Retain' which is conceptually incorrect since it is supposed to be removed when the OVA provider is removed. Here we set the reclaim policy of the PV to 'Delete' instead. While the PV would still remain after the OVA provider is removed (since the PV is not set with a storage class and therefore there is no driver that can remove it), the state of the PV would change to 'Failed' rather than 'Released' in this case, which better reflects its true state. Removing the PV for an OVA provider when the latter is removed will be addressed separately. Signed-off-by: Arik Hadas --- pkg/controller/provider/ova-setup.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/provider/ova-setup.go b/pkg/controller/provider/ova-setup.go index 2f445052b..a4bc7476f 100644 --- a/pkg/controller/provider/ova-setup.go +++ b/pkg/controller/provider/ova-setup.go @@ -85,6 +85,7 @@ func (r *Reconciler) createPvForNfs(provider *api.Provider, ctx context.Context, Server: nfsServer, }, }, + PersistentVolumeReclaimPolicy: core.PersistentVolumeReclaimDelete, }, } err = r.Create(ctx, pv)