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 new host to be used for container #1997

Merged
merged 32 commits into from
Aug 9, 2024
Merged

Add new host to be used for container #1997

merged 32 commits into from
Aug 9, 2024

Conversation

bording
Copy link
Member

@bording bording commented Aug 6, 2024

This PR makes the following changes:

  • A new container host, based on Kestrel, has been introduced
    • For now, the new host is used only for the container and the PlatformSample.
    • The new host reads in the environment variables directly and uses the values to serve a dynamically constructed app.contants.js result, removing the need to write those out to disk before serving the website.
    • The new host also includes a reverse proxy so that all requests can go directly to the host's web application. API requests to ServiceControl and Monitoring APIs are forwarded by the revery proxy.
  • A new MONITORING_URL setting has been added to the container. It accepts a single URI instead of an array of URIs. If this new setting and the old MONITORING_URLS are both set, the new setting takes precedence.
  • The container now runs processes internally as non-root, so the default port has been changed to 9090.
  • Some miscellaneous cleanup and improvements to other projects in the repo have also been included.

@bording bording merged commit 99b048f into master Aug 9, 2024
4 checks passed
@bording bording deleted the container-host branch August 9, 2024 17:05
@DavidBoike DavidBoike added this to the 1.41.0 milestone Aug 9, 2024
@PhilBastian PhilBastian added the Type: Improvement Type: Improvement label Aug 12, 2024
@ivanl-out
Copy link

ivanl-out commented Aug 12, 2024

Thanks @bording.

Just note that the change from port 90 to 9090 was a breaking change for us since our ingress in Azure Container Apps previously exposed port 90. We also had a startup probe hitting port 90. It might be good to include this in the release notes for others (I may have missed this).

Edit: I see the new port was noted in Container-README.md, so please disregard my comment, I just didn't read it since I was upgrading from a previous version.

@bording
Copy link
Member Author

bording commented Aug 12, 2024

Thanks @bording.

Just note that the change from port 90 to 9090 was a breaking change for us since our ingress in Azure Container Apps previously exposed port 90. We also had a startup probe hitting port 90. It might be good to include this in the release notes for others (I may have missed this).

Edit: I see the new port was noted in Container-README.md, so please disregard my comment, I just didn't read it since I was upgrading from a previous version.

We have added a note to the release notes and the announcement regarding the change.

However, the only thing that really changed was the port inside the container. Externally, you can continue to map that to whatever port you desire.

For example using docker:

docker run -p 90:9090 particular/servicepulse:latest

Would let you keep using 90 as the public-facing port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement Type: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants