diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go index a3ab08a939..e100b2701a 100644 --- a/pkg/operator/controller/ingress/controller.go +++ b/pkg/operator/controller/ingress/controller.go @@ -501,31 +501,34 @@ func filterTLS13Ciphers(ciphers []string) []string { // the finalizer. func (r *reconciler) ensureIngressDeleted(ingress *operatorv1.IngressController) error { errs := []error{} - if err := r.finalizeLoadBalancerService(ingress); err != nil { - errs = append(errs, fmt.Errorf("failed to finalize load balancer service for %s/%s: %v", ingress.Namespace, ingress.Name, err)) + if svcExists, err := r.finalizeLoadBalancerService(ingress); err != nil { + errs = append(errs, fmt.Errorf("failed to finalize load balancer service for ingress %s/%s: %v", ingress.Namespace, ingress.Name, err)) + } else if svcExists { + errs = append(errs, fmt.Errorf("load balancer service exists for ingress %s/%s", ingress.Namespace, ingress.Name)) } // Delete the wildcard DNS record, and block ingresscontroller finalization // until the dnsrecord has been finalized. if err := r.deleteWildcardDNSRecord(ingress); err != nil { - errs = append(errs, fmt.Errorf("failed to delete wildcard dnsrecord: %v", err)) + errs = append(errs, fmt.Errorf("failed to delete wildcard dnsrecord for ingress %s/%s: %v", ingress.Namespace, ingress.Name, err)) } if record, err := r.currentWildcardDNSRecord(ingress); err != nil { - 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) - return nil - } + errs = append(errs, fmt.Errorf("failed to get current wildcard dnsrecord for ingress %s/%s: %v", ingress.Namespace, ingress.Name, err)) + } else if record != nil { + errs = append(errs, fmt.Errorf("wildcard dnsrecord exists for ingress %s/%s", ingress.Namespace, ingress.Name)) } if err := r.ensureRouterDeleted(ingress); err != nil { - errs = append(errs, 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/%s: %v", ingress.Namespace, ingress.Name, err)) + } + if deploy, err := r.currentRouterDeployment(ingress); err != nil { + errs = append(errs, fmt.Errorf("failed to get deployment for ingress %s/%s: %v", ingress.Namespace, ingress.Name, err)) + } else if deploy != nil { + errs = append(errs, fmt.Errorf("deployment still exists for ingress %s/%s", ingress.Namespace, ingress.Name)) } if len(errs) == 0 { - // Remove the "ingresscontroller.operator.openshift.io/finalizer-ingresscontroller" finalizer - // to allow the ingresscontroller to be deleted. + // Remove the ingresscontroller finalizer. if slice.ContainsString(ingress.Finalizers, manifests.IngressControllerFinalizer) { updated := ingress.DeepCopy() updated.Finalizers = slice.RemoveString(updated.Finalizers, manifests.IngressControllerFinalizer) diff --git a/pkg/operator/controller/ingress/load_balancer_service.go b/pkg/operator/controller/ingress/load_balancer_service.go index 7935881147..20f0b3b64c 100644 --- a/pkg/operator/controller/ingress/load_balancer_service.go +++ b/pkg/operator/controller/ingress/load_balancer_service.go @@ -23,10 +23,6 @@ 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 ( @@ -144,13 +140,13 @@ func (r *reconciler) currentLoadBalancerService(ci *operatorv1.IngressController // 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 { +func (r *reconciler) finalizeLoadBalancerService(ci *operatorv1.IngressController) (bool, error) { service, err := r.currentLoadBalancerService(ci) if err != nil { - return err + return false, err } if service == nil { - return nil + return false, nil } // Mutate a copy to avoid assuming we know where the current one came from // (i.e. it could have been from a cache). @@ -158,14 +154,10 @@ func (r *reconciler) finalizeLoadBalancerService(ci *operatorv1.IngressControlle if slice.ContainsString(updated.Finalizers, manifests.LoadBalancerServiceFinalizer) { updated.Finalizers = slice.RemoveString(updated.Finalizers, manifests.LoadBalancerServiceFinalizer) if err := r.client.Update(context.TODO(), updated); err != nil { - return fmt.Errorf("failed to remove finalizer from service %s for ingress %s/%s: %v", service.Namespace, service.Name, ci.Name, err) + return true, fmt.Errorf("failed to remove finalizer from service %s/%s for ingress %s/%s: %v", + service.Namespace, service.Name, ci.Namespace, 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 + return true, nil }