diff --git a/go.mod b/go.mod index af03fc1bde..587c509421 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index fd736a9270..dade221b3e 100644 --- a/go.sum +++ b/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= diff --git a/pkg/controller/ec2/internetgateway/controller.go b/pkg/controller/ec2/internetgateway/controller.go index 034e24f539..9ad1c12709 100644 --- a/pkg/controller/ec2/internetgateway/controller.go +++ b/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) { diff --git a/pkg/controller/ec2/internetgateway/controller_test.go b/pkg/controller/ec2/internetgateway/controller_test.go index ddceb1a5de..3e11da3b25 100644 --- a/pkg/controller/ec2/internetgateway/controller_test.go +++ b/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": { diff --git a/pkg/controller/ec2/routetable/controller.go b/pkg/controller/ec2/routetable/controller.go index 68f0d11230..79b6c4b1fd 100644 --- a/pkg/controller/ec2/routetable/controller.go +++ b/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 diff --git a/pkg/controller/ec2/routetable/controller_test.go b/pkg/controller/ec2/routetable/controller_test.go index 8c9eb8ec12..84e429eabf 100644 --- a/pkg/controller/ec2/routetable/controller_test.go +++ b/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 != "" {