From 490ed1c94b815d004da9f970875998aac0771357 Mon Sep 17 00:00:00 2001 From: Tomash Sidei <43379202+tomashibm@users.noreply.github.com> Date: Thu, 19 May 2022 12:41:04 +0300 Subject: [PATCH] Allow to omit non ready backends (#70) * Allow users to choose if they want to include or exclude not ready backend pods from VCL. Signed-off-by: Tomash Sidei Co-authored-by: Craig Ingram --- api/v1alpha1/varnishcluster_types.go | 1 + .../bases/caching.ibm.com_varnishclusters.yaml | 2 ++ docs/varnish-cluster-configuration.md | 1 + docs/vcl-configuration.md | 8 +++++++- pkg/varnishcontroller/controller/endpoints.go | 17 ++++++++++++----- pkg/varnishcontroller/podutil/podutil.go | 16 ++++++++++++++++ .../predicates/label_matcher.go | 5 +++++ varnish-operator/crds/varnishcluster.yaml | 2 ++ 8 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 pkg/varnishcontroller/podutil/podutil.go diff --git a/api/v1alpha1/varnishcluster_types.go b/api/v1alpha1/varnishcluster_types.go index c049a1f8..971a7668 100644 --- a/api/v1alpha1/varnishcluster_types.go +++ b/api/v1alpha1/varnishcluster_types.go @@ -196,6 +196,7 @@ type VarnishClusterBackend struct { // +kubebuilder:validation:Required Port *intstr.IntOrString `json:"port,omitempty"` Namespaces []string `json:"namespaces,omitempty"` + OnlyReady bool `json:"onlyReady,omitempty"` ZoneBalancing *VarnishClusterBackendZoneBalancing `json:"zoneBalancing,omitempty"` } diff --git a/config/crd/bases/caching.ibm.com_varnishclusters.yaml b/config/crd/bases/caching.ibm.com_varnishclusters.yaml index 64da0455..de453e6c 100644 --- a/config/crd/bases/caching.ibm.com_varnishclusters.yaml +++ b/config/crd/bases/caching.ibm.com_varnishclusters.yaml @@ -868,6 +868,8 @@ spec: items: type: string type: array + onlyReady: + type: boolean port: anyOf: - type: integer diff --git a/docs/varnish-cluster-configuration.md b/docs/varnish-cluster-configuration.md index 5e93e692..6b03d2ea 100644 --- a/docs/varnish-cluster-configuration.md +++ b/docs/varnish-cluster-configuration.md @@ -4,6 +4,7 @@ |-------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------| | `affinity ` | [Affinity](https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity) settings for the pods. It allows you to configure onto which nodes Varnish pods should prefer being scheduled. | `optional` | | `backend.namespaces ` | Namespace(s) to look for backend pods. By default - namespace the VarnishCluster is deployed to. | `required` | +| `backend.onlyReady ` | Include (`false`, by default) or exclude (`true`) backend pods from the VCL (.Backends template var). Alters `.Backends` template variable based on Kubernetes health checks (by default not ready pods are also included in VCL) instead of [Varnish health probes](https://varnish-cache.org/docs/6.6/reference/vcl-probe.html#backend-health-probes). | `optional` | | `backend.port ` | The port of the backend pods being cached by Varnish. Can be port name or port number. | `required` | | `backend.selector ` | The selector used to identify the backend Pods. | `required` | | `backend.zoneBalancing ` | Controls Varnish backend topology aware routing which can assign weights to backends according to their geographical location. | `optional` | diff --git a/docs/vcl-configuration.md b/docs/vcl-configuration.md index ba60a88e..5bde3e1c 100644 --- a/docs/vcl-configuration.md +++ b/docs/vcl-configuration.md @@ -121,7 +121,13 @@ backend nginx-backend-6f4c6cbc6c-ckqmv { } ``` -The `.Backends` array includes all backend nodes, no matter if the instance is ready or not. For that reason, it is strongly recommended to set [health checks](https://varnish-cache.org/docs/3.0/tutorial/advanced_backend_servers.html#health-checks) in your VCL configuration. Otherwise, Varnish could send traffic to not yet ready backends. +The `.Backends` array can include both all backend nodes (default behavior) and only ready nodes. It depends on `.spec.backends.onlyReady` field. +It depends on which readiness checks you want to respect: +* Kubernetes readiness checks - field set to `true`. In this case the `.Backends` template var in VCL will include only ready backend pods. +Keep in mind that every backend state change will trigger a Varnish VCL config reload. +* Varnish readiness probes - field set to `false` or omitted. This way the `.Backends` array will include all scheduled pods, so it is strongly recommended to set [health checks](https://varnish-cache.org/docs/6.6/reference/vcl-probe.html#backend-health-probes) in your VCL configuration. Otherwise, Varnish could send traffic to not yet ready backends. + +You can also combine approaches and use both Kubernetes and Varnish health probes. ### Using User Defined VCL Code Versions diff --git a/pkg/varnishcontroller/controller/endpoints.go b/pkg/varnishcontroller/controller/endpoints.go index 90ffd337..e29de5ca 100644 --- a/pkg/varnishcontroller/controller/endpoints.go +++ b/pkg/varnishcontroller/controller/endpoints.go @@ -5,6 +5,8 @@ import ( "fmt" "sort" + "github.com/ibm/varnish-operator/pkg/varnishcontroller/podutil" + "github.com/ibm/varnish-operator/pkg/varnishcontroller/events" "github.com/ibm/varnish-operator/api/v1alpha1" @@ -34,12 +36,13 @@ func (r *ReconcileVarnish) getBackendEndpoints(ctx context.Context, vc *v1alpha1 actualLocalWeight := 1.0 actualRemoteWeight := 1.0 - namespaces := []string{r.config.Namespace} + ns := []string{r.config.Namespace} if len(vc.Spec.Backend.Namespaces) > 0 { - namespaces = vc.Spec.Backend.Namespaces + ns = vc.Spec.Backend.Namespaces } - backendList, portNumber, err := r.getPodsInfo(ctx, vc, namespaces, labels.SelectorFromSet(vc.Spec.Backend.Selector), *vc.Spec.Backend.Port) + selector := labels.SelectorFromSet(vc.Spec.Backend.Selector) + backendList, portNumber, err := r.getPodsInfo(ctx, vc, ns, selector, *vc.Spec.Backend.Port, vc.Spec.Backend.OnlyReady) if err != nil { return nil, 0, 0, 0, errors.WithStack(err) } @@ -105,7 +108,7 @@ func (r *ReconcileVarnish) getVarnishEndpoints(ctx context.Context, vc *v1alpha1 varnishLables := labels.SelectorFromSet(vclabels.CombinedComponentLabels(vc, v1alpha1.VarnishComponentVarnish)) varnishPort := intstr.FromString(v1alpha1.VarnishPortName) - varnishEndpoints, _, err := r.getPodsInfo(ctx, vc, []string{r.config.Namespace}, varnishLables, varnishPort) + varnishEndpoints, _, err := r.getPodsInfo(ctx, vc, []string{r.config.Namespace}, varnishLables, varnishPort, false) if err != nil { return nil, errors.WithStack(err) } @@ -113,7 +116,7 @@ func (r *ReconcileVarnish) getVarnishEndpoints(ctx context.Context, vc *v1alpha1 return varnishEndpoints, nil } -func (r *ReconcileVarnish) getPodsInfo(ctx context.Context, vc *v1alpha1.VarnishCluster, namespaces []string, labels labels.Selector, validPort intstr.IntOrString) ([]PodInfo, int32, error) { +func (r *ReconcileVarnish) getPodsInfo(ctx context.Context, vc *v1alpha1.VarnishCluster, namespaces []string, labels labels.Selector, validPort intstr.IntOrString, onlyReady bool) ([]PodInfo, int32, error) { var pods []v1.Pod for _, namespace := range namespaces { listOptions := []client.ListOption{ @@ -142,6 +145,10 @@ func (r *ReconcileVarnish) getPodsInfo(ctx context.Context, vc *v1alpha1.Varnish continue } + if onlyReady && !podutil.PodReady(pod) { + continue + } + portFound := false for _, container := range pod.Spec.Containers { for _, containerPort := range container.Ports { diff --git a/pkg/varnishcontroller/podutil/podutil.go b/pkg/varnishcontroller/podutil/podutil.go new file mode 100644 index 00000000..879f62f9 --- /dev/null +++ b/pkg/varnishcontroller/podutil/podutil.go @@ -0,0 +1,16 @@ +package podutil + +import v1 "k8s.io/api/core/v1" + +func PodReady(pod v1.Pod) bool { + if len(pod.Status.ContainerStatuses) == 0 { + return false + } + for _, status := range pod.Status.ContainerStatuses { + if !status.Ready { + return false + } + } + + return true +} diff --git a/pkg/varnishcontroller/predicates/label_matcher.go b/pkg/varnishcontroller/predicates/label_matcher.go index b65a33cd..0807210b 100644 --- a/pkg/varnishcontroller/predicates/label_matcher.go +++ b/pkg/varnishcontroller/predicates/label_matcher.go @@ -2,6 +2,7 @@ package predicates import ( "github.com/ibm/varnish-operator/pkg/logger" + "github.com/ibm/varnish-operator/pkg/varnishcontroller/podutil" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/event" @@ -55,6 +56,10 @@ func (p *LabelMatcherPredicate) Update(e event.UpdateEvent) bool { return true } + if podutil.PodReady(*newPod) != podutil.PodReady(*oldPod) { + return true + } + return false } diff --git a/varnish-operator/crds/varnishcluster.yaml b/varnish-operator/crds/varnishcluster.yaml index 453b9bbf..90d64855 100644 --- a/varnish-operator/crds/varnishcluster.yaml +++ b/varnish-operator/crds/varnishcluster.yaml @@ -871,6 +871,8 @@ spec: items: type: string type: array + onlyReady: + type: boolean port: anyOf: - type: integer