Outdated documentation

[lolodi]

1. Azure Dev Spaces is currently deprecated, Bridge to Kubernetes should be used instead: https://docs.microsoft.com/en-us/visualstudio/containers/bridge-to-kubernetes?view=vs-2019

2. The deploy-all script currently fails with unknown flag --purge and --name. This is probably because it is expecting Helm 2 instead of the now recommended Helm3.

3. Why using a script to deploy all these helm charts when they can be grouped in an outer helm chart that can be deployed atomically. We shouldn't point users to run scripts to deploy a set of helm charts.

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: 861b238d-38e3-d054-9360-414da87f9b5c
• Version Independent ID: d2f38f79-a9d7-55dd-984f-2dab5f2219b8
• Content: Deploying eShopOnContainers to Azure
• Content Source: docs/architecture/cloud-native/deploy-eshoponcontainers-azure.md
• Product: dotnet-architecture
• Technology: cloud-native
• GitHub Login: @nishanil
• Microsoft Alias: nanil

[sughosneo]

Hi @lolodi, thank you for your feedback.

1. Yes, we are in process of evaluating Bridge to Kubernetes to incorporate that in the reference application eShopOnContainers. Once it's done we will update the app and the eBooks.
2. We have updated the reference application to use Helm 3 already. Here is the wiki page for more details.
3. The reason deploy-all script is used to deploy the reference application eShopOnContainers, is because to control different parameters (for e.g sslSupport, externalDns, aksName, aksRg etc..) along with deployment in different environments in both local and AKS with different available options.

[lolodi]

2. I realized I had checkout master instead of dev. If dev is the go-to branch, master should be removed to avoid confusion.
3. I understand this is the case, but wouldn't it be better to make the script modify a value.yaml file and let helm do an atomic deployment of the whole app? This way if something goes bad, rollback becomes easier and we would show the user the proper way of doing helm deployments.

--Extra point--
4. I noticed that all the services hardcode their ports in the code, this is an antipattern in a scenario in which the user wants to run the services both locally and remotely (e.g. with Bridge To Kubernetes). The port should be defined using the ASPNETCORE_URLS env variable. This way, if it is not set when running locally it defaults to 5000 (and avoids possible conflicts on port 80) and it can be set in the Dockerfile to run on port 80 when deployed.

[sughosneo]

Hi @lolodi, thank you for your feedback.

2. master branch is the previous LTS release branch where dev always contains the latest changes. Similarly, when we would update the eShopOnContainers app to .NET 5.0, all the changes will happen in the dev branch and the previous changes would be merged to master for future reference.
3. I see, we could explore that too. Would you like to submit a PR to eShopOnContainers repo ?
4. Each service-related configuration has been kept in the docker-compose file, which primary overrides the value in appsettings.json for each service. These configurations are also passed into the docker container as environment variables for each service to use in it's code. For e.g : if you notice the webmvc related configuration in the docker-compose.override.yml file, you would see required service-specific configuration are passed as environment variables which further overrides the value in appsettings.json setting and used in the code like class BasketService : IBasketService

We are also in process of exploring tye with eShopOnContainers, for more details you can refer issue #1387

[lolodi]

2. Given the effort to get rid of "master" branches, I believe this would be a good opportunity to rename master into "stable", "legacy" or "LTS". This way the semantic is clearer and we don't confuse users.

3. Not sure I have enough cycles to take care of this. I'll let you know if things changes, but it's something that should be done.

4. For this one my point was more about the fact that in code the default port is set to 80 e.g.: https://github.com/dotnet-architecture/eShopOnContainers/blob/9dec3b16e826d91e64f2c33a4c5ae043bf0e9a98/src/Services/Basket/Basket.API/Program.cs#L111
   in the appsettings.json there is no port info and therefore the execution falls back on port 80, value set in the code.
   I agree that, when the service is deployed containerized, the ports should be set in the Dockerfile, but we should have a port different from 80 when running them locally uncontainerized and we should not set that port in code, but using the proper ASPNET ways as described above.
   Having a non standard port (!=5000) defined in the code brakes the tooling that looks for the ASPNETCORE_URLS env variable to figure out what port the service is going to use when running locally.