diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go index 58ae939148..1adf8b8d41 100644 --- a/pkg/operator/controller/ingress/controller.go +++ b/pkg/operator/controller/ingress/controller.go @@ -337,18 +337,18 @@ func validateDomainUniqueness(desired *operatorv1.IngressController, existing [] // ensureIngressDeleted tries to delete ingress, and if successful, will remove // the finalizer. func (r *reconciler) ensureIngressDeleted(ingress *operatorv1.IngressController) error { + errs := []error{} if err := r.finalizeLoadBalancerService(ingress); err != nil { - return fmt.Errorf("failed to finalize load balancer service for %s: %v", ingress.Name, err) + errs = append(errs, fmt.Errorf("failed to finalize load balancer service for %s/%s: %v", ingress.Namespace, ingress.Name, err)) } - log.Info("finalized load balancer service for ingress", "namespace", ingress.Namespace, "name", ingress.Name) // Delete the wildcard DNS record, and block ingresscontroller finalization // until the dnsrecord has been finalized. if err := r.deleteWildcardDNSRecord(ingress); err != nil { - return fmt.Errorf("failed to delete wildcard dnsrecord: %v", err) + errs = append(errs, fmt.Errorf("failed to delete wildcard dnsrecord: %v", err)) } if record, err := r.currentWildcardDNSRecord(ingress); err != nil { - return fmt.Errorf("failed to get current wildcard dnsrecord: %v", err) + errs = append(errs, fmt.Errorf("failed to get current wildcard dnsrecord: %v", err)) } else { if record != nil { log.V(1).Info("waiting for wildcard dnsrecord to be deleted", "dnsrecord", record) @@ -357,19 +357,21 @@ func (r *reconciler) ensureIngressDeleted(ingress *operatorv1.IngressController) } if err := r.ensureRouterDeleted(ingress); err != nil { - return fmt.Errorf("failed to delete deployment for ingress %s: %v", ingress.Name, err) + errs = append(errs, fmt.Errorf("failed to delete deployment for ingress %s: %v", ingress.Name, err)) } - log.Info("deleted deployment for ingress", "namespace", ingress.Namespace, "name", ingress.Name) - // Clean up the finalizer to allow the ingresscontroller to be deleted. - if slice.ContainsString(ingress.Finalizers, manifests.IngressControllerFinalizer) { - updated := ingress.DeepCopy() - updated.Finalizers = slice.RemoveString(updated.Finalizers, manifests.IngressControllerFinalizer) - if err := r.client.Update(context.TODO(), updated); err != nil { - return fmt.Errorf("failed to remove finalizer from ingresscontroller %s: %v", ingress.Name, err) + if len(errs) == 0 { + // Remove the "ingresscontroller.operator.openshift.io/finalizer-ingresscontroller" finalizer + // to allow the ingresscontroller to be deleted. + if slice.ContainsString(ingress.Finalizers, manifests.IngressControllerFinalizer) { + updated := ingress.DeepCopy() + updated.Finalizers = slice.RemoveString(updated.Finalizers, manifests.IngressControllerFinalizer) + if err := r.client.Update(context.TODO(), updated); err != nil { + errs = append(errs, fmt.Errorf("failed to remove finalizer from ingresscontroller %s: %v", ingress.Name, err)) + } } } - return nil + return utilerrors.NewAggregate(errs) } // ensureIngressController ensures all necessary router resources exist for a given ingresscontroller. diff --git a/pkg/operator/controller/ingress/deployment.go b/pkg/operator/controller/ingress/deployment.go index 5670cc4e5e..31fa34dc5e 100644 --- a/pkg/operator/controller/ingress/deployment.go +++ b/pkg/operator/controller/ingress/deployment.go @@ -60,6 +60,8 @@ func (r *reconciler) ensureRouterDeleted(ci *operatorv1.IngressController) error return err } } + log.Info("deleted deployment", "namespace", deployment.Namespace, "name", deployment.Name) + r.recorder.Eventf(ci, "Normal", "DeletedDeployment", "Deleted deployment %s/%s", deployment.Namespace, deployment.Name) return nil } diff --git a/pkg/operator/controller/ingress/load_balancer_service.go b/pkg/operator/controller/ingress/load_balancer_service.go index 48c232348a..7935881147 100644 --- a/pkg/operator/controller/ingress/load_balancer_service.go +++ b/pkg/operator/controller/ingress/load_balancer_service.go @@ -23,6 +23,10 @@ const ( // // https://kubernetes.io/docs/concepts/services-networking/service/#proxy-protocol-support-on-aws awsLBProxyProtocolAnnotation = "service.beta.kubernetes.io/aws-load-balancer-proxy-protocol" + // lbServiceCleanUpFinalizer is a finalizer attached to any service that has type=LoadBalancer. + // Upon deletion of the service, the actual deletion of the resource will be blocked until + // this finalizer is removed. + lbServiceCleanUpFinalizer = "service.kubernetes.io/load-balancer-cleanup" ) var ( @@ -135,10 +139,10 @@ func (r *reconciler) currentLoadBalancerService(ci *operatorv1.IngressController return service, nil } -// finalizeLoadBalancerService removes finalizers from any LB service. This used -// to be to help with DNS cleanup, but now that's no longer necessary, and so we -// just need to clear the finalizer which might exist on existing resources. -// +// finalizeLoadBalancerService removes the "ingress.openshift.io/operator" finalizer +// from the load balancer service of ci. This was for helping with DNS cleanup, but +// that's no longer necessary. We just need to clear the finalizer which might exist +// on existing resources. // TODO: How can we delete this code? func (r *reconciler) finalizeLoadBalancerService(ci *operatorv1.IngressController) error { service, err := r.currentLoadBalancerService(ci) @@ -157,5 +161,11 @@ func (r *reconciler) finalizeLoadBalancerService(ci *operatorv1.IngressControlle return fmt.Errorf("failed to remove finalizer from service %s for ingress %s/%s: %v", service.Namespace, service.Name, ci.Name, err) } } + // The load balancer service is not deleted until the cloud infra is cleaned-up. For more details, see: + // https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/20190423-service-lb-finalizer.md + if slice.ContainsString(service.Finalizers, lbServiceCleanUpFinalizer) { + return fmt.Errorf("finalizer %s exists for service %s/%s", lbServiceCleanUpFinalizer, service.Namespace, service.Name) + } + log.Info("finalized load balancer service for ingress", "namespace", ci.Namespace, "name", ci.Name) return nil }