Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

support http2 protocol (#1523) #2306

Merged
merged 5 commits into from
Nov 2, 2023
Merged

support http2 protocol (#1523) #2306

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f1e3179
support http2 protocol (#1523)
Oct 30, 2023
160eab9
simplify some logic and fix unit tests
Oct 31, 2023
02b82ac
always send at least 1 through and copy status condition over
Oct 31, 2023
3e0e94f
refactor check code in the api
Nov 2, 2023
7ca4aa3
change the created service name to reduce chance of collisions
Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions pkg/apis/internal.acorn.io/v1/appspec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ func (in Build) BaseBuild() Build {
type Protocol string

var (
ProtocolTCP = Protocol("tcp")
ProtocolUDP = Protocol("udp")
ProtocolHTTP = Protocol("http")
ProtocolTCP = Protocol("tcp")
ProtocolUDP = Protocol("udp")
ProtocolHTTP = Protocol("http")
ProtocolHTTP2 = Protocol("http2")
)

type PublishProtocol string
Expand All @@ -74,6 +75,7 @@ var (
PublishProtocolUDP = PublishProtocol("udp")
PublishProtocolHTTP = PublishProtocol("http")
PublishProtocolHTTPS = PublishProtocol("https")
PublishProtocolHTTP2 = PublishProtocol("http2")
)

type PortBindings []PortBinding
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/internal.acorn.io/v1/ports.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ func validProto(p string) (Protocol, bool) {
case ProtocolUDP:
fallthrough
case ProtocolHTTP:
fallthrough
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: all of these cases can be put on the same line:

switch ret {
    case ProtocolTCP, ProtocolUDP, ProtocolHTTP, ProtocolHTTP2, "":
        return ret, true
}

case ProtocolHTTP2:
return ret, true
case "":
return ret, true
Expand Down
6 changes: 3 additions & 3 deletions pkg/appdefinition/app.acorn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,15 @@

PortSingle: (int > 0 && int < 65536) || PortRegexp
Port: (int > 0 && int < 65536) || PortRegexp || PortSpec
PortRegexp: string =~ `^([a-z][-a-z0-9.]+:)?([0-9]+:)?([a-z][-a-z0-9]+:)?([0-9]+)(/(tcp|udp|http))?$`
PortRegexp: string =~ `^([a-z][-a-z0-9.]+:)?([0-9]+:)?([a-z][-a-z0-9]+:)?([0-9]+)(/(tcp|udp|http|http2))?$`

PortSpec: {
publish?: bool
dev?: bool
hostname?: string
port?: int
targetPort?: int
protocol?: enum("tcp", "udp", "http")
protocol?: enum("tcp", "udp", "http", "http2")
}

Metrics: {
Expand Down Expand Up @@ -385,7 +385,7 @@
hostname?: string
targetPort?: int
targetServiceName: DNSName
protocol?: enum("tcp", "udp", "http")
protocol?: enum("tcp", "udp", "http", "http2")
} || string || int

Router: {
Expand Down
57 changes: 42 additions & 15 deletions pkg/controller/service/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package service

import (
"fmt"

"github.com/acorn-io/baaah/pkg/router"
v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
"github.com/acorn-io/runtime/pkg/ports"
"github.com/acorn-io/runtime/pkg/publish"
"github.com/acorn-io/runtime/pkg/services"
)
Expand All @@ -13,25 +16,49 @@ func RenderServices(req router.Request, resp router.Response) error {
// reset, should get repopulated
svcInstance.Status.Endpoints = nil

objs, _, err := services.ToK8sService(req, svcInstance)
if err != nil {
return err
var svcList []*v1.ServiceInstance
http2Ports := ports.ByProtocol(svcInstance.Spec.Ports, true, v1.ProtocolHTTP2)
otherPorts := ports.ByProtocol(svcInstance.Spec.Ports, false, v1.ProtocolHTTP2)
if len(http2Ports) > 0 {
svcCopy := svcInstance.DeepCopy()
svcCopy.Spec.Ports = http2Ports
svcCopy.Name = fmt.Sprintf("%s-%s", svcInstance.Name, v1.ProtocolHTTP2)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am agreeing with @StrongMonkey here. Creating an additional ServiceInstance with a different name may cause issues with other parts of the system. For example, if something depends on the service, but the service has a different name. Or, the user may create a service that results in a ServiceInstance being created with the same name you are creating here.

Would it be possible to render the corev1.Service with the special annotations instead of creating an additional ServiceInstance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ServiceInstance is only created as a temporary copy for our purposes so that we can pass the same value into the function logic. We don't actually return the temporarily created ServiceInstance object, we just use it to create a new corev1.Service.

Copy link
Contributor

Choose a reason for hiding this comment

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

Spoke offline about a slightly different approach that would alleviate the concerns above.

if svcCopy.Spec.Annotations == nil {
svcCopy.Spec.Annotations = map[string]string{}
}
svcCopy.Spec.Annotations["traefik.ingress.kubernetes.io/service.serversscheme"] = "h2c"
svcList = append(svcList, svcCopy)
}
resp.Objects(objs...)
svcInstance.Status.HasService = len(objs) > 0

objs, err = publish.ServiceLoadBalancer(req, svcInstance)
if err != nil {
return err
if len(otherPorts) > 0 {
svcCopy := svcInstance.DeepCopy()
svcCopy.Spec.Ports = otherPorts
svcList = append(svcList, svcCopy)
}
resp.Objects(objs...)
svcInstance.Status.HasService = svcInstance.Status.HasService || len(objs) > 0

objs, err = publish.Ingress(req, svcInstance)
if err != nil {
return err
for _, svc := range svcList {
objs, _, err := services.ToK8sService(req, svc)
if err != nil {
return err
}
resp.Objects(objs...)
svcInstance.Status.HasService = svcInstance.Status.HasService || len(objs) > 0

objs, err = publish.ServiceLoadBalancer(req, svc)
if err != nil {
return err
}
resp.Objects(objs...)
svcInstance.Status.HasService = svcInstance.Status.HasService || len(objs) > 0

objs, err = publish.Ingress(req, svc)
if err != nil {
return err
}
resp.Objects(objs...)

// Copy all modifications made by the above publish.Ingress call
svcInstance.Status.Endpoints = append(svcInstance.Status.Endpoints, svc.Status.Endpoints...)
}
resp.Objects(objs...)

resp.Objects(svcInstance)
return nil
Expand Down
8 changes: 6 additions & 2 deletions pkg/ports/publish.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,18 @@ func IsLinked(app *v1.AppInstance, name string) bool {
return false
}

func ByProtocol(ports []v1.PortDef, protocols ...v1.Protocol) (result []v1.PortDef) {
func ByProtocol(ports []v1.PortDef, include bool, protocols ...v1.Protocol) (result []v1.PortDef) {
for _, port := range ports {
matched := false
for _, proto := range protocols {
if port.Complete().Protocol == proto {
result = append(result, port)
matched = true
break
}
}
if matched == include {
result = append(result, port)
}
}
return result
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/publish/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func Ingress(req router.Request, svc *v1.ServiceInstance) (result []kclient.Obje
return nil, err
}

bindings := ports.ApplyBindings(svc.Spec.PublishMode, svc.Spec.Publish, ports.ByProtocol(svc.Spec.Ports, v1.ProtocolHTTP))
bindings := ports.ApplyBindings(svc.Spec.PublishMode, svc.Spec.Publish, ports.ByProtocol(svc.Spec.Ports, true, v1.ProtocolHTTP, v1.ProtocolHTTP2))

if len(bindings) == 0 {
return nil, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/publish/servicelb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func ServiceLoadBalancer(req router.Request, svc *v1.ServiceInstance) (result []
}

bindings := ports.ApplyBindings(svc.Spec.PublishMode, svc.Spec.Publish,
ports.ByProtocol(svc.Spec.Ports, v1.ProtocolTCP, v1.ProtocolUDP))
ports.ByProtocol(svc.Spec.Ports, true, v1.ProtocolTCP, v1.ProtocolUDP))

if len(bindings) == 0 {
return nil, nil
Expand Down