Skip to content

Commit

Permalink
Handle force delete for bastion resource (gardener#8608)
Browse files Browse the repository at this point in the history
* Add ForceDelete for container runtime

* Handle bastion force deletion

Co-Authored-By: Shafeeque E S <shafeeque.e.s@sap.com>

* Add integration test

---------

Co-authored-by: Shafeeque E S <shafeeque.e.s@sap.com>
  • Loading branch information
acumino and shafeeqes authored Oct 6, 2023
1 parent fe36f98 commit c29e975
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 44 deletions.
4 changes: 3 additions & 1 deletion extensions/pkg/controller/bastion/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ import (

extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
v1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
"github.com/gardener/gardener/pkg/controllerutils"
reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler"
"github.com/gardener/gardener/pkg/extensions"
kubernetesutils "github.com/gardener/gardener/pkg/utils/kubernetes"
)

type reconciler struct {
Expand Down Expand Up @@ -148,7 +150,7 @@ func (r *reconciler) delete(

log.Info("Starting the deletion of Bastion")
var err error
if cluster != nil && v1beta1helper.ShootNeedsForceDeletion(cluster.Shoot) {
if kubernetesutils.HasMetaDataAnnotation(&bastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true") {
err = r.actuator.ForceDelete(ctx, log, bastion, cluster)
} else {
err = r.actuator.Delete(ctx, log, bastion, cluster)
Expand Down
2 changes: 2 additions & 0 deletions extensions/pkg/controller/containerruntime/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ type Actuator interface {
Reconcile(context.Context, logr.Logger, *extensionsv1alpha1.ContainerRuntime, *extensionscontroller.Cluster) error
// Delete the ContainerRuntime resource.
Delete(context.Context, logr.Logger, *extensionsv1alpha1.ContainerRuntime, *extensionscontroller.Cluster) error
// ForceDelete forcefully deletes the ContainerRuntime.
ForceDelete(context.Context, logr.Logger, *extensionsv1alpha1.ContainerRuntime, *extensionscontroller.Cluster) error
// Restore the ContainerRuntime resource.
Restore(context.Context, logr.Logger, *extensionsv1alpha1.ContainerRuntime, *extensionscontroller.Cluster) error
// Migrate the ContainerRuntime resource.
Expand Down
8 changes: 7 additions & 1 deletion extensions/pkg/controller/containerruntime/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,13 @@ func (r *reconciler) delete(
return reconcile.Result{}, err
}

if err := r.actuator.Delete(ctx, log, cr, cluster); err != nil {
var err error
if cluster != nil && v1beta1helper.ShootNeedsForceDeletion(cluster.Shoot) {
err = r.actuator.ForceDelete(ctx, log, cr, cluster)
} else {
err = r.actuator.Delete(ctx, log, cr, cluster)
}
if err != nil {
_ = r.statusUpdater.Error(ctx, log, cr, reconcilerutils.ReconcileErrCauseOrErr(err), gardencorev1beta1.LastOperationTypeDelete, "Error deleting ContainerRuntime")
return reconcilerutils.ReconcileErr(err)
}
Expand Down
16 changes: 15 additions & 1 deletion pkg/gardenlet/controller/bastion/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,22 @@ func (r *Reconciler) cleanupBastion(
return fmt.Errorf("failed patching ready condition of Bastion: %w", err)
}

// delete bastion extension resource in seed cluster
extensionBastion := newBastionExtension(bastion, shoot)

if kubernetesutils.HasMetaDataAnnotation(&bastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true") {
if err := r.SeedClient.Get(seedCtx, client.ObjectKeyFromObject(extensionBastion), extensionBastion); client.IgnoreNotFound(err) != nil {
return fmt.Errorf("failed getting extension bastion %s: %w", client.ObjectKeyFromObject(extensionBastion), err)
}

if !kubernetesutils.HasMetaDataAnnotation(&extensionBastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true") {
patch := client.MergeFrom(extensionBastion.DeepCopy())
metav1.SetMetaDataAnnotation(&extensionBastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true")
if err := r.SeedClient.Patch(seedCtx, extensionBastion, patch); client.IgnoreNotFound(err) != nil {
return fmt.Errorf("failed patching extension bastion %s: %w", client.ObjectKeyFromObject(extensionBastion), err)
}
}
}

if err := r.SeedClient.Delete(seedCtx, extensionBastion); err != nil {
if apierrors.IsNotFound(err) {
log.Info("Successfully deleted")
Expand Down
1 change: 0 additions & 1 deletion pkg/gardenlet/controller/shoot/shoot/cleaner.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ var (

var (
extensionKindToObjectList = map[string]client.ObjectList{
extensionsv1alpha1.BastionResource: &extensionsv1alpha1.BastionList{},
extensionsv1alpha1.ContainerRuntimeResource: &extensionsv1alpha1.ContainerRuntimeList{},
extensionsv1alpha1.ControlPlaneResource: &extensionsv1alpha1.ControlPlaneList{},
extensionsv1alpha1.DNSRecordResource: &extensionsv1alpha1.DNSRecordList{},
Expand Down
20 changes: 4 additions & 16 deletions pkg/gardenlet/controller/shoot/shoot/cleaner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ var _ = Describe("cleaner", func() {
secret2 *corev1.Secret
configMap1 *corev1.ConfigMap
configMap2 *corev1.ConfigMap
bastion *extensionsv1alpha1.Bastion
cluster *extensionsv1alpha1.Cluster
controlPlane *extensionsv1alpha1.ControlPlane
extension *extensionsv1alpha1.Extension
Expand Down Expand Up @@ -102,12 +101,6 @@ var _ = Describe("cleaner", func() {
Namespace: namespace,
},
}
bastion = &extensionsv1alpha1.Bastion{
ObjectMeta: metav1.ObjectMeta{
Name: "test-obj",
Namespace: namespace,
},
}
cluster = &extensionsv1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
Expand Down Expand Up @@ -172,19 +165,18 @@ var _ = Describe("cleaner", func() {

Describe("#DeleteExtensionObjects", func() {
It("should successfully delete all extension objects in the given namespace", func() {
bastion.Finalizers = []string{finalizer}
controlPlane.Finalizers = []string{finalizer}
extension.Finalizers = []string{finalizer}

copies := makeCopies(otherNamespace, bastion.DeepCopy(), controlPlane.DeepCopy(), extension.DeepCopy())
copies := makeCopies(otherNamespace, controlPlane.DeepCopy(), extension.DeepCopy())

for _, object := range append([]client.Object{bastion, controlPlane, extension}, copies...) {
for _, object := range append([]client.Object{controlPlane, extension}, copies...) {
Expect(seedClient.Create(ctx, object)).To(Succeed())
}

Expect(cleaner.DeleteExtensionObjects(ctx)).To(Succeed())

for _, object := range []client.Object{bastion, controlPlane, extension} {
for _, object := range []client.Object{controlPlane, extension} {
Expect(seedClient.Get(ctx, client.ObjectKeyFromObject(object), object)).To(Succeed())
Expect(object.GetDeletionTimestamp()).NotTo(BeNil())
}
Expand All @@ -198,23 +190,19 @@ var _ = Describe("cleaner", func() {

Describe("#WaitUntilExtensionObjectsDeleted", func() {
It("should fail to delete if extension has status.lastError", func() {
bastion.Status.LastError = &gardencorev1beta1.LastError{
Description: "invalid credentials",
}
controlPlane.Status.LastError = &gardencorev1beta1.LastError{
Description: "invalid credentials",
}
extension.Status.LastError = &gardencorev1beta1.LastError{
Description: "invalid credentials",
}

for _, object := range []client.Object{bastion, controlPlane, extension} {
for _, object := range []client.Object{controlPlane, extension} {
Expect(seedClient.Create(ctx, object)).To(Succeed())
}

err := cleaner.WaitUntilExtensionObjectsDeleted(ctx)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Failed to delete Bastion"))
Expect(err.Error()).To(ContainSubstring("Failed to delete ControlPlane"))
Expect(err.Error()).To(ContainSubstring("Failed to delete Extension"))
})
Expand Down
45 changes: 45 additions & 0 deletions pkg/gardenlet/controller/shoot/shoot/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,17 @@ func (r *Reconciler) deleteShoot(ctx context.Context, log logr.Logger, shoot *ga
return reconcile.Result{}, fmt.Errorf("failed to check for related Bastions: %w", err)
}
if hasBastions {
if v1beta1helper.ShootNeedsForceDeletion(shoot) {
bastionsPatched, err := r.patchBastions(ctx, shoot)
if err != nil {
return reconcile.Result{}, err
}

if bastionsPatched {
return reconcile.Result{RequeueAfter: 15 * time.Second}, nil
}
}

hasBastionErr := errors.New("shoot has still Bastions")
updateErr := r.patchShootStatusOperationError(ctx, shoot, hasBastionErr.Error(), operationType, shoot.Status.LastErrors...)
return reconcile.Result{}, errorsutils.WithSuppressed(hasBastionErr, updateErr)
Expand Down Expand Up @@ -880,6 +891,40 @@ func needsControlPlaneDeployment(ctx context.Context, o *operation.Operation, ku
}
}

func (r *Reconciler) patchBastions(ctx context.Context, shoot *gardencorev1beta1.Shoot) (bool, error) {
var (
fns []flow.TaskFn
bastions = &operationsv1alpha1.BastionList{}
bastionsPatched bool
)

if err := r.GardenClient.List(ctx, bastions, client.MatchingFields{operations.BastionShootName: shoot.Name}); err != nil {
return false, fmt.Errorf("failed to list related Bastions: %w", err)
}

for _, b := range bastions.Items {
bastion := b

if !kubernetesutils.HasMetaDataAnnotation(&bastion, v1beta1constants.AnnotationConfirmationForceDeletion, "true") {
fns = append(fns, func(ctx context.Context) error {
patch := client.MergeFrom(bastion.DeepCopy())
metav1.SetMetaDataAnnotation(&bastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true")
if err := r.GardenClient.Patch(ctx, &bastion, patch); err != nil {
return fmt.Errorf("failed to patch bastion %s: %w", client.ObjectKeyFromObject(&bastion), err)
}
bastionsPatched = true
return nil
})
}
}

if err := flow.Parallel(fns...)(ctx); err != nil {
return false, fmt.Errorf("failed to patch Bastions: %w", err)
}

return bastionsPatched, nil
}

func extensionResourceStillExists(ctx context.Context, reader client.Reader, obj client.Object, namespace, name string) (bool, bool, error) {
if err := reader.Get(ctx, kubernetesutils.Key(namespace, name), obj); err != nil {
if apierrors.IsNotFound(err) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/registry/operations/bastion/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ func mustIncreaseGeneration(oldBastion, newBastion *operations.Bastion) bool {
return true
}

if kubernetesutils.HasMetaDataAnnotation(&newBastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true") &&
!kubernetesutils.HasMetaDataAnnotation(&oldBastion.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true") {
return true
}

return false
}

Expand Down
55 changes: 55 additions & 0 deletions pkg/registry/operations/bastion/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,61 @@ var _ = Describe("PrepareForUpdate", func() {
Expect(bastion.Status.ExpirationTimestamp).NotTo(BeNil())
Expect(bastion.Annotations[v1beta1constants.GardenerOperation]).To(BeEmpty())
})

Context("generation increment", func() {
var (
oldBastion *operations.Bastion
newBastion *operations.Bastion
)

BeforeEach(func() {
oldBastion = &operations.Bastion{}
newBastion = oldBastion.DeepCopy()
})

DescribeTable("standard tests",
func(mutateNewBastion func(*operations.Bastion), shouldIncreaseGeneration bool) {
if mutateNewBastion != nil {
mutateNewBastion(newBastion)
}

Strategy.PrepareForUpdate(context.TODO(), newBastion, oldBastion)

expectedGeneration := oldBastion.Generation
if shouldIncreaseGeneration {
expectedGeneration++
}

Expect(newBastion.Generation).To(Equal(expectedGeneration))
},

Entry("no change",
nil,
false,
),
Entry("only label change",
func(b *operations.Bastion) { b.Labels = map[string]string{"foo": "bar"} },
false,
),
Entry("some spec change",
func(b *operations.Bastion) { b.Spec.SSHPublicKey = "foo" },
true,
),
Entry("deletion timestamp gets set",
func(b *operations.Bastion) {
deletionTimestamp := metav1.Now()
b.DeletionTimestamp = &deletionTimestamp
},
true,
),
Entry("force-deletion annotation",
func(b *operations.Bastion) {
metav1.SetMetaDataAnnotation(&b.ObjectMeta, v1beta1constants.AnnotationConfirmationForceDeletion, "true")
},
true,
),
)
})
})

var _ = Describe("heartbeat", func() {
Expand Down
Loading

0 comments on commit c29e975

Please sign in to comment.