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

feat: add schema ContainerPort and Route, to expose Service Container #16

Closed
wants to merge 3 commits into from

Conversation

healthjyk
Copy link
Contributor

@healthjyk healthjyk commented Aug 17, 2023

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add Schema ContainerPort and Route which belong to network, so that the Service Container can be accessed within and outside the cluster.

Special notes for your reviewer:

ContainerPort and Route aims to the TYPICAL scenario of exposing service within and outside cluster. Not all the Kubernetes Service types and CSP loadBalancers are supported.
The the most significant feature of ContainerPort and Route is concise, which provides out of box experience(OOBE).
Task a great reference of acorn Network.

@healthjyk healthjyk changed the title feat: add schema ContainerPort and Route, to expose the Service Conta… feat: add schema ContainerPort and Route, to expose Service Container Aug 17, 2023
----------
port: int, default is 80, required.
The available port on the container.
accessMode: "Internal" | "Exposed", default is "Internal", required.
Copy link
Member

Choose a reason for hiding this comment

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

Generally, we consider Internal / Internet and exposed / unexposed as two different expressions of network type. Combining Internal and Exposed together sounds a little awkward. Any suggestions? @ffforest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Internal" and "Exposed" are take a reference of Acorn, ref: acorn ports

Copy link
Contributor

@ffforest ffforest Aug 18, 2023

Choose a reason for hiding this comment

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

I do agree they should be two sets of configurations. However, combining them into one is still reasonable in the sense that it describes the range to expose the container/port: Pod(Internal), Cluster(Exposed), Intranet, Internet.
Also I don't think this should be a container-port level configuration. That's too much details for the user to consider compared to "whether my application needs to be exposed to other applications".
There are corner cases like when a container has multiple ports, and some need to be exposed while some don't. But we should draw the line somewhere instead of trying to cater to every single edge case.

We have a few options here (there are probably problems with implementation details in each one):
1.We keep this as a container-port-level configuration and let user decide. My suggestion is to change this to a bool named exposed if we consider Internet/Intranet a separate question.
2.We consider this a container-level configuration. This introduces some constraints but it's easier to configure and understand.
3.We consider this an app configuration.
4.We can decide for the user (and eliminate this field) that for each container port defined, we will always expose it via a Kubernetes Service.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer to combine network/container_port and service/container_port together to reduce the user's cognitive load, as demonstrated by Acorn.

pathType?: "Exact" | "Prefix"

# The backend container accessPort.
containerAccessPort: int
Copy link
Member

Choose a reason for hiding this comment

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

Can we simplify this field as accessPort like the naming in the ContainerPort? Are there any other kinds of access ports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my first design. If use accessPort, is there any confusing question? Does Route only serve container?

Copy link
Member

Choose a reason for hiding this comment

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

According to my understanding, both route and container_port are designed for service or service containers. There is no other kind of accessPort used without a container.

models/schema/v1/workload/service.k Show resolved Hide resolved
@healthjyk healthjyk closed this Aug 28, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2023
@SparkYuan SparkYuan deleted the network branch September 8, 2023 07:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants