feat: add HoldApplicationUntilProxyStarts and GracefulExitUntilDownst…

…reamEnds features. (#517) * add HoldApplicationUntilProxyStarts feature. Signed-off-by: Cybwan <[email protected]> * add GracefulExitUntilDownstreamEnds feature. Signed-off-by: Cybwan <[email protected]> --------- Signed-off-by: Cybwan <[email protected]>
flomesh-io · Dec 16, 2024 · ddb051e · ddb051e
1 parent 07c2602
commit ddb051e
Show file tree

Hide file tree

Showing 28 changed files with 573 additions and 187 deletions.
diff --git a/charts/fsm/README.md b/charts/fsm/README.md
@@ -447,8 +447,10 @@ The following table lists the configurable parameters of the fsm chart and their
 | fsm.serviceLB.image.name | string | `"mirrored-klipper-lb"` | service-lb image name |
 | fsm.serviceLB.image.registry | string | `"flomesh"` | Registry for service-lb image |
 | fsm.serviceLB.image.tag | string | `"v0.4.7"` | service-lb image tag |
-| fsm.sidecar | object | `{"compressConfig":true,"image":{"name":"pipy","registry":"flomesh","tag":"1.5.8"},"sidecarDisabledMTLS":false,"sidecarLogLevel":"error","sidecarTimeout":60}` | Sidecar supported by fsm |
+| fsm.sidecar | object | `{"compressConfig":true,"gracefulExitUntilDownstreamEnds":true,"holdApplicationUntilProxyStarts":true,"image":{"name":"pipy","registry":"flomesh","tag":"1.5.8"},"sidecarDisabledMTLS":false,"sidecarLogLevel":"error","sidecarTimeout":60}` | Sidecar supported by fsm |
 | fsm.sidecar.compressConfig | bool | `true` | Sidecar compresses config.json |
+| fsm.sidecar.gracefulExitUntilDownstreamEnds | bool | `true` | This feature delays the pod proxy exit until active downstream connections end. |
+| fsm.sidecar.holdApplicationUntilProxyStarts | bool | `true` | This feature delays application startup until the pod proxy is ready to accept traffic, mitigating some startup race conditions. |
 | fsm.sidecar.image.name | string | `"pipy"` | Sidecar image name |
 | fsm.sidecar.image.registry | string | `"flomesh"` | Registry for sidecar image |
 | fsm.sidecar.image.tag | string | `"1.5.8"` | Sidecar image tag |

diff --git a/charts/fsm/templates/preset-mesh-config.yaml b/charts/fsm/templates/preset-mesh-config.yaml
@@ -25,6 +25,8 @@ data:
         "maxDataPlaneConnections": {{.Values.fsm.maxDataPlaneConnections | mustToJson}},
         "configResyncInterval": {{.Values.fsm.configResyncInterval | mustToJson}},
         "compressConfig": {{.Values.fsm.sidecar.compressConfig | mustToJson}},
+        "holdApplicationUntilProxyStarts": {{.Values.fsm.sidecar.holdApplicationUntilProxyStarts | mustToJson}},
+        "gracefulExitUntilDownstreamEnds": {{.Values.fsm.sidecar.gracefulExitUntilDownstreamEnds | mustToJson}},
         "sidecarImage": "{{ include "sidecar.image" .}}",
         "sidecarDisabledMTLS": {{.Values.fsm.sidecar.sidecarDisabledMTLS | mustToJson }},
         "sidecarTimeout": {{.Values.fsm.sidecar.sidecarTimeout | mustToJson}},

diff --git a/charts/fsm/values.schema.json b/charts/fsm/values.schema.json
@@ -816,6 +816,8 @@
                     "image",
                     "sidecarDisabledMTLS",
                     "compressConfig",
+                    "holdApplicationUntilProxyStarts",
+                    "gracefulExitUntilDownstreamEnds",
                     "sidecarLogLevel",
                     "sidecarTimeout"
                   ],
@@ -894,6 +896,24 @@
                         false
                       ]
                     },
+                    "holdApplicationUntilProxyStarts": {
+                      "$id": "#/properties/fsm/properties/sidecar/properties/holdApplicationUntilProxyStarts",
+                      "type": "boolean",
+                      "title": "The holdApplicationUntilProxyStarts schema",
+                      "description": "This feature delays application startup until the pod proxy is ready to accept traffic, mitigating some startup race conditions.",
+                      "examples": [
+                        false
+                      ]
+                    },
+                    "gracefulExitUntilDownstreamEnds": {
+                      "$id": "#/properties/fsm/properties/sidecar/properties/gracefulExitUntilDownstreamEnds",
+                      "type": "boolean",
+                      "title": "The gracefulExitUntilDownstreamEnds schema",
+                      "description": "This feature delays the pod proxy exit until active downstream connections end.",
+                      "examples": [
+                        false
+                      ]
+                    },
                     "sidecarLogLevel": {
                       "$id": "#/properties/fsm/properties/sidecar/properties/sidecarLogLevel",
                       "type": "string",

diff --git a/charts/fsm/values.yaml b/charts/fsm/values.yaml
@@ -85,6 +85,10 @@ fsm:
     sidecarDisabledMTLS: false
     # -- Sidecar compresses config.json
     compressConfig: true
+    # -- This feature delays application startup until the pod proxy is ready to accept traffic, mitigating some startup race conditions.
+    holdApplicationUntilProxyStarts: true
+    # -- This feature delays the pod proxy exit until active downstream connections end.
+    gracefulExitUntilDownstreamEnds: true
     # -- Log level for the proxy sidecar. Non developers should generally never set this value. In production environments the LogLevel should be set to `error`
     sidecarLogLevel: error
     # -- Sets connect/idle/read/write timeout

diff --git a/cmd/fsm-bootstrap/crds/config.flomesh.io_meshconfigs.yaml b/cmd/fsm-bootstrap/crds/config.flomesh.io_meshconfigs.yaml
@@ -1969,6 +1969,12 @@ spec:
                     description: EnablePrivilegedInitContainer defines a boolean indicating
                       whether the init container for a meshed pod should run as privileged.
                     type: boolean
+                  gracefulExitUntilDownstreamEnds:
+                    default: true
+                    description: |-
+                      GracefulExitUntilDownstreamEnds feature delays the pod proxy exit until active downstream connections end.
+                      Default value is 'true'.
+                    type: boolean
                   healthcheckResources:
                     description: HealthcheckResources defines the compute resources
                       for init container.
@@ -2029,6 +2035,13 @@ spec:
                           More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
                         type: object
                     type: object
+                  holdApplicationUntilProxyStarts:
+                    default: true
+                    description: |-
+                      HoldApplicationUntilProxyStarts feature delays application startup until the pod proxy
+                      is ready to accept traffic, mitigating some startup race conditions.
+                      Default value is 'true'.
+                    type: boolean
                   initResources:
                     description: InitResources defines the compute resources for init
                       container.

diff --git a/pkg/apis/config/v1alpha3/mesh_config.go b/pkg/apis/config/v1alpha3/mesh_config.go
@@ -152,6 +152,19 @@ type SidecarSpec struct {
 	// +optional
 	CompressConfig bool `json:"compressConfig"`
 
+	// HoldApplicationUntilProxyStarts feature delays application startup until the pod proxy
+	// is ready to accept traffic, mitigating some startup race conditions.
+	// Default value is 'true'.
+	// +kubebuilder:default=true
+	// +optional
+	HoldApplicationUntilProxyStarts bool `json:"holdApplicationUntilProxyStarts"`
+
+	// GracefulExitUntilDownstreamEnds feature delays the pod proxy exit until active downstream connections end.
+	// Default value is 'true'.
+	// +kubebuilder:default=true
+	// +optional
+	GracefulExitUntilDownstreamEnds bool `json:"gracefulExitUntilDownstreamEnds"`
+
 	// LogLevel defines the logging level for the sidecar's logs. Non developers should generally never set this value. In production environments the LogLevel should be set to error.
 	LogLevel string `json:"logLevel,omitempty"`
 

diff --git a/pkg/configurator/methods.go b/pkg/configurator/methods.go
@@ -117,6 +117,16 @@ func (c *Client) GetLocalDNSProxySecondaryUpstream() string {
 	return c.getMeshConfig().Spec.Sidecar.LocalDNSProxy.SecondaryUpstreamDNSServerIPAddr
 }
 
+// IsHoldApplicationUntilProxyStarts returns whether hold application until proxy starts
+func (c *Client) IsHoldApplicationUntilProxyStarts() bool {
+	return c.getMeshConfig().Spec.Sidecar.HoldApplicationUntilProxyStarts
+}
+
+// IsGracefulExitUntilDownstreamEnds returns whether delays the pod proxy exit until active downstream connections end
+func (c *Client) IsGracefulExitUntilDownstreamEnds() bool {
+	return c.getMeshConfig().Spec.Sidecar.GracefulExitUntilDownstreamEnds
+}
+
 // GenerateIPv6BasedOnIPv4 returns whether auto generate IPv6 based on IPv4
 func (c *Client) GenerateIPv6BasedOnIPv4() bool {
 	return c.getMeshConfig().Spec.Sidecar.LocalDNSProxy.GenerateIPv6BasedOnIPv4

diff --git a/pkg/configurator/mock_client_generated.go b/pkg/configurator/mock_client_generated.go
diff --git a/pkg/configurator/types.go b/pkg/configurator/types.go
@@ -65,6 +65,13 @@ type Configurator interface {
 	// GetLocalDNSProxySecondaryUpstream returns the secondary upstream DNS server for local DNS Proxy
 	GetLocalDNSProxySecondaryUpstream() string
 
+	// IsHoldApplicationUntilProxyStarts returns whether delay application startup
+	// until the pod proxy is ready to accept traffic
+	IsHoldApplicationUntilProxyStarts() bool
+
+	// IsGracefulExitUntilDownstreamEnds returns whether delays the pod proxy exit until active downstream connections end
+	IsGracefulExitUntilDownstreamEnds() bool
+
 	// GenerateIPv6BasedOnIPv4 returns whether auto generate IPv6 based on IPv4
 	GenerateIPv6BasedOnIPv4() bool
 

diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
@@ -215,6 +215,14 @@ const (
 	// SidecarInjectionAnnotation is the annotation used for sidecar injection
 	SidecarInjectionAnnotation = "flomesh.io/sidecar-injection"
 
+	// HoldApplicationUntilProxyStartsAnnotation is the annotation to delay application startup
+	// until the pod proxy is ready to accept traffic, mitigating some startup race conditions.
+	HoldApplicationUntilProxyStartsAnnotation = "flomesh.io/hold-application-until-proxy-starts"
+
+	// GracefulExitUntilDownstreamEndsAnnotation is the annotation to delays the pod proxy exit
+	// until active downstream connections end.
+	GracefulExitUntilDownstreamEndsAnnotation = "flomesh.io/graceful-exit-until-downstream-ends"
+
 	// SidecarImageAnnotation is the annotation used for sidecar injection
 	SidecarImageAnnotation = "flomesh.io/sidecar-image"
 

diff --git a/pkg/injector/webhook.go b/pkg/injector/webhook.go
@@ -327,7 +327,7 @@ func isAnnotatedForInjection(annotations map[string]string, objectKind string, o
 	case "disabled", "no", "false":
 		enabled = false
 	default:
-		err = fmt.Errorf("Invalid annotation value for key %q: %s", constants.SidecarInjectionAnnotation, inject)
+		err = fmt.Errorf("invalid annotation value for key %q: %s", constants.SidecarInjectionAnnotation, inject)
 	}
 	return
 }

diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go
@@ -493,37 +493,54 @@ func GetTargetPortFromEndpoints(endpointName string, endpoints corev1.Endpoints)
 }
 
 func (c *client) GetPodForProxy(proxy models.Proxy) (*v1.Pod, error) {
+	var pod *corev1.Pod
 	proxyUUID, svcAccount := proxy.GetUUID().String(), proxy.GetIdentity().ToK8sServiceAccount()
 	log.Trace().Msgf("Looking for pod with label %q=%q", constants.SidecarUniqueIDLabelName, proxyUUID)
-	podList := c.ListPods()
-	var pods []v1.Pod
 
-	for _, pod := range podList {
-		if uuid, labelFound := pod.Labels[constants.SidecarUniqueIDLabelName]; labelFound && uuid == proxyUUID {
-			pods = append(pods, *pod)
+	if len(proxy.GetPodNamespace()) > 0 && len(proxy.GetPodName()) > 0 {
+		podIf, exists, err := c.informers.GetByKey(fsminformers.InformerKeyPod, fmt.Sprintf("%s/%s", proxy.GetPodNamespace(), proxy.GetPodName()))
+		if podIf == nil || !exists || err != nil {
+			return nil, errDidNotFindPodForUUID
 		}
-	}
+		pod = podIf.(*corev1.Pod)
+		if uuid, labelFound := pod.Labels[constants.SidecarUniqueIDLabelName]; !labelFound || uuid != proxyUUID {
+			log.Info().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingPodFromCert)).
+				Msgf("Did not find Pod with label %s = %s in namespace %s",
+					constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
+			return nil, errDidNotFindPodForUUID
+		}
+	} else {
+		var pods []v1.Pod
+		podList := c.ListPods()
 
-	if len(pods) == 0 {
-		log.Info().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingPodFromCert)).
-			Msgf("Did not find Pod with label %s = %s in namespace %s",
-				constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
-		return nil, errDidNotFindPodForUUID
-	}
+		for _, pod := range podList {
+			if uuid, labelFound := pod.Labels[constants.SidecarUniqueIDLabelName]; labelFound && uuid == proxyUUID {
+				pods = append(pods, *pod)
+			}
+		}
+
+		if len(pods) == 0 {
+			log.Info().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingPodFromCert)).
+				Msgf("Did not find Pod with label %s = %s in namespace %s",
+					constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
+			return nil, errDidNotFindPodForUUID
+		}
+
+		// Each pod is assigned a unique UUID at the time of sidecar injection.
+		// The certificate's CommonName encodes this UUID, and we lookup the pod
+		// whose label matches this UUID.
+		// Only 1 pod must match the UUID encoded in the given certificate. If multiple
+		// pods match, it is an error.
+		if len(pods) > 1 {
+			log.Error().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrPodBelongsToMultipleServices)).
+				Msgf("Found more than one pod with label %s = %s in namespace %s. There can be only one!",
+					constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
+			return nil, errMoreThanOnePodForUUID
+		}
 
-	// Each pod is assigned a unique UUID at the time of sidecar injection.
-	// The certificate's CommonName encodes this UUID, and we lookup the pod
-	// whose label matches this UUID.
-	// Only 1 pod must match the UUID encoded in the given certificate. If multiple
-	// pods match, it is an error.
-	if len(pods) > 1 {
-		log.Error().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrPodBelongsToMultipleServices)).
-			Msgf("Found more than one pod with label %s = %s in namespace %s. There can be only one!",
-				constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
-		return nil, errMoreThanOnePodForUUID
+		pod = &pods[0]
 	}
 
-	pod := pods[0]
 	log.Trace().Msgf("Found Pod with UID=%s for proxyID %s", pod.ObjectMeta.UID, proxyUUID)
 
 	if pod.Namespace != svcAccount.Namespace {
@@ -543,41 +560,58 @@ func (c *client) GetPodForProxy(proxy models.Proxy) (*v1.Pod, error) {
 		return nil, errServiceAccountDoesNotMatchProxy
 	}
 
-	return &pod, nil
+	return pod, nil
 }
 
 func (c *client) GetVmForProxy(proxy models.Proxy) (*machinev1alpha1.VirtualMachine, error) {
+	var vm *machinev1alpha1.VirtualMachine
 	proxyUUID, svcAccount := proxy.GetUUID().String(), proxy.GetIdentity().ToK8sServiceAccount()
 	log.Trace().Msgf("Looking for VM with label %q=%q", constants.SidecarUniqueIDLabelName, proxyUUID)
-	vmList := c.ListVms()
-	var vms []machinev1alpha1.VirtualMachine
 
-	for _, vm := range vmList {
-		if uuid, labelFound := vm.Labels[constants.SidecarUniqueIDLabelName]; labelFound && uuid == proxyUUID {
-			vms = append(vms, *vm)
+	if len(proxy.GetPodNamespace()) > 0 && len(proxy.GetPodName()) > 0 {
+		vmIf, exists, err := c.informers.GetByKey(fsminformers.InformerKeyVirtualMachine, fmt.Sprintf("%s/%s", proxy.GetPodNamespace(), proxy.GetPodName()))
+		if vmIf == nil || !exists || err != nil {
+			return nil, errDidNotFindPodForUUID
 		}
-	}
+		vm = vmIf.(*machinev1alpha1.VirtualMachine)
+		if uuid, labelFound := vm.Labels[constants.SidecarUniqueIDLabelName]; !labelFound || uuid != proxyUUID {
+			log.Info().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingPodFromCert)).
+				Msgf("Did not find VM with label %s = %s in namespace %s",
+					constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
+			return nil, errDidNotFindPodForUUID
+		}
+	} else {
+		vmList := c.ListVms()
+		var vms []machinev1alpha1.VirtualMachine
 
-	if len(vms) == 0 {
-		log.Info().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingPodFromCert)).
-			Msgf("Did not find VM with label %s = %s in namespace %s",
-				constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
-		return nil, errDidNotFindPodForUUID
-	}
+		for _, vm := range vmList {
+			if uuid, labelFound := vm.Labels[constants.SidecarUniqueIDLabelName]; labelFound && uuid == proxyUUID {
+				vms = append(vms, *vm)
+			}
+		}
+
+		if len(vms) == 0 {
+			log.Info().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingPodFromCert)).
+				Msgf("Did not find VM with label %s = %s in namespace %s",
+					constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
+			return nil, errDidNotFindPodForUUID
+		}
+
+		// Each VM is assigned a unique UUID at the time of sidecar injection.
+		// The certificate's CommonName encodes this UUID, and we lookup the vm
+		// whose label matches this UUID.
+		// Only 1 vm must match the UUID encoded in the given certificate. If multiple
+		// vms match, it is an error.
+		if len(vms) > 1 {
+			log.Error().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrPodBelongsToMultipleServices)).
+				Msgf("Found more than one vm with label %s = %s in namespace %s. There can be only one!",
+					constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
+			return nil, errMoreThanOnePodForUUID
+		}
 
-	// Each VM is assigned a unique UUID at the time of sidecar injection.
-	// The certificate's CommonName encodes this UUID, and we lookup the vm
-	// whose label matches this UUID.
-	// Only 1 vm must match the UUID encoded in the given certificate. If multiple
-	// vms match, it is an error.
-	if len(vms) > 1 {
-		log.Error().Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrPodBelongsToMultipleServices)).
-			Msgf("Found more than one vm with label %s = %s in namespace %s. There can be only one!",
-				constants.SidecarUniqueIDLabelName, proxyUUID, svcAccount.Namespace)
-		return nil, errMoreThanOnePodForUUID
+		vm = &vms[0]
 	}
 
-	vm := vms[0]
 	log.Trace().Msgf("Found VM with UID=%s for proxyID %s", vm.ObjectMeta.UID, proxyUUID)
 
 	if vm.Namespace != svcAccount.Namespace {
@@ -597,7 +631,7 @@ func (c *client) GetVmForProxy(proxy models.Proxy) (*machinev1alpha1.VirtualMach
 		return nil, errServiceAccountDoesNotMatchProxy
 	}
 
-	return &vm, nil
+	return vm, nil
 }
 
 // GetTargetPortForServicePort returns the TargetPort corresponding to the Port used by clients

diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go
@@ -1106,7 +1106,9 @@ type proxy struct {
 	// UUID of the proxy
 	uuid.UUID
 	// Identity is of the form <name>.<namespace>.cluster.local
-	Identity identity.ServiceIdentity
+	Identity     identity.ServiceIdentity
+	podName      string
+	podNamespace string
 	// kind is the proxy's kind (ex. sidecar, gateway)
 	kind models.ProxyKind
 }
@@ -1119,6 +1121,14 @@ func (p proxy) GetIdentity() identity.ServiceIdentity {
 	return p.Identity
 }
 
+func (p *proxy) GetPodName() string {
+	return p.podName
+}
+
+func (p *proxy) GetPodNamespace() string {
+	return p.podNamespace
+}
+
 func (p proxy) GetConnectedAt() time.Time {
 	return time.Now()
 }