Skip to content

Commit

Permalink
Skip explicit LB deletion for Kubernetes >= v1.16 (gardener#290)
Browse files Browse the repository at this point in the history
  • Loading branch information
rfranzke committed Mar 5, 2021
1 parent 52c7bec commit 2ad7820
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 6 deletions.
36 changes: 30 additions & 6 deletions pkg/controller/infrastructure/actuator_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,35 @@ import (
gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
"github.com/gardener/gardener/pkg/utils/flow"
versionutils "github.com/gardener/gardener/pkg/utils/version"
"github.com/go-logr/logr"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func (a *actuator) Delete(ctx context.Context, infrastructure *extensionsv1alpha1.Infrastructure, _ *extensionscontroller.Cluster) error {
func (a *actuator) Delete(ctx context.Context, infrastructure *extensionsv1alpha1.Infrastructure, cluster *extensionscontroller.Cluster) error {
logger := a.logger.WithValues("infrastructure", client.ObjectKeyFromObject(infrastructure), "operation", "delete")
return Delete(ctx, logger, a.RESTConfig(), a.Client(), infrastructure)

// Prior to Kubernetes v1.16 there was no finalizer support for `Service` objects. This is especially painful for
// `Service`s of type `LoadBalancer` because infrastructure artefacts are created. An deletion of the `Service`
// object leads to immediate disappearance from the system/cluster while the service-controller is still processing
// the deletion of the infrastructure artefacts in the background. Consequently, as an end-user you cannot know when
// the service-controller finished its work and cleaned up properly.
// If it didn't finish the destruction fast enough then the infrastructure deletion (trying to destroy the subnets,
// VPC, etc.) will fail due to the fact that there are still resources (load balancers, security groups) that we
// are not aware of. As a result, manual intervention is required to cleanup those artefacts to allow our infrastructure
// deletion to properly proceed.
// To mitigate this, we are explicitly deletion load balancers and security groups for Kubernetes versions lower
// than v1.16 (in fact, we are duplicating the logic of the service-controller here (at least parts of it)).
// See more details: https://github.com/kubernetes/enhancements/issues/980
// TODO: Remove all this code when the extension does only support clusters with at least Kubernetes v1.16.
needsExplicitLoadBalancerDeletion, err := versionutils.CheckVersionMeetsConstraint(cluster.Shoot.Spec.Kubernetes.Version, "< 1.16")
if err != nil {
return err
}

return Delete(ctx, logger, a.RESTConfig(), a.Client(), infrastructure, needsExplicitLoadBalancerDeletion)
}

// Delete deletes the given Infrastructure.
Expand All @@ -45,6 +65,7 @@ func Delete(
restConfig *rest.Config,
c client.Client,
infrastructure *extensionsv1alpha1.Infrastructure,
needsExplicitLoadBalancerDeletion bool,
) error {
tf, err := newTerraformer(logger, restConfig, aws.TerraformerPurposeInfra, infrastructure)
if err != nil {
Expand Down Expand Up @@ -73,9 +94,12 @@ func Delete(
return err
}

awsClient, err := awsclient.NewClient(string(credentials.AccessKeyID), string(credentials.SecretAccessKey), infrastructure.Spec.Region)
if err != nil {
return err
var awsClient *awsclient.Client
if needsExplicitLoadBalancerDeletion {
awsClient, err = awsclient.NewClient(string(credentials.AccessKeyID), string(credentials.SecretAccessKey), infrastructure.Spec.Region)
if err != nil {
return err
}
}

var (
Expand All @@ -97,7 +121,7 @@ func Delete(
return gardencorev1beta1helper.DetermineError(err, fmt.Sprintf("Failed to destroy load balancers and security groups: %+v", err.Error()))
}
return nil
}).RetryUntilTimeout(10*time.Second, 5*time.Minute).DoIf(configExists),
}).RetryUntilTimeout(10*time.Second, 5*time.Minute).DoIf(needsExplicitLoadBalancerDeletion && configExists),
})

_ = g.Add(flow.Task{
Expand Down
16 changes: 16 additions & 0 deletions test/integration/infrastructure/infrastructure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/iam"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
"github.com/gardener/gardener/pkg/operation/common"
gardenerutils "github.com/gardener/gardener/pkg/utils"
Expand Down Expand Up @@ -300,6 +301,21 @@ func runTest(ctx context.Context, logger *logrus.Entry, c client.Client, namespa
ObjectMeta: metav1.ObjectMeta{
Name: namespaceName,
},
Spec: extensionsv1alpha1.ClusterSpec{
Shoot: runtime.RawExtension{
Object: &gardencorev1beta1.Shoot{
TypeMeta: metav1.TypeMeta{
APIVersion: gardencorev1beta1.SchemeGroupVersion.String(),
Kind: "Shoot",
},
Spec: gardencorev1beta1.ShootSpec{
Kubernetes: gardencorev1beta1.Kubernetes{
Version: "1.15.5",
},
},
},
},
},
}
if err := c.Create(ctx, cluster); err != nil {
return err
Expand Down

0 comments on commit 2ad7820

Please sign in to comment.