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

allow exposing gateway on nodeport service - status updates #1392

Merged
merged 4 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
12 changes: 8 additions & 4 deletions api/config/v1alpha1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,21 @@ type KubernetesContainerSpec struct {

// ServiceType string describes ingress methods for a service
// +enum
// +kubebuilder:validation:Enum=LoadBalancer;ClusterIP
// +kubebuilder:validation:Enum=ClusterIP;LoadBalancer;NodePort
type ServiceType string

const (
// ServiceTypeClusterIP means a service will only be accessible inside the
// cluster, via the cluster IP.
ServiceTypeClusterIP ServiceType = "ClusterIP"

// ServiceTypeLoadBalancer means a service will be exposed via an
// external load balancer (if the cloud provider supports it).
ServiceTypeLoadBalancer ServiceType = "LoadBalancer"

// ServiceTypeClusterIP means a service will only be accessible inside the
// cluster, via the cluster IP.
ServiceTypeClusterIP ServiceType = "ClusterIP"
// ServiceTypeNodePort means a service will be exposed on each Kubernetes Node
// at a static Port, common across all Nodes.
ServiceTypeNodePort ServiceType = "NodePort"
)

// KubernetesServiceSpec defines the desired state of the Kubernetes service resource.
Expand Down
4 changes: 3 additions & 1 deletion api/config/v1alpha1/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ func validateServiceType(spec *EnvoyProxySpec) []error {
var errs []error
if spec.Provider.Kubernetes != nil && spec.Provider.Kubernetes.EnvoyService != nil {
if serviceType := spec.Provider.Kubernetes.EnvoyService.Type; serviceType != nil {
if *serviceType != ServiceTypeLoadBalancer && *serviceType != ServiceTypeClusterIP {
if *serviceType != ServiceTypeLoadBalancer &&
Copy link
Member

Choose a reason for hiding this comment

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

can you simply this lines with a function in a followup

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

*serviceType != ServiceTypeClusterIP &&
*serviceType != ServiceTypeNodePort {
errs = append(errs, fmt.Errorf("unsupported envoy service type %v", serviceType))
}
}
Expand Down
2 changes: 1 addition & 1 deletion api/config/v1alpha1/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestValidateEnvoyProxy(t *testing.T) {
},
},
},
expected: false,
expected: true,
},
{
name: "valid envoy service type 'LoadBalancer'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,8 +716,9 @@ spec:
only be accessible inside the cluster, via the cluster
IP.
enum:
- LoadBalancer
- ClusterIP
- LoadBalancer
- NodePort
type: string
type: object
type: object
Expand Down
1 change: 1 addition & 0 deletions charts/gateway-helm/templates/generated/rbac/roles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ rules:
- apiGroups:
- ""
resources:
- nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

geo-check is failing because this also need to be part of rbac.go

- namespaces
- secrets
- services
Expand Down
15 changes: 14 additions & 1 deletion internal/provider/kubernetes/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type gatewayAPIReconciler struct {
log logr.Logger
statusUpdater status.Updater
classController gwapiv1b1.GatewayController
store *kubernetesProviderStore
namespace string

resources *message.ProviderResources
Expand Down Expand Up @@ -88,6 +89,7 @@ func newGatewayAPIController(mgr manager.Manager, cfg *config.Server, su status.
statusUpdater: su,
resources: resources,
extGVKs: extGVKs,
store: newProviderStore(),
}

c, err := controller.New("gatewayapi", mgr, controller.Options{Reconciler: r})
Expand Down Expand Up @@ -327,7 +329,7 @@ func (r *gatewayAPIReconciler) statusUpdateForGateway(ctx context.Context, gtw *
// update accepted condition
status.UpdateGatewayStatusAcceptedCondition(gtw, true)
// update address field and programmed condition
status.UpdateGatewayStatusProgrammedCondition(gtw, svc, deploy)
status.UpdateGatewayStatusProgrammedCondition(gtw, svc, deploy, r.store.listNodeAddresses()...)

key := utils.NamespacedName(gtw)

Expand Down Expand Up @@ -1144,6 +1146,17 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M
return err
}

// Watch Node CRUDs to update Gateway Address exposed by Service of type NodePort.
// Node creation/deletion and ExternalIP updates would require update in the Gateway
// resource address.
if err := c.Watch(
source.Kind(mgr.GetCache(), &corev1.Node{}),
&handler.EnqueueRequestForObject{},
predicate.NewPredicateFuncs(r.handleNode),
); err != nil {
return err
}

// Watch Secret CRUDs and process affected Gateways.
if err := c.Watch(
source.Kind(mgr.GetCache(), &corev1.Secret{}),
Expand Down
22 changes: 22 additions & 0 deletions internal/provider/kubernetes/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,25 @@ func (r gatewayAPIReconciler) findOwningGateway(ctx context.Context, labels map[

return gtw
}

func (r *gatewayAPIReconciler) handleNode(obj client.Object) bool {
ctx := context.Background()
node, ok := obj.(*corev1.Node)
if !ok {
r.log.Info("unexpected object type, bypassing reconciliation", "object", obj)
return false
}

key := types.NamespacedName{Name: node.Name}
if err := r.client.Get(ctx, key, node); err != nil {
if kerrors.IsNotFound(err) {
r.store.removeNode(node)
return true
}
r.log.Error(err, "unable to find node", "name", node.Name)
return false
}

r.store.addNode(node)
return true
}
65 changes: 65 additions & 0 deletions internal/provider/kubernetes/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright Envoy Gateway Authors
// SPDX-License-Identifier: Apache-2.0
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

package kubernetes

import (
corev1 "k8s.io/api/core/v1"
)

type nodeDetails struct {
name string
address string
}

// kubernetesProviderStore holds cached information for the kubernetes provider.
type kubernetesProviderStore struct {
// nodes holds information required for updating Gateway status with the Node
// addresses, in case the Gateway is exposed on every Node of the cluster, using
// Service of type NodePort.
nodes map[string]nodeDetails
}

func newProviderStore() *kubernetesProviderStore {
return &kubernetesProviderStore{
nodes: make(map[string]nodeDetails),
}
}

func (p *kubernetesProviderStore) addNode(n *corev1.Node) {
details := nodeDetails{name: n.Name}

var internalIP, externalIP string
for _, addr := range n.Status.Addresses {
if addr.Type == corev1.NodeExternalIP {
externalIP = addr.Address
}
if addr.Type == corev1.NodeInternalIP {
internalIP = addr.Address
}
}

// In certain scenarios (like in local KinD clusters), the Node
// externalIP is not provided, in that case we default back
// to the internalIP of the Node.
if externalIP != "" {
details.address = externalIP
return
}
details.address = internalIP
p.nodes[n.Name] = details
}

func (p *kubernetesProviderStore) removeNode(n *corev1.Node) {
delete(p.nodes, n.Name)
}

func (p *kubernetesProviderStore) listNodeAddresses() []string {
addrs := []string{}
for _, n := range p.nodes {
addrs = append(addrs, n.address)
}
return addrs
}
6 changes: 5 additions & 1 deletion internal/status/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func UpdateGatewayStatusAcceptedCondition(gw *gwapiv1b1.Gateway, accepted bool)
// UpdateGatewayStatusProgrammedCondition updates the status addresses for the provided gateway
// based on the status IP/Hostname of svc and updates the Programmed condition based on the
// service and deployment state.
func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1b1.Gateway, svc *corev1.Service, deployment *appsv1.Deployment) {
func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1b1.Gateway, svc *corev1.Service, deployment *appsv1.Deployment, nodeAddresses ...string) {
var addresses, hostnames []string
// Update the status addresses field.
if svc != nil {
Expand Down Expand Up @@ -50,6 +50,10 @@ func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1b1.Gateway, svc *corev1.S
}
}

if svc.Spec.Type == corev1.ServiceTypeNodePort {
addresses = nodeAddresses
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also append the NodePort to this address, and expose it in the status ?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like we cannot expose the port in a clean way, because there will be a unique node port per listener port

}

addresses = append(addresses, svc.Spec.ExternalIPs...)

var gwAddresses []gwapiv1b1.GatewayAddress
Expand Down