Run Envoy Gateway like DaemonSet #3031

zvlb · 2024-03-26T16:01:09Z

Issue - #2969

I decided not to wait for architecture discussions and implemented the functionality.

The issue suggested using the OpenTelemetry Operator structure for implementing the functionality. However, if we had followed the OpenTelemetry approach, we would have lost backward compatibility.

In the implemented functionality, the "old" logic remains intact, and the new logic is added for configuring Envoy Proxy as a DaemonSet.

A couple of points:

I haven't implemented the DaemonSet logic for EnvoyGateway, only for Gateway. (Here you can see a placeholder). I hope this is not urgent and not needed in the near future (we can create a new task to implement this logic).
I have only written tests for envoyService (if it doesn't need to be brought up). I'll write tests for the main functionality as soon as it's confirmed that everything is okay and this functionality is accepted.
I'll write documentation after confirmation of the functionality as well, too.

Signed-off-by: zvlb <[email protected]>

arkodg · 2024-03-27T09:23:37Z

api/v1alpha1/shared_types.go

+	// +optional
+	Patch *KubernetesPatchSpec `json:"patch,omitempty"`
+
+	// The daemonset strategy to use to replace existing pods with new ones.


for your use case, do you need all these fields - pod, container, initcontainers ?

In my case I test many cases, but most important:

kind: EnvoyProxy metadata: name: envoy-proxy-config namespace: envoy-gateway-system spec: provider: type: Kubernetes kubernetes: mode: DaemonSet envoyService: disable: true envoyDaemonSet: pod: hostNetwork: true tolerations: - key: node-role.kubernetes.io/control-plane operator: Exists nodeSelector: node-role.kubernetes.io/control-plane: ""

Start Envoy Proxy like daemonset on special nodes with hostnetwork=true

May be for another cases someone need this?

arkodg · 2024-03-27T09:25:06Z

api/v1alpha1/shared_types.go

+	// Default to false.
+	//
+	// +optional
+	HostNetwork bool `json:"hostNetwork,omitempty"`


this is something we dont recommend, its rm this, suggest using patch to achieve this

Why? It's what I realy need for my case.
patch - not very comfortable

-1 on hostNetwork, we should never recommend using it, patch can help edge case.

arkodg · 2024-03-27T09:25:16Z

api/v1alpha1/shared_types.go

@@ -221,6 +271,11 @@ const (
 // +kubebuilder:validation:XValidation:message="allocateLoadBalancerNodePorts can only be set for LoadBalancer type",rule="!has(self.allocateLoadBalancerNodePorts) || self.type == 'LoadBalancer'"
 // +kubebuilder:validation:XValidation:message="loadBalancerIP can only be set for LoadBalancer type",rule="!has(self.loadBalancerIP) || self.type == 'LoadBalancer'"
 type KubernetesServiceSpec struct {
+	// Disable disables service if set to true.
+	// By default, the service is enabled.


curious, why this is needed

Bc if I run EnvoyProxy like daemonSet with hostNetwork - I don't need service

arkodg · 2024-03-27T09:26:43Z

api/v1alpha1/envoyproxy_types.go

@@ -137,13 +137,25 @@ type ShutdownConfig struct {
 // EnvoyProxyKubernetesProvider defines configuration for the Kubernetes resource
 // provider.
 type EnvoyProxyKubernetesProvider struct {
+	// Mode is the type of the Kubernetes resource provider. If unspecified, it defaults to "Deployment".


do we need an explicit mode field or can we use CEL validation to make sure

if Deployment is set, then DaemonSet cannot be set

If in future someone add tag +kubebuilder:default:="****" to field in KubernetesDeploymentSpec or KubernetesDaemonSetSpec, then field envoyDaemonSet or envoyDaemonSetwill not be empty. How to implement this case?

the implicit behavior is a bit confusing

will that be rejected by validating webhook?

I'm not sure I understand what you're referring to. Does Envoy Gateway implement an Admission (Validation) webhook?

@arkodg Can you help here? Which kubebuilder tag need you for this logic?

use a ptr type called mode to decide b/w Deployment and DaemonSet, defaulting to Deployment

use CEL to make sure when mode is set to DaemonSet , user cannot populate the deployment field e.g.

gateway/api/v1alpha1/ext_auth_types.go

Line 13 in 0f87fcb

// +kubebuilder:validation:XValidation:rule="(has(self.grpc) && !has(self.http)) || (!has(self.grpc) && has(self.http))",message="only one of grpc or http can be specified"

do we need an explicit mode field or can we use CEL validation to make sure

May be we don't need mode? Only EnvoyDeployment and EnvoyDaemonSet fields with validation. If not set - use Deployment (with default preset)

do we need an explicit mode field or can we use CEL validation to make sure

May be we don't need mode? Only EnvoyDeployment and EnvoyDaemonSet fields with validation. If not set - use Deployment (with default preset)

I prefer this approach

zvlb · 2024-03-27T10:05:05Z

Here's a more detailed description of your case:

I have bare-metal installations of Kubernetes clusters. Since the ServiceType LoadBalancer is not supported in my setup, I need to deploy Envoy Proxy on specific nodes to route traffic to pods.

arkodg · 2024-03-27T11:19:53Z

Im not sure its going to work the way you've intended to

there's a servicePort/ gateway listener port - containerPort mapping
you will also need to set dns policy ClusterFirstWithHostNet
more in Add hostNetwork DNSPolicy support #2309
I'll defer to @zzjin @Xunzhuo for more details

zirain · 2024-03-27T11:45:14Z

please kindly proposal API first, otherwise there will be a lot of waste.

zvlb · 2024-04-03T14:23:25Z

Let's decide how to ultimately implement the necessary functionality and resolve contentious issues:

@arkodg believes that using the hostNetwork field is undesirable and suggests implementing this functionality through patch. I'm not sure if this is a good solution because it seems like we're providing users with functionality to manage the hostNetwork field, but doing so inconveniently. Considering there's a real use case for when this is needed, I see no reason not to expose this field.
@arkodg also suggested that we should expose the dnsPolicy field as controlled fields to set the value ClusterFirstWithHostNet for DaemonSets.
@arkodg сlarified whether we need to expose the container and initContainers fields for DaemonSet control. For my use case, this isn't necessary, but during implementation, I tried to maintain all the flexibility of DaemonSet configuration so that it works the same as Deployment.

Perhaps you want a different implementation for DaemonSet. I'm ready to listen to all proposals and implement what is approved.

arkodg · 2024-04-03T14:39:31Z

@zvlb I suggest breaking this up into 3 tasks

Adding support for DaemonSet
Any additional fields in PodSpec
Any addition fields in Service

Suggest starting with 1. to make faster progress/ easier to agree on a small set of things and add the minimum fields needed along with the Patch field that supports extensibility

zvlb · 2024-04-03T14:41:52Z

I will create 3 new PR for this. It's okay?

arkodg · 2024-04-03T14:46:26Z

I will create 3 new PR for this. It's okay?

sure, but reviewers will most likely review in above order since imo they range from must have to good to have

arkodg · 2024-04-09T06:28:54Z

@zvlb suggest running make lint, make test, make generate to repro the errors locally

zvlb · 2024-04-09T07:41:53Z

Actual PR - #3092

zvlb and others added 3 commits March 25, 2024 16:41

Add logic for disable service for envoy proxy

61461c8

Signed-off-by: zvlb <[email protected]>

add logic for daemonset

b80c517

Signed-off-by: zvlb <[email protected]>

Merge branch 'envoyproxy:main' into add-ds-kubernetes-provider

8f00ad3

zvlb requested a review from a team as a code owner March 26, 2024 16:01

arkodg reviewed Mar 27, 2024

View reviewed changes

zvlb mentioned this pull request Apr 4, 2024

Run Envoy Gateway like DaemonSet #3092

Merged

zvlb closed this Apr 9, 2024

zvlb deleted the add-ds-kubernetes-provider branch May 2, 2024 10:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run Envoy Gateway like DaemonSet #3031

Run Envoy Gateway like DaemonSet #3031

zvlb commented Mar 26, 2024 •

edited

Loading

arkodg Mar 27, 2024

zvlb Mar 27, 2024

arkodg Mar 27, 2024

zvlb Mar 27, 2024

zirain Mar 27, 2024

arkodg Mar 27, 2024

zvlb Mar 27, 2024

arkodg Mar 27, 2024

zvlb Mar 27, 2024

zirain Mar 27, 2024

zvlb Mar 27, 2024

zirain Mar 27, 2024

zvlb Apr 3, 2024

arkodg Apr 3, 2024

zvlb Apr 3, 2024 •

edited

Loading

arkodg Apr 4, 2024

zvlb commented Mar 27, 2024

arkodg commented Mar 27, 2024

zirain commented Mar 27, 2024

zvlb commented Apr 3, 2024

arkodg commented Apr 3, 2024

zvlb commented Apr 3, 2024

arkodg commented Apr 3, 2024

arkodg commented Apr 9, 2024

zvlb commented Apr 9, 2024

Run Envoy Gateway like DaemonSet #3031

Run Envoy Gateway like DaemonSet #3031

Conversation

zvlb commented Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zvlb Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zvlb commented Mar 27, 2024

arkodg commented Mar 27, 2024

zirain commented Mar 27, 2024

zvlb commented Apr 3, 2024

arkodg commented Apr 3, 2024

zvlb commented Apr 3, 2024

arkodg commented Apr 3, 2024

arkodg commented Apr 9, 2024

zvlb commented Apr 9, 2024

zvlb commented Mar 26, 2024 •

edited

Loading

zvlb Apr 3, 2024 •

edited

Loading