From 7a0e1068df73b31562e8acaaf0b6b1536c8c8258 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 6 Oct 2019 01:43:06 -0700 Subject: [PATCH] Revert "pkg/destroy: data/aws: delete undiscoverable AWS objects" This reverts commit 6ae7598063863c2f318b137e1b2354aef2514fc1, #1268. That was a workaround to recover from openshift-dev clusters where an in-house pruner is removing instances but not their associated instance profiles. Folks using the installer's destroy code won't need it, and while the risk of accidental name collision is low, I don't think it's worth taking that risk. With this commit, folks using external reapers are responsible for ensuring that they reap instance profiles when they reap instances, and we get deletion logic that is easier to explain to folks mixing multiple clusters in the same account. --- pkg/destroy/aws/aws.go | 76 ++++++++++-------------------------------- 1 file changed, 18 insertions(+), 58 deletions(-) diff --git a/pkg/destroy/aws/aws.go b/pkg/destroy/aws/aws.go index acaa663ed35..78c14fd81be 100644 --- a/pkg/destroy/aws/aws.go +++ b/pkg/destroy/aws/aws.go @@ -55,10 +55,9 @@ type ClusterUninstaller struct { // } // // will match resources with (a:b and c:d) or d:e. - Filters []Filter // filter(s) we will be searching for - Logger logrus.FieldLogger - Region string - ClusterID string + Filters []Filter // filter(s) we will be searching for + Logger logrus.FieldLogger + Region string // Session is the AWS session to be used for deletion. If nil, a // new session will be created based on the usual credential @@ -79,11 +78,10 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers. } return &ClusterUninstaller{ - Filters: filters, - Region: metadata.ClusterPlatformMetadata.AWS.Region, - Logger: logger, - ClusterID: metadata.InfraID, - Session: session, + Filters: filters, + Region: metadata.ClusterPlatformMetadata.AWS.Region, + Logger: logger, + Session: session, }, nil } @@ -148,7 +146,7 @@ func (o *ClusterUninstaller) Run() error { return err } - err = wait.PollImmediateInfinite( + return wait.PollImmediateInfinite( time.Second*10, func() (done bool, err error) { var loopError error @@ -254,16 +252,6 @@ func (o *ClusterUninstaller) Run() error { return len(tagClients) == 0 && loopError == nil, nil }, ) - if err != nil { - return err - } - - o.Logger.Debug("search for untaggable resources") - if err := o.deleteUntaggedResources(awsSession); err != nil { - o.Logger.Debug(err) - return err - } - return nil } func splitSlash(name string, input string) (base string, suffix string, err error) { @@ -755,27 +743,6 @@ func terminateEC2InstancesByTags(ec2Client *ec2.EC2, iamClient *iam.IAM, filters return terminated, err } -// This is a bit of hack. Some objects, like Instance Profiles, can not be tagged in AWS. -// We "normally" find those objects by their relation to other objects. We have found, -// however, that people regularly delete all of their instances and roles outside of -// openshift-install destroy cluster. This means that we are unable to find the Instance -// Profiles. -// -// This code is a place to find specific objects like this which might be dangling. -func (o *ClusterUninstaller) deleteUntaggedResources(awsSession *session.Session) error { - iamClient := iam.New(awsSession) - masterProfile := fmt.Sprintf("%s-master-profile", o.ClusterID) - if err := deleteIAMInstanceProfileByName(iamClient, &masterProfile, o.Logger); err != nil { - return err - } - workerProfile := fmt.Sprintf("%s-worker-profile", o.ClusterID) - if err := deleteIAMInstanceProfileByName(iamClient, &workerProfile, o.Logger); err != nil { - return err - } - - return nil -} - func deleteEC2InternetGateway(client *ec2.EC2, id string, logger logrus.FieldLogger) error { response, err := client.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ InternetGatewayIds: []*string{aws.String(id)}, @@ -1464,25 +1431,12 @@ func deleteIAM(session *session.Session, arn arn.ARN, logger logrus.FieldLogger) } } -func deleteIAMInstanceProfileByName(client *iam.IAM, name *string, logger logrus.FieldLogger) error { - _, err := client.DeleteInstanceProfile(&iam.DeleteInstanceProfileInput{ - InstanceProfileName: name, - }) - if err != nil { - if err.(awserr.Error).Code() == iam.ErrCodeNoSuchEntityException { - return nil - } - return err - } - logger.WithField("InstanceProfileName", *name).Info("Deleted") - return err -} - func deleteIAMInstanceProfile(client *iam.IAM, profileARN arn.ARN, logger logrus.FieldLogger) error { resourceType, name, err := splitSlash("resource", profileARN.Resource) if err != nil { return err } + logger = logger.WithField("name", name) if resourceType != "instance-profile" { return errors.Errorf("%s ARN passed to deleteIAMInstanceProfile: %s", resourceType, profileARN.String()) @@ -1507,14 +1461,20 @@ func deleteIAMInstanceProfile(client *iam.IAM, profileARN arn.ARN, logger logrus if err != nil { return errors.Wrapf(err, "dissociating %s", *role.RoleName) } - logger.WithField("name", name).WithField("role", *role.RoleName).Info("Disassociated") + logger.WithField("role", *role.RoleName).Info("Disassociated") } logger = logger.WithField("arn", profileARN.String()) - if err := deleteIAMInstanceProfileByName(client, profile.InstanceProfileName, logger); err != nil { + _, err = client.DeleteInstanceProfile(&iam.DeleteInstanceProfileInput{ + InstanceProfileName: profile.InstanceProfileName, + }) + if err != nil { + if err.(awserr.Error).Code() == iam.ErrCodeNoSuchEntityException { + return nil + } return err } - + logger.Info("Deleted") return nil }