diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile.go index 25cdf1afc0..81e4abaa4e 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile.go @@ -10,7 +10,6 @@ import ( routev1 "github.com/openshift/api/route/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -89,11 +88,9 @@ func ReconcileDefaultIngressController(ingressController *operatorv1.IngressCont Scope: loadBalancerScope, ProviderParameters: &operatorv1.ProviderLoadBalancerParameters{ Type: operatorv1.OpenStackLoadBalancerProvider, - // TODO(emilien): add the field once bumped openshift/api and also remove `ReconcileDefaultIngressControllerWithUnstructured`. - // https://github.com/openshift/hypershift/pull/4927 - // OpenStack: &operatorv1.OpenStackLoadBalancerParameters{ - // FloatingIP: loadBalancerIP, - // }, + OpenStack: &operatorv1.OpenStackLoadBalancerParameters{ + FloatingIP: loadBalancerIP, + }, }, }, } @@ -117,32 +114,6 @@ func ReconcileDefaultIngressController(ingressController *operatorv1.IngressCont return nil } -// ReconcileDefaultIngressControllerWithUnstructured reconciles the default ingress controller with an unstructured object -// that has custom fields set per platform. -func ReconcileOpenStackDefaultIngressController(ingressController *unstructured.Unstructured, ingressSubdomain string, replicas int32, isPrivate bool, loadBalancerScope operatorv1.LoadBalancerScope, loadBalancerIP string) error { - // If ingress controller already exists, skip reconciliation to allow day-2 configuration - if ingressController.GetResourceVersion() != "" { - return nil - } - - unstructured.SetNestedField(ingressController.Object, ingressSubdomain, "spec", "domain") - unstructured.SetNestedField(ingressController.Object, string(operatorv1.LoadBalancerServiceStrategyType), "spec", "endpointPublishingStrategy", "type") - unstructured.SetNestedField(ingressController.Object, int64(replicas), "spec", "replicas") - - unstructured.SetNestedField(ingressController.Object, string(loadBalancerScope), "spec", "endpointPublishingStrategy", "loadBalancer", "scope") - unstructured.SetNestedField(ingressController.Object, string(operatorv1.OpenStackLoadBalancerProvider), "spec", "endpointPublishingStrategy", "loadBalancer", "providerParameters", "type") - if loadBalancerIP != "" { - unstructured.SetNestedField(ingressController.Object, loadBalancerIP, "spec", "endpointPublishingStrategy", "loadBalancer", "providerParameters", "openstack", "floatingIP") - } - unstructured.SetNestedField(ingressController.Object, manifests.IngressDefaultIngressControllerCert().Name, "spec", "defaultCertificate", "name") - - if isPrivate { - unstructured.SetNestedField(ingressController.Object, operatorv1.PrivateStrategyType, "spec", "endpointPublishingStrategy", "type") - unstructured.SetNestedMap(ingressController.Object, map[string]interface{}{}, "spec", "endpointPublishingStrategy", "private") - } - return nil -} - func ReconcileDefaultIngressControllerCertSecret(certSecret *corev1.Secret, sourceSecret *corev1.Secret) error { if _, hasCertKey := sourceSecret.Data[corev1.TLSCertKey]; !hasCertKey { return fmt.Errorf("source secret %s/%s does not have a cert key", sourceSecret.Namespace, sourceSecret.Name) diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile_test.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile_test.go index 450b9f80c9..ed554d0f35 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile_test.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile_test.go @@ -12,11 +12,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/utils/ptr" - - "github.com/google/go-cmp/cmp" ) func TestReconcileDefaultIngressController(t *testing.T) { @@ -293,6 +288,37 @@ func TestReconcileDefaultIngressController(t *testing.T) { }, }, }, + { + name: "OpenStack uses Loadbalancer publishing strategy with a floating IP", + inputIngressController: manifests.IngressDefaultIngressController(), + inputIngressDomain: fakeIngressDomain, + inputPlatformType: hyperv1.OpenStackPlatform, + inputReplicas: fakeInputReplicas, + inputIsIBMCloudUPI: false, + inputIsPrivate: false, + inputLoadBalancerIP: "1.2.3.4", + expectedIngressController: &operatorv1.IngressController{ + ObjectMeta: manifests.IngressDefaultIngressController().ObjectMeta, + Spec: operatorv1.IngressControllerSpec{ + Domain: fakeIngressDomain, + Replicas: &fakeInputReplicas, + EndpointPublishingStrategy: &operatorv1.EndpointPublishingStrategy{ + Type: operatorv1.LoadBalancerServiceStrategyType, + LoadBalancer: &operatorv1.LoadBalancerStrategy{ + ProviderParameters: &operatorv1.ProviderLoadBalancerParameters{ + Type: operatorv1.OpenStackLoadBalancerProvider, + OpenStack: &operatorv1.OpenStackLoadBalancerParameters{ + FloatingIP: "1.2.3.4", + }, + }, + }, + }, + DefaultCertificate: &corev1.LocalObjectReference{ + Name: manifests.IngressDefaultIngressControllerCert().Name, + }, + }, + }, + }, } for _, tc := range testsCases { t.Run(tc.name, func(t *testing.T) { @@ -303,44 +329,3 @@ func TestReconcileDefaultIngressController(t *testing.T) { }) } } - -func TestReconcileOpenStackDefaultIngressController(t *testing.T) { - ingressController := manifests.IngressDefaultIngressControllerAsUnstructured() - if err := ReconcileOpenStackDefaultIngressController(ingressController, "example.com", 1, false, operatorv1.ExternalLoadBalancer, "1.2.3.4"); err != nil { - t.Errorf("unexpected: %v", err) - } - typedIngressController := &operatorv1.IngressController{} - expected := &operatorv1.IngressController{ - TypeMeta: metav1.TypeMeta{ - Kind: "IngressController", - APIVersion: operatorv1.GroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: manifests.IngressDefaultIngressController().Name, - Namespace: manifests.IngressDefaultIngressController().Namespace, - }, - Spec: operatorv1.IngressControllerSpec{ - Domain: "example.com", - Replicas: ptr.To[int32](1), - EndpointPublishingStrategy: &operatorv1.EndpointPublishingStrategy{ - Type: operatorv1.LoadBalancerServiceStrategyType, - LoadBalancer: &operatorv1.LoadBalancerStrategy{ - Scope: operatorv1.ExternalLoadBalancer, - }, - }, - DefaultCertificate: &corev1.LocalObjectReference{ - Name: "default-ingress-cert", - }, - }, - } - - // Before converting, remove extra field from provider parameters, since it does not exist in type yet - unstructured.RemoveNestedField(ingressController.Object, "spec", "endpointPublishingStrategy", "loadBalancer", "providerParameters") - - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(ingressController.Object, typedIngressController); err != nil { - t.Errorf("unexpected: %v", err) - } - if diff := cmp.Diff(typedIngressController, expected); diff != "" { - t.Errorf("IngressController does not match expected:\n%s", diff) - } -} diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/ingress.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/ingress.go index c41d9d2068..11969db36d 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/ingress.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/ingress.go @@ -6,7 +6,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func IngressDefaultIngressController() *operatorv1.IngressController { @@ -18,16 +17,6 @@ func IngressDefaultIngressController() *operatorv1.IngressController { } } -func IngressDefaultIngressControllerAsUnstructured() *unstructured.Unstructured { - src := IngressDefaultIngressController() - obj := &unstructured.Unstructured{} - obj.SetAPIVersion(operatorv1.GroupVersion.String()) - obj.SetKind("IngressController") - obj.SetName(src.Name) - obj.SetNamespace(src.Namespace) - return obj -} - func IngressDefaultIngressControllerCert() *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go index cd4812fe3d..9ccd6e3d0e 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go @@ -1005,23 +1005,11 @@ func (r *reconciler) reconcileRBAC(ctx context.Context) error { func (r *reconciler) reconcileIngressController(ctx context.Context, hcp *hyperv1.HostedControlPlane) error { var errs []error p := ingress.NewIngressParams(hcp) - - // This is a workaround until we bump openshift/api which has the floatingIP field in the IngessControllerSpec - // TODO (cewong): Remove this when updated openshift/api is vendored - if hcp.Spec.Platform.Type == hyperv1.OpenStackPlatform { - ingressController := manifests.IngressDefaultIngressControllerAsUnstructured() - if _, err := r.CreateOrUpdate(ctx, r.client, ingressController, func() error { - return ingress.ReconcileOpenStackDefaultIngressController(ingressController, p.IngressSubdomain, p.Replicas, p.IsPrivate, p.LoadBalancerScope, p.LoadBalancerIP) - }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile openstack default ingress controller: %w", err)) - } - } else { - ingressController := manifests.IngressDefaultIngressController() - if _, err := r.CreateOrUpdate(ctx, r.client, ingressController, func() error { - return ingress.ReconcileDefaultIngressController(ingressController, p.IngressSubdomain, p.PlatformType, p.Replicas, p.IBMCloudUPI, p.IsPrivate, p.AWSNLB, p.LoadBalancerScope, p.LoadBalancerIP) - }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile default ingress controller: %w", err)) - } + ingressController := manifests.IngressDefaultIngressController() + if _, err := r.CreateOrUpdate(ctx, r.client, ingressController, func() error { + return ingress.ReconcileDefaultIngressController(ingressController, p.IngressSubdomain, p.PlatformType, p.Replicas, p.IBMCloudUPI, p.IsPrivate, p.AWSNLB, p.LoadBalancerScope, p.LoadBalancerIP) + }); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile default ingress controller: %w", err)) } sourceCert := cpomanifests.IngressCert(hcp.Namespace)