From df700115b838e6a03a607c56780d5f3b2acf1b04 Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Wed, 25 Oct 2023 14:33:57 +0200 Subject: [PATCH] feat: allow image deletion for in-deleting pools if a pool has a deletionTimestamp set, it's fine to not validate the pool-CR again to force the deletion of the CR Signed-off-by: Mario Constanti --- api/v1alpha1/image_webhook.go | 7 ++-- api/v1alpha1/pool_webhook.go | 67 ++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/api/v1alpha1/image_webhook.go b/api/v1alpha1/image_webhook.go index 172ea19a..033fbebf 100644 --- a/api/v1alpha1/image_webhook.go +++ b/api/v1alpha1/image_webhook.go @@ -66,8 +66,11 @@ func (i *Image) attachedPools(ctx context.Context) ([]Pool, error) { } for _, pool := range pools.Items { - if pool.Spec.ImageName == i.Name { - result = append(result, pool) + // we do not care about pools that are already deleted + if pool.GetDeletionTimestamp() == nil { + if pool.Spec.ImageName == i.Name { + result = append(result, pool) + } } } diff --git a/api/v1alpha1/pool_webhook.go b/api/v1alpha1/pool_webhook.go index 19fe1df3..5477a97f 100644 --- a/api/v1alpha1/pool_webhook.go +++ b/api/v1alpha1/pool_webhook.go @@ -77,7 +77,7 @@ func (r *Pool) ValidateCreate() (admission.Warnings, error) { if len(poolList.Items) > 0 { existing := poolList.Items[0] return nil, apierrors.NewBadRequest( - fmt.Sprintf("can not create pool, pool=%s with same image=%s , flavor=%s and provider=%s already exists for specified GitHubScope=%s", existing.Name, existing.Spec.ImageName, existing.Spec.Flavor, existing.Spec.ProviderName, existing.Spec.GitHubScopeRef.Name)) + fmt.Sprintf("can not create pool, pool=%s with same image=%s, flavor=%s and provider=%s already exists for specified GitHubScope=%s", existing.Name, existing.Spec.ImageName, existing.Spec.Flavor, existing.Spec.ProviderName, existing.Spec.GitHubScopeRef.Name)) } return nil, nil @@ -85,43 +85,46 @@ func (r *Pool) ValidateCreate() (admission.Warnings, error) { // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type func (r *Pool) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - poollog.Info("validate update", "name", r) + poollog.Info("validate update", "name", r.Name, "namespace", r.Namespace) oldCRD, ok := old.(*Pool) if !ok { return nil, apierrors.NewBadRequest("failed to convert runtime.Object to Pool CRD") } - err := validateImageName(context.Background(), r) - if err != nil { - return nil, apierrors.NewInvalid( - schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, - r.Name, - field.ErrorList{err}, - ) - } - - if err := r.validateExtraSpec(); err != nil { - return nil, apierrors.NewInvalid(schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, - r.Name, - field.ErrorList{err}, - ) - } - - if err := r.validateProviderName(oldCRD); err != nil { - return nil, apierrors.NewInvalid( - schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, - r.Name, - field.ErrorList{err}, - ) - } - - if err := r.validateGitHubScope(oldCRD); err != nil { - return nil, apierrors.NewInvalid( - schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, - r.Name, - field.ErrorList{err}, - ) + // if the object is being deleted, skip validation + if r.GetDeletionTimestamp() == nil { + err := validateImageName(context.Background(), r) + if err != nil { + return nil, apierrors.NewInvalid( + schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, + r.Name, + field.ErrorList{err}, + ) + } + + if err := r.validateExtraSpec(); err != nil { + return nil, apierrors.NewInvalid(schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, + r.Name, + field.ErrorList{err}, + ) + } + + if err := r.validateProviderName(oldCRD); err != nil { + return nil, apierrors.NewInvalid( + schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, + r.Name, + field.ErrorList{err}, + ) + } + + if err := r.validateGitHubScope(oldCRD); err != nil { + return nil, apierrors.NewInvalid( + schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"}, + r.Name, + field.ErrorList{err}, + ) + } } return nil, nil