DO NOT MERGE: Give some EC2 networking APIs a creation grace time

Per crossplane-contrib#802 there seems to be some
lag between when some EC2 networking resources (RouteTables, InternetGateways)
are created and when they actually show up in queries. This commit leverages
crossplane/crossplane-runtime#280 to allow for this.

Signed-off-by: Nic Cope <negz@rk0n.org>

go.mod

@@ -2,6 +2,8 @@ module github.com/crossplane/provider-aws
 
 go 1.16
 
+replace github.com/crossplane/crossplane-runtime => github.com/negz/crossplane-runtime v0.0.0-20210816051713-7a3539db4ad5
+
 require (
 	github.com/aws/aws-sdk-go v1.37.4
 	github.com/aws/aws-sdk-go-v2 v0.23.0

go.sum

@@ -106,8 +106,6 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsr
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/crossplane/crossplane-runtime v0.14.1-0.20210722005935-0b469fcc77cd h1:2ZdR/HyjXFIo6KxmM08jBLeiJs7GRdGmb6qPKQANGvI=
-github.com/crossplane/crossplane-runtime v0.14.1-0.20210722005935-0b469fcc77cd/go.mod h1:0sB8XOV2zy1GdZvSMY0/5QzKQJUiNSek08wbAYHJbws=
 github.com/crossplane/crossplane-tools v0.0.0-20210320162312-1baca298c527 h1:9M6hMLKqjxtL9d9nwfcaAt59Ey0CPfSXQ3iIdYRUNaE=
 github.com/crossplane/crossplane-tools v0.0.0-20210320162312-1baca298c527/go.mod h1:C735A9X0x0lR8iGVOOxb49Mt70Ua4EM2b7PGaRPBLd4=
 github.com/dave/jennifer v1.3.0 h1:p3tl41zjjCZTNBytMwrUuiAnherNUZktlhPTKoF/sEk=
@@ -411,6 +409,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
+github.com/negz/crossplane-runtime v0.0.0-20210816051713-7a3539db4ad5 h1:AV8PD8PX+oawzdLmJLlE/6lDEe8RztTEzPgKZK6s6/c=
+github.com/negz/crossplane-runtime v0.0.0-20210816051713-7a3539db4ad5/go.mod h1:XvktCTRFTkdP2jR2PecrvhsxzSO8XT3jHxTOk/k+NL8=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=

pkg/controller/ec2/internetgateway/controller.go

@@ -55,6 +55,14 @@ const (
 	errCreateTags          = "failed to create tags for the InternetGateway resource"
 )
 
+// TODO(negz): Tune the below grace period - it was chosen fairly arbitrarily.
+// In my testing 3 minutes appears to be sufficient.
+
+// We wait up to 3 minutes for DescribeInternetGateways to start reporting that
+// a newly created InternetGateway exists, to work around what appears to be a
+// somewhat rare eventual consistency in the AWS API.
+const creationGracePeriod = 3 * time.Minute
+
 // SetupInternetGateway adds a controller that reconciles InternetGateways.
 func SetupInternetGateway(mgr ctrl.Manager, l logging.Logger, rl workqueue.RateLimiter, poll time.Duration) error {
 	name := managed.ControllerName(v1beta1.InternetGatewayGroupKind)
@@ -105,16 +113,21 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E
 	}
 
 	if meta.GetExternalName(cr) == "" {
-		return managed.ExternalObservation{
-			ResourceExists: false,
-		}, nil
+		return managed.ExternalObservation{ResourceExists: false}, nil
 	}
 
-	response, err := e.client.DescribeInternetGatewaysRequest(&awsec2.DescribeInternetGatewaysInput{
-		InternetGatewayIds: []string{meta.GetExternalName(cr)},
-	}).Send(ctx)
+	i := &awsec2.DescribeInternetGatewaysInput{InternetGatewayIds: []string{meta.GetExternalName(cr)}}
+	response, err := e.client.DescribeInternetGatewaysRequest(i).Send(ctx)
+	if ec2.IsInternetGatewayNotFoundErr(err) {
+		if t := meta.GetExternalCreateTime(cr); t != nil && time.Since(t.Time) < creationGracePeriod {
+			return managed.ExternalObservation{ResourcePending: true}, nil
+		}
+		// If our grace period is up we assume the managed resource does
+		// not exist.
+		return managed.ExternalObservation{ResourceExists: false}, nil
+	}
 	if err != nil {
-		return managed.ExternalObservation{}, awsclient.Wrap(resource.Ignore(ec2.IsInternetGatewayNotFoundErr, err), errDescribe)
+		return managed.ExternalObservation{}, awsclient.Wrap(err, errDescribe)
 	}
 
 	// in a successful response, there should be one and only one object
@@ -155,8 +168,9 @@ func (e *external) Create(ctx context.Context, mgd resource.Managed) (managed.Ex
 	}
 
 	meta.SetExternalName(cr, aws.StringValue(ig.InternetGateway.InternetGatewayId))
+	meta.SetExternalCreateTime(cr)
 
-	return managed.ExternalCreation{ExternalNameAssigned: true}, nil
+	return managed.ExternalCreation{ExternalNameAssigned: true, ExternalCreateTimeSet: true}, nil
 }
 
 func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.ExternalUpdate, error) {

pkg/controller/ec2/internetgateway/controller_test.go

@@ -58,6 +58,10 @@ func withExternalName(name string) igModifier {
 	return func(r *v1beta1.InternetGateway) { meta.SetExternalName(r, name) }
 }
 
+func withExternameCreateTime() igModifier {
+	return func(r *v1beta1.InternetGateway) { meta.SetExternalCreateTime(r) }
+}
+
 func withConditions(c ...xpv1.Condition) igModifier {
 	return func(r *v1beta1.InternetGateway) { r.Status.ConditionedStatus.Conditions = c }
 }
@@ -257,12 +261,16 @@ func TestCreate(t *testing.T) {
 				})),
 			},
 			want: want{
-				cr: ig(withSpec(v1beta1.InternetGatewayParameters{
-					VPCID: aws.String(vpcID),
-				}),
+				cr: ig(
+					withSpec(v1beta1.InternetGatewayParameters{VPCID: aws.String(vpcID)}),
 					withExternalName(igID),
-					withConditions(xpv1.Creating())),
-				result: managed.ExternalCreation{ExternalNameAssigned: true},
+					withExternameCreateTime(),
+					withConditions(xpv1.Creating()),
+				),
+				result: managed.ExternalCreation{
+					ExternalNameAssigned:  true,
+					ExternalCreateTimeSet: true,
+				},
 			},
 		},
 		"FailedRequest": {

pkg/controller/ec2/routetable/controller.go

@@ -59,6 +59,15 @@ const (
 	errDeleteTags         = "failed to delete tags for the RouteTable resource"
 )
 
+// TODO(negz): Tune the below grace period - it was chosen fairly arbitrarily.
+// In my testing 3 minutes appears to be sufficient. The only other value I
+// tested was 8 seconds, which was not long enough.
+
+// We wait up to 3 minutes for DescribeRouteTables to start reporting that a
+// newly created RouteTable exists, to work around what appears to be a somewhat
+// rare eventual consistency in the AWS API.
+const creationGracePeriod = 3 * time.Minute
+
 // SetupRouteTable adds a controller that reconciles RouteTables.
 func SetupRouteTable(mgr ctrl.Manager, l logging.Logger, rl workqueue.RateLimiter, poll time.Duration) error {
 	name := managed.ControllerName(v1beta1.RouteTableGroupKind)
@@ -107,21 +116,32 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E
 		return managed.ExternalObservation{}, errors.New(errUnexpectedObject)
 	}
 
-	// To find out whether a RouteTable exist:
-	// - the object's ExternalName should have routeTableId populated
-	// - a RouteTable with the given routeTableId should exist
+	// The AWS API returns an unpredictable ID for each RouteTable at create
+	// time. We rely on this ID having been recorded at create time in order
+	// to be able to determine whether the table exists.
 	if meta.GetExternalName(cr) == "" {
-		return managed.ExternalObservation{
-			ResourceExists: false,
-		}, nil
+		return managed.ExternalObservation{ResourceExists: false}, nil
 	}
 
-	response, err := e.client.DescribeRouteTablesRequest(&awsec2.DescribeRouteTablesInput{
-		RouteTableIds: []string{meta.GetExternalName(cr)},
-	}).Send(ctx)
-
+	// If we've recorded an ExternalName we must have created the table at
+	// some point. Unfortunately there is sometimes a slight delay between
+	// the creation of a RouteTable and when the AWS API reports that it
+	// exists. If the table doesn't appear to exist at this point it most
+	// likely has either been deleted, or was only recently created. We
+	// handle the latter case by waiting a few minutes after the most recent
+	// creation before we decide whether or not it exists.
+	i := &awsec2.DescribeRouteTablesInput{RouteTableIds: []string{meta.GetExternalName(cr)}}
+	response, err := e.client.DescribeRouteTablesRequest(i).Send(ctx)
+	if ec2.IsRouteTableNotFoundErr(err) {
+		if t := meta.GetExternalCreateTime(cr); t != nil && time.Since(t.Time) < creationGracePeriod {
+			return managed.ExternalObservation{ResourcePending: true}, nil
+		}
+		// If our grace period is up we assume the managed resource does
+		// not exist.
+		return managed.ExternalObservation{ResourceExists: false}, nil
+	}
 	if err != nil {
-		return managed.ExternalObservation{}, awsclient.Wrap(resource.Ignore(ec2.IsRouteTableNotFoundErr, err), errDescribe)
+		return managed.ExternalObservation{}, awsclient.Wrap(err, errDescribe)
 	}
 
 	// in a successful response, there should be one and only one object
@@ -163,14 +183,13 @@ func (e *external) Create(ctx context.Context, mgd resource.Managed) (managed.Ex
 	if !ok {
 		return managed.ExternalCreation{}, errors.New(errUnexpectedObject)
 	}
-	result, err := e.client.CreateRouteTableRequest(&awsec2.CreateRouteTableInput{
-		VpcId: cr.Spec.ForProvider.VPCID,
-	}).Send(ctx)
+	result, err := e.client.CreateRouteTableRequest(&awsec2.CreateRouteTableInput{VpcId: cr.Spec.ForProvider.VPCID}).Send(ctx)
 	if err != nil {
 		return managed.ExternalCreation{}, awsclient.Wrap(err, errCreate)
 	}
 	meta.SetExternalName(cr, aws.StringValue(result.RouteTable.RouteTableId))
-	return managed.ExternalCreation{ExternalNameAssigned: true}, nil
+	meta.SetExternalCreateTime(cr)
+	return managed.ExternalCreation{ExternalNameAssigned: true, ExternalCreateTimeSet: true}, nil
 }
 
 func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.ExternalUpdate, error) { // nolint:gocyclo

pkg/controller/ec2/routetable/controller_test.go

@@ -20,10 +20,12 @@ import (
 	"context"
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/pkg/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -65,6 +67,10 @@ func withExternalName(name string) rtModifier {
 	return func(r *v1beta1.RouteTable) { meta.SetExternalName(r, name) }
 }
 
+func withExternalCreateTime() rtModifier {
+	return func(r *v1beta1.RouteTable) { meta.SetExternalCreateTime(r) }
+}
+
 func withSpec(p v1beta1.RouteTableParameters) rtModifier {
 	return func(r *v1beta1.RouteTable) { r.Spec.ForProvider = p }
 }
@@ -211,10 +217,15 @@ func TestCreate(t *testing.T) {
 				})),
 			},
 			want: want{
-				cr: rt(withSpec(v1beta1.RouteTableParameters{
-					VPCID: aws.String(vpcID),
-				}), withExternalName(rtID)),
-				result: managed.ExternalCreation{ExternalNameAssigned: true},
+				cr: rt(
+					withExternalName(rtID),
+					withExternalCreateTime(),
+					withSpec(v1beta1.RouteTableParameters{VPCID: aws.String(vpcID)}),
+				),
+				result: managed.ExternalCreation{
+					ExternalNameAssigned:  true,
+					ExternalCreateTimeSet: true,
+				},
 			},
 		},
 		"CreateFailed": {
@@ -251,7 +262,7 @@ func TestCreate(t *testing.T) {
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
 				t.Errorf("r: -want, +got:\n%s", diff)
 			}
-			if diff := cmp.Diff(tc.want.cr, tc.args.cr, test.EquateConditions()); diff != "" {
+			if diff := cmp.Diff(tc.want.cr, tc.args.cr, test.EquateConditions(), cmpopts.EquateApproxTime(1*time.Minute)); diff != "" {
 				t.Errorf("r: -want, +got:\n%s", diff)
 			}
 			if diff := cmp.Diff(tc.want.result, o); diff != "" {