Bug 1709958: Use graceful shutdown for router

assets/router/deployment.yaml

@@ -36,12 +36,12 @@ spec:
           livenessProbe:
             initialDelaySeconds: 10
             httpGet:
-              path: /healthz
+              path: /healthz/live
               port: 1936
           readinessProbe:
             initialDelaySeconds: 10
             httpGet:
-              path: /healthz
+              path: /healthz/ready
               port: 1936
           resources:
             requests:

manifests/00-cluster-role.yaml

@@ -31,6 +31,13 @@ rules:
   verbs:
   - "*"
 
+- apiGroups:
+  - policy
+  resources:
+  - poddisruptionbudgets
+  verbs:
+  - "*"
+
 - apiGroups:
   - monitoring.coreos.com
   resources:

pkg/operator/controller/controller.go

@@ -384,20 +384,26 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d
 			Controller: &trueVar,
 		}
 
-		if lbService, err := r.ensureLoadBalancerService(ci, deploymentRef, infraConfig); err != nil {
+		if wantHealthz, healthzSvc, err := r.ensureHealthzService(ci, deploymentRef); err != nil {
+			errs = append(errs, fmt.Errorf("failed to create healthz service for ingresscontroller %s: %v", ci.Name, err))
+		} else if haveLBService, lbService, err := r.ensureLoadBalancerService(ci, wantHealthz, healthzSvc, deploymentRef, infraConfig); err != nil {
 			errs = append(errs, fmt.Errorf("failed to ensure load balancer service for %s: %v", ci.Name, err))
-		} else if lbService != nil {
+		} else if haveLBService {
 			if err := r.ensureDNS(ci, lbService, dnsConfig); err != nil {
 				errs = append(errs, fmt.Errorf("failed to ensure DNS for %s: %v", ci.Name, err))
 			}
 		}
 
-		if internalSvc, err := r.ensureInternalIngressControllerService(ci, deploymentRef); err != nil {
+		if _, internalSvc, err := r.ensureInternalIngressControllerService(ci, deploymentRef); err != nil {
 			errs = append(errs, fmt.Errorf("failed to create internal router service for ingresscontroller %s: %v", ci.Name, err))
 		} else if err := r.ensureMetricsIntegration(ci, internalSvc, deploymentRef); err != nil {
 			errs = append(errs, fmt.Errorf("failed to integrate metrics with openshift-monitoring for ingresscontroller %s: %v", ci.Name, err))
 		}
 
+		if _, _, err := r.ensureRouterPodDisruptionBudget(ci, deploymentRef); err != nil {
+			errs = append(errs, err)
+		}
+
 		if err := r.syncIngressControllerStatus(deployment, ci); err != nil {
 			errs = append(errs, fmt.Errorf("failed to sync ingresscontroller status: %v", err))
 		}

pkg/operator/controller/controller_healthz_service.go

@@ -0,0 +1,117 @@
+package controller
+
+import (
+	"context"
+	"fmt"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	"github.com/openshift/cluster-ingress-operator/pkg/manifests"
+
+	corev1 "k8s.io/api/core/v1"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// ensureHealthzService ensures that a healthz service exists for the given
+// ingresscontroller.  Returns a Boolean indicating whether the service exists,
+// the service if it does exist, and an error value.
+func (r *reconciler) ensureHealthzService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service, error) {
+	haveHealthzService, current, err := r.currentHealthzService(ic)
+	if err != nil {
+		return false, nil, err
+	}
+	wantHealthzService, desired := desiredHealthzService(ic, deploymentRef)
+	switch {
+	case !wantHealthzService && !haveHealthzService:
+		return false, nil, nil
+	case !wantHealthzService && haveHealthzService:
+		if deleted, err := r.deleteHealthzService(current); err != nil {
+			return true, current, fmt.Errorf("failed to delete healthz service: %v", err)
+		} else if deleted {
+			log.Info("deleted healthz service", "service", current)
+		}
+	case wantHealthzService && !haveHealthzService:
+		if created, err := r.createHealthzService(desired); err != nil {
+			return false, nil, fmt.Errorf("failed to create healthz service: %v", err)
+		} else if created {
+			log.Info("created healthz service", "service", desired)
+		}
+	case wantHealthzService && haveHealthzService:
+		return true, current, nil
+	}
+	return r.currentHealthzService(ic)
+}
+
+// currentHealthzService gets the current healthz service, if any, for the given
+// ingresscontroller.  Returns a Boolean indicating whether the service existed,
+// the service if it did exist, and an error value.
+func (r *reconciler) currentHealthzService(ic *operatorv1.IngressController) (bool, *corev1.Service, error) {
+	current := &corev1.Service{}
+	err := r.client.Get(context.TODO(), HealthzServiceName(ic), current)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil, nil
+		}
+		return false, nil, err
+	}
+	return true, current, nil
+}
+
+// desiredHealthzService returns the desired healthz service, if any, for the
+// given ingresscontroller.  Returns a Boolean indicating whether a service is
+// desired, as well as the service if one is desired.
+func desiredHealthzService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service) {
+	if ic.Status.EndpointPublishingStrategy.Type != operatorv1.LoadBalancerServiceStrategyType {
+		return false, nil
+	}
+
+	name := HealthzServiceName(ic)
+	service := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name.Name,
+			Namespace: name.Namespace,
+			Labels: map[string]string{
+				manifests.OwningIngressControllerLabel: ic.Name,
+			},
+		},
+		Spec: corev1.ServiceSpec{
+			Type:     corev1.ServiceTypeNodePort,
+			ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal,
+			Selector: IngressControllerDeploymentPodSelector(ic).MatchLabels,
+			Ports: []corev1.ServicePort{
+				{
+					Name:       "metrics",
+					Port:       int32(1936),
+					Protocol:   corev1.ProtocolTCP,
+					TargetPort: intstr.FromInt(1936),
+				},
+			},
+		},
+	}
+	service.SetOwnerReferences([]metav1.OwnerReference{deploymentRef})
+
+	return true, service
+}
+
+// createHealthzService creates the given healthz service.  Returns a Boolean
+// indicating whether the service was created, and an error value.
+func (r *reconciler) createHealthzService(s *corev1.Service) (bool, error) {
+	if err := r.client.Create(context.TODO(), s); err != nil {
+		return false, err
+	}
+	return true, nil
+}
+
+// deleteHealthzService deletes the given healthz service.  Returns a Boolean
+// indicating whether the service was deleted, and an error value.
+func (r *reconciler) deleteHealthzService(s *corev1.Service) (bool, error) {
+	if err := r.client.Delete(context.TODO(), s); err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	return true, nil
+}

pkg/operator/controller/controller_internal_service.go

@@ -19,38 +19,51 @@ const (
 	ServingCertSecretAnnotation = "service.alpha.openshift.io/serving-cert-secret-name"
 )
 
-// ensureInternalRouterServiceForIngress ensures that an internal service exists
-// for a given IngressController.
-func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (*corev1.Service, error) {
-	desired := desiredInternalIngressControllerService(ic, deploymentRef)
-	current, err := r.currentInternalIngressControllerService(ic)
+// ensureInternalIngressControllerService ensures that a ClusterIP internal
+// service exists for the given ingresscontroller.  Returns a Boolean indicating
+// whether the service exists, the service if it does exist, and an error value.
+func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service, error) {
+	wantInternalService, desired := desiredInternalService(ic, deploymentRef)
+	haveInternalService, current, err := r.currentInternalService(ic)
 	if err != nil {
-		return nil, err
-	}
-	if current != nil {
-		return current, nil
+		return false, nil, err
 	}
 
-	if err := r.client.Create(context.TODO(), desired); err != nil {
-		return nil, fmt.Errorf("failed to create internal ingresscontroller service: %v", err)
+	switch {
+	case !wantInternalService && !haveInternalService:
+		return false, nil, nil
+	case !wantInternalService && haveInternalService:
+		if deleted, err := r.deleteInternalService(current); err != nil {
+			return true, current, fmt.Errorf("failed to delete internal service: %v", err)
+		} else if deleted {
+			log.Info("deleted internal service", "service", current)
+		}
+	case wantInternalService && !haveInternalService:
+		if created, err := r.createInternalService(desired); err != nil {
+			return false, nil, fmt.Errorf("failed to create internal service: %v", err)
+		} else if created {
+			log.Info("created internal service", "service", desired)
+		}
+	case wantInternalService && haveInternalService:
+		return true, current, nil
 	}
-	log.Info("created internal ingresscontroller service", "service", desired)
-	return desired, nil
+
+	return r.currentInternalService(ic)
 }
 
-func (r *reconciler) currentInternalIngressControllerService(ic *operatorv1.IngressController) (*corev1.Service, error) {
+func (r *reconciler) currentInternalService(ic *operatorv1.IngressController) (bool, *corev1.Service, error) {
 	current := &corev1.Service{}
 	err := r.client.Get(context.TODO(), InternalIngressControllerServiceName(ic), current)
 	if err != nil {
 		if errors.IsNotFound(err) {
-			return nil, nil
+			return false, nil, nil
 		}
-		return nil, err
+		return false, nil, err
 	}
-	return current, nil
+	return true, current, nil
 }
 
-func desiredInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) *corev1.Service {
+func desiredInternalService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service) {
 	s := manifests.InternalIngressControllerService()
 
 	name := InternalIngressControllerServiceName(ic)
@@ -71,5 +84,26 @@ func desiredInternalIngressControllerService(ic *operatorv1.IngressController, d
 
 	s.SetOwnerReferences([]metav1.OwnerReference{deploymentRef})
 
-	return s
+	return true, s
+}
+
+// createInternalService creates the given internal service.  Returns a Boolean
+// indicating whether the service was created, and an error value.
+func (r *reconciler) createInternalService(s *corev1.Service) (bool, error) {
+	if err := r.client.Create(context.TODO(), s); err != nil {
+		return false, err
+	}
+	return true, nil
+}
+
+// deleteInternalService deletes the given internal service.  Returns a Boolean
+// indicating whether the service was deleted, and an error value.
+func (r *reconciler) deleteInternalService(s *corev1.Service) (bool, error) {
+	if err := r.client.Delete(context.TODO(), s); err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	return true, nil
 }