From 6c885c17fae209435f3dc41a8009aa8da97341f8 Mon Sep 17 00:00:00 2001 From: Dustin Row Date: Tue, 20 Oct 2020 15:47:10 -0700 Subject: [PATCH] Modify reconcile of ingresscontroller to support an update to an existing ingresscontroller for mutable lb scope. --- ...20_cloud-ingress-operator.ClusterRole.yaml | 1 + .../publishingstrategy_controller.go | 199 ++++++------------ .../publishingstrategy_controller_test.go | 12 +- 3 files changed, 66 insertions(+), 146 deletions(-) diff --git a/deploy/20_cloud-ingress-operator.ClusterRole.yaml b/deploy/20_cloud-ingress-operator.ClusterRole.yaml index 00315f94..1eaf4410 100644 --- a/deploy/20_cloud-ingress-operator.ClusterRole.yaml +++ b/deploy/20_cloud-ingress-operator.ClusterRole.yaml @@ -26,6 +26,7 @@ rules: - watch - delete - create + - update - apiGroups: - config.openshift.io resources: diff --git a/pkg/controller/publishingstrategy/publishingstrategy_controller.go b/pkg/controller/publishingstrategy/publishingstrategy_controller.go index ced97f14..f4a50e96 100644 --- a/pkg/controller/publishingstrategy/publishingstrategy_controller.go +++ b/pkg/controller/publishingstrategy/publishingstrategy_controller.go @@ -18,6 +18,7 @@ import ( utils "github.com/openshift/cloud-ingress-operator/pkg/controller/utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -25,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" + logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -119,49 +121,21 @@ func (r *ReconcilePublishingStrategy) Reconcile(request reconcile.Request) (reco return reconcile.Result{}, err } - // delete non-default with proper annotation if not publishingStratagy CR - err = r.deleteIngressWithAnnotation(instance.Spec.ApplicationIngress, ingressControllerList) - if err != nil { - log.Error(err, "Cannot delete ingresscontroller with annotation") - return reconcile.Result{}, err - } - - // create list of applicationIngress - var ingressNotOnCluster []cloudingressv1alpha1.ApplicationIngress - - exisitingIngressMap := convertIngressControllerToMap(ingressControllerList.Items) - - // loop through every applicationingress in publishing strategy - for _, publishingStrategyIngress := range instance.Spec.ApplicationIngress { - if !checkExistingIngress(exisitingIngressMap, &publishingStrategyIngress) { - ingressNotOnCluster = append(ingressNotOnCluster, publishingStrategyIngress) - } - } - // ----------------------------TODO: remove these debug logs afterwards -------------------------------------- var serializedIngressControllerList bytes.Buffer _ = serializer.Encode(ingressControllerList, &serializedIngressControllerList) log.Info(fmt.Sprintf("initial ingresscontroller list: %s", serializedIngressControllerList.String())) log.Info(fmt.Sprintf("appingress on publishingstrategy CR: %+v", instance.Spec.ApplicationIngress)) - log.Info(fmt.Sprintf("ingress to reconcile: %+v", ingressNotOnCluster)) // ----------------------------------------------------------------------------------------------------------- - for _, appingress := range ingressNotOnCluster { + for _, appingress := range instance.Spec.ApplicationIngress { newCertificate := &corev1.LocalObjectReference{ Name: appingress.Certificate.Name, } - if appingress.Default == true { - err := r.defaultIngressHandle(appingress, ingressControllerList, newCertificate) - if err != nil { - log.Error(err, fmt.Sprintf("failed to handle default ingresscontroller %v", appingress)) - return reconcile.Result{}, err - } - continue - } - err := r.nonDefaultIngressHandle(appingress, ingressControllerList, newCertificate) + err := r.createOrUpdateIngressController(appingress, newCertificate) if err != nil { - log.Error(err, fmt.Sprintf("failed to handle non-default ingresscontroller %v", appingress)) + log.Error(err, fmt.Sprintf("failed to create or update ingresscontroller %v", appingress)) } } @@ -415,35 +389,6 @@ func validateIngress(ingressController operatorv1.IngressController) bool { return true } -// get a list of all ingress on cluster that has annotation owner cloud-ingress-operator -// and delete all non-default ingresses -func (r *ReconcilePublishingStrategy) deleteIngressWithAnnotation(appIngressList []cloudingressv1alpha1.ApplicationIngress, ingressControllerList *operatorv1.IngressControllerList) error { - for _, ingress := range ingressControllerList.Items { - // if ingress does not have correct annotations, continue to next one - if _, ok := ingress.Annotations["Owner"]; !ok { - continue - } - // if ingress is default, skip it since we are only looking at non-default ingresses - if ingress.Name == "default" { - continue - } - // only delete the ingress that does not exist on publishingStrategy - // true means the ingress (on cluster) exists on the publishingStrategy CR as well, so no deletion - // false means the ingress (on cluster) do not exist on the CR, so delete to have desired result - if !contains(appIngressList, &ingress) { - err := r.client.Delete(context.TODO(), &ingress) - if err != nil { - log.Error(err, "Failed to delete ingresscontroller") - return err - } - // wait 60 seconds for deletion to be completed - log.Info("waited 60 seconds for necessary ingresscontroller deletions") - time.Sleep(time.Duration(60) * time.Second) - } - } - return nil -} - // contains check if an individual non-default ingress on cluster matches with any non-default applicationingress // return true if ingress (on cluster) matches with any applicationIngress in PublishingStrategy CR // return false if ingress (on cluster) DOES NOT match with ANY applicationIngress in PublishingStrategy CR @@ -469,87 +414,45 @@ func contains(appIngressList []cloudingressv1alpha1.ApplicationIngress, ingressC return isContained } -// defaultIngressHandle will delete the existing default ingresscontroller, and create a new one with fields from publishingstrategySpec.ApplicationIngress -func (r *ReconcilePublishingStrategy) defaultIngressHandle(appingress cloudingressv1alpha1.ApplicationIngress, ingressControllerList *operatorv1.IngressControllerList, newCertificate *corev1.LocalObjectReference) error { - // delete the default appingress on cluster - for _, ingresscontroller := range ingressControllerList.Items { - if ingresscontroller.Name == defaultIngressName { - err := r.client.Delete(context.TODO(), &ingresscontroller) - if err != nil { - log.Error(err, "failed to delete existing ingresscontroller") - return err - } - } - } - newDefaultIngressController, err := newApplicationIngressControllerCR(defaultIngressName, string(appingress.Listening), appingress.DNSName, newCertificate, appingress.RouteSelector.MatchLabels) +// createOrUpdateIngressController will create or update the given ingresscontroller with fields from publishingstrategySpec.ApplicationIngress +func (r *ReconcilePublishingStrategy) createOrUpdateIngressController(appingress cloudingressv1alpha1.ApplicationIngress, newCertificate *corev1.LocalObjectReference) error { + ingressControllerName := defaultIngressName + // get name from the domain + if appingress.Default != true { + ingressControllerName = getIngressName(appingress.DNSName) + } + // try to get the ingresscontroller + ingressController := &operatorv1.IngressController{} + err := r.client.Get(context.TODO(), types.NamespacedName{ + Namespace: ingressControllerNamespace, + Name: ingressControllerName, + }, ingressController) + + // ingress controller doesn't exist, so create it if err != nil { - log.Error(err, fmt.Sprintf("failed to generate information for default ingresscontroller with domain %s", appingress.DNSName)) - return err - } - err = r.client.Create(context.TODO(), newDefaultIngressController) - if err != nil { - if k8serr.IsAlreadyExists(err) { - for i := 0; i < 60; i++ { - if i == 60 { - log.Error(err, "out of retries") - return err - } - time.Sleep(time.Duration(1) * time.Second) - - err = r.client.Create(context.TODO(), newDefaultIngressController) - if err != nil { - continue - } - // if err not nil then successful - log.Info(fmt.Sprintf("successfully created default ingresscontroller for %s", newDefaultIngressController.Spec.Domain)) - break - } - } else { - log.Error(err, fmt.Sprintf("failed to create new ingresscontroller with domain %s", appingress.DNSName)) + newIngressController, err := newApplicationIngressControllerCR(ingressControllerName, string(appingress.Listening), appingress.DNSName, newCertificate, appingress.RouteSelector.MatchLabels) + if err != nil { + log.Error(err, fmt.Sprintf("failed to generate information for default ingresscontroller with domain %s", appingress.DNSName)) return err } - } - return nil -} - -// nonDefaultIngressHandle will delete the existing non-default ingresscontroller, and create a new one with fields from publishingstrategySpec.ApplicationIngress -func (r *ReconcilePublishingStrategy) nonDefaultIngressHandle(appingress cloudingressv1alpha1.ApplicationIngress, ingressControllerList *operatorv1.IngressControllerList, newCertificate *corev1.LocalObjectReference) error { - newIngressControllerName := getIngressName(appingress.DNSName) - // if ingress with same name exists on cluster then delete - for _, ingresscontroller := range ingressControllerList.Items { - if ingresscontroller.Name == newIngressControllerName { - err := r.client.Delete(context.TODO(), &ingresscontroller) - if err != nil { - log.Error(err, "failed to delete existing ingresscontroller") - return err - } + err = r.client.Create(context.TODO(), newIngressController) + if err != nil { + log.Error(err, fmt.Sprintf("got error trying to create %s", ingressControllerName)) + return err } - } - - // create the ingress - newIngressController, err := newApplicationIngressControllerCR(newIngressControllerName, string(appingress.Listening), appingress.DNSName, newCertificate, appingress.RouteSelector.MatchLabels) - if err != nil { - log.Error(err, fmt.Sprintf("failed to generate information for ingresscontroller with domain %s", appingress.DNSName)) - } - err = r.client.Create(context.TODO(), newIngressController) - if err != nil { - if k8serr.IsAlreadyExists(err) { - for i := 1; i < 24; i++ { - if i == 24 { - log.Error(err, "out of retries to create non-default ingress") - } - time.Sleep(time.Duration(i) * time.Second) - err = r.client.Create(context.TODO(), newIngressController) - if err != nil { - continue - } - log.Info(fmt.Sprintf("successfully created non-default ingresscontroller for %s", newIngressController.Spec.Domain)) - break - } - } else { - log.Error(err, fmt.Sprintf("got error trying to create %s", newIngressController.GetName())) + log.Info(fmt.Sprintf("created %s ingresscontroller successfully", ingressControllerName)) + } else { + err = modifyApplicationIngressControllerCR(ingressController, string(appingress.Listening), appingress.DNSName, newCertificate, appingress.RouteSelector.MatchLabels) + if err != nil { + log.Error(err, fmt.Sprintf("got error trying to modify %s", ingressControllerName)) + return err + } + err = r.client.Update(context.TODO(), ingressController) + if err != nil { + log.Error(err, fmt.Sprintf("got error trying to update %s", ingressControllerName)) return err } + log.Info(fmt.Sprintf("updated %s ingresscontroller successfully", ingressControllerName)) } return nil } @@ -597,6 +500,32 @@ func newApplicationIngressControllerCR(ingressControllerCRName, scope, dnsName s }, nil } +// modifyApplicationIngressControllerCR modifies an existing IngressController CR +func modifyApplicationIngressControllerCR(existing *operatorv1.IngressController, scope, dnsName string, certificate *corev1.LocalObjectReference, matchLabels map[string]string) error { + loadBalancerScope := operatorv1.LoadBalancerScope("") + switch scope { + case "internal": + loadBalancerScope = operatorv1.InternalLoadBalancer + case "external": + loadBalancerScope = operatorv1.ExternalLoadBalancer + default: + return errors.New("ErrUpdatingIngressController") + } + + existing.Spec.DefaultCertificate = certificate + existing.Spec.Domain = dnsName + existing.Spec.EndpointPublishingStrategy = &operatorv1.EndpointPublishingStrategy{ + Type: operatorv1.LoadBalancerServiceStrategyType, + LoadBalancer: &operatorv1.LoadBalancerStrategy{ + Scope: loadBalancerScope, + }, + } + existing.Spec.RouteSelector = &metav1.LabelSelector{ + MatchLabels: matchLabels, + } + return nil +} + // convertIngressControllerToMap takes in on cluster ingresscontroller list and returns them as a map with key Spec.Domain and value operatorv1.IngressController func convertIngressControllerToMap(existingIngress []operatorv1.IngressController) map[string]operatorv1.IngressController { ingressMap := make(map[string]operatorv1.IngressController) diff --git a/pkg/controller/publishingstrategy/publishingstrategy_controller_test.go b/pkg/controller/publishingstrategy/publishingstrategy_controller_test.go index 34cfc312..8f7ccd54 100644 --- a/pkg/controller/publishingstrategy/publishingstrategy_controller_test.go +++ b/pkg/controller/publishingstrategy/publishingstrategy_controller_test.go @@ -375,15 +375,10 @@ func TestIngressHandle(t *testing.T) { Name: "new-certificate", } - err = r.defaultIngressHandle(mockPublishingStrategy().Spec.ApplicationIngress[0], list, newCertificate) + err = r.createOrUpdateIngressController(mockPublishingStrategy().Spec.ApplicationIngress[0], newCertificate) if err != nil { t.Fatalf("couldn't handle default ingress") } - - err = r.nonDefaultIngressHandle(mockPublishingStrategy().Spec.ApplicationIngress[1], list, newCertificate) - if err != nil { - t.Fatalf("couldn't handle non-default ingress") - } } // TestDeleteIngressWithAnnotation tests the deleteIngressWithAnnotation @@ -423,11 +418,6 @@ func TestDeleteIngressWithAnnotation(t *testing.T) { if err != nil { t.Errorf("couldn't get ingresscontroller list %s", err) } - // if ingress without annotation hit method, then it should not be removed - err = r.deleteIngressWithAnnotation(mockPublishingStrategy().Spec.ApplicationIngress, ingressControllerList) - if err != nil { - t.Fatalf("couldn't delete ingress") - } err = r.client.List(ctx, ingressControllerList, &opts) if err != nil {