Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an ASP.NET app that runs in AWS ECS Fargate #532

Merged
merged 3 commits into from
Jan 30, 2020

Conversation

joeduffy
Copy link
Member

This adds an example of a basic ASP.NET app that runs in AWS ECS
Fargate. This includes the infrastructure required to build and publish
that application as a Docker container image to a private ECR repo,
spin up a new ECS Fargate cluster, run a load balancer listening for
external Internet traffic on port 80, and run services across all
subnets in the default VPC for the target deployment region.

This adds an example of a basic ASP.NET app that runs in AWS ECS
Fargate. This includes the infrastructure required to build and publish
that application as a Docker container image to a private ECR repo,
spin up a new ECS Fargate cluster, run a load balancer listening for
external Internet traffic on port 80, and run services across all
subnets in the default VPC for the target deployment region.
Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Several nits on formatting.

using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;

namespace dotnet_core_tutorial
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename the namespace to something .NET-y

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, all of this is whatever dotnet new spit out, fwiw.

"Microsoft": "Information"
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but I would omit this file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Was autogenerated by dotnet new, wasn't sure what it is 😄

@@ -0,0 +1,2 @@
/bin/
/obj/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe merge .gitignores in the root folder of the example?

// Create a SecurityGroup that permits HTTP ingress and unrestricted egress.
var webSg = new Ec2.SecurityGroup("web-sg", new Ec2.SecurityGroupArgs {
VpcId = vpc.Id,
Egress = new[] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new[] is not required here and down below

{
static Task<int> Main()
{
return Deployment.RunAsync(async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's common to put { on a new line, also in our .NET code

Copy link
Member Author

@joeduffy joeduffy Jan 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, will fix those that are obvious.

Not quite sure what the style is meant to be in all places, e.g.

    LoadBalancers = {
        x
    },

or

    LoadBalancers =
    {
        x
    },

Do we have plans to add style/linter checking?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my, the code is now twice as long! 🤦‍♂ But I shall resist the temptation to comment on style guidelines ... (oh wait, too late)

ImageName = appRepo.RepositoryUrl,
Registry = new Docker.ImageRegistry {
Server = appRepo.RepositoryUrl,
Username = appRepoCreds.Apply(creds => creds[0]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to do appRepoCreds.Get(0)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm: error CS1061: 'Output<string[]>' does not contain a definition for 'Get' and no accessible extension method 'Get' accepting a first argument of type 'Output<string[]>' could be found (are you missing a using directive or an assembly reference?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NetworkMode = "awsvpc",
RequiresCompatibilities = new[] { "FARGATE" },
ExecutionRoleArn = taskExecRole.Arn,
ContainerDefinitions = image.ImageName.Apply(imgName => @"[{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imgName -> imageName in .NET

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


// Export the resulting web address.
return new Dictionary<string, object?>{
{ "url", webLb.DnsName.Apply(url => $"http://{url}") },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try changing to Output.Format

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* [Install Pulumi](https://www.pulumi.com/docs/get-started/install/)
* [Install .NET Core 3](https://dotnet.microsoft.com/download)
* [Connect Pulumi to AWS](https://www.pulumi.com/docs/intro/cloud-providers/aws/setup/) (if your AWS CLI is
configured, this will just work)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker is another pre

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed.

2 changes. 9 unchanged
```

Notice that `pulumi up` redeploys just the parts of the application/infrastructure that you've edited.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does the re-publishing of the docker image work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does! I filed an issue for the lack of diffs (pulumi/pulumi-docker#131) which led me astray. The root problem was that I was using the ECR registry URL and not the built image's full name, which is just the ECR registry plus the hash. The hash is required to ensure the ECS service gets restarted upon a new image being pushed.

@joeduffy joeduffy merged commit e38cc4b into master Jan 30, 2020
@pulumi-bot pulumi-bot deleted the joeduffy/aws_cs_fargate branch January 30, 2020 02:26
dixler pushed a commit that referenced this pull request Jan 21, 2022
* Add an ASP.NET app that runs in AWS ECS Fargate

This adds an example of a basic ASP.NET app that runs in AWS ECS
Fargate. This includes the infrastructure required to build and publish
that application as a Docker container image to a private ECR repo,
spin up a new ECS Fargate cluster, run a load balancer listening for
external Internet traffic on port 80, and run services across all
subnets in the default VPC for the target deployment region.

* Incorporate code review feedback

* More brace movement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants