From 152a2257e6a56546b7959bd208d44c72c8ca590b Mon Sep 17 00:00:00 2001 From: Yossi Boaron Date: Thu, 13 Jul 2023 16:53:22 +0300 Subject: [PATCH] Provision GW LB service in accordance with OCP platform A LoadBalancer type Service is a typical way to expose an application to the internet. A LoadBalancer type Service relies on the cloud provider to create an external load balancer with an IP address in the relevant network space. Different cloud providers support different Service annotations, this PR updates LB service annotations in accordance with OCP platform. Fixes: https://github.com/submariner-io/submariner-operator/issues/2603 Signed-off-by: Yossi Boaron --- .../submariner-operator/ocp_cluster_role.yaml | 6 ++ .../submariner/loadbalancer_resources.go | 70 ++++++++++++++++--- .../submariner/submariner_controller_test.go | 55 +++++++++++++-- .../submariner/submariner_suite_test.go | 2 + go.mod | 3 +- go.sum | 6 ++ main.go | 3 + pkg/embeddedyamls/yamls.go | 6 ++ 8 files changed, 135 insertions(+), 16 deletions(-) diff --git a/config/rbac/submariner-operator/ocp_cluster_role.yaml b/config/rbac/submariner-operator/ocp_cluster_role.yaml index b1842dfb02..6948228cf2 100644 --- a/config/rbac/submariner-operator/ocp_cluster_role.yaml +++ b/config/rbac/submariner-operator/ocp_cluster_role.yaml @@ -12,3 +12,9 @@ rules: - securitycontextconstraints verbs: - use + - apiGroups: + - config.openshift.io + resources: + - infrastructures + verbs: + - get diff --git a/controllers/submariner/loadbalancer_resources.go b/controllers/submariner/loadbalancer_resources.go index 8f02d61b53..7f3490bafb 100644 --- a/controllers/submariner/loadbalancer_resources.go +++ b/controllers/submariner/loadbalancer_resources.go @@ -20,8 +20,11 @@ package submariner import ( "context" + "fmt" + "github.com/pkg/errors" "github.com/go-logr/logr" + configv1 "github.com/openshift/api/config/v1" "github.com/submariner-io/submariner-operator/api/v1alpha1" "github.com/submariner-io/submariner-operator/controllers/apply" "github.com/submariner-io/submariner-operator/pkg/names" @@ -29,6 +32,7 @@ import ( "github.com/submariner-io/submariner/pkg/port" corev1 "k8s.io/api/core/v1" v1meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -43,18 +47,68 @@ const ( func (r *Reconciler) reconcileLoadBalancer( ctx context.Context, instance *v1alpha1.Submariner, reqLogger logr.Logger, ) (*corev1.Service, error) { - return apply.Service(ctx, instance, newLoadBalancerService(instance), reqLogger, r.config.ScopedClient, r.config.Scheme) + var err error + + platformTypeOCP, err := r.getOCPPlatformType(ctx) + if err != nil { + return nil, err + } + + svc := newLoadBalancerService(instance, platformTypeOCP) + svc, err = apply.Service(ctx, instance, svc, reqLogger, r.config.ScopedClient, r.config.Scheme) + + if err != nil { + return nil, err + } + + // For IBM cloud also needs to annotate the allocated health check node port + if platformTypeOCP == string(configv1.IBMCloudPlatformType) { + healthPortStr := fmt.Sprintf("%d", svc.Spec.HealthCheckNodePort) + svc.ObjectMeta.Annotations = map[string]string{ + "service.kubernetes.io/ibm-load-balancer-cloud-provider-vpc-health-check-port": healthPortStr, + } + svc, err = apply.Service(ctx, instance, svc, reqLogger, r.config.ScopedClient, r.config.Scheme) + } + + return svc, err +} + +func (r *Reconciler) getOCPPlatformType(ctx context.Context) (string, error) { + clusterInfra := &configv1.Infrastructure{} + if err := r.config.GeneralClient.Get(ctx, types.NamespacedName{Name: "cluster"}, clusterInfra); err != nil { + return "", errors.Wrap(err, "error retrieving cluster Infrastructure resource") + } + + if clusterInfra.Status.PlatformStatus == nil { + return "", nil + } + + return string(clusterInfra.Status.PlatformStatus.Type), nil } -func newLoadBalancerService(instance *v1alpha1.Submariner) *corev1.Service { +func newLoadBalancerService(instance *v1alpha1.Submariner, platformTypeOCP string) *corev1.Service { + var svcAnnotations map[string]string + + switch platformTypeOCP { + case string(configv1.AWSPlatformType): + svcAnnotations = map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", + } + case string(configv1.IBMCloudPlatformType): + svcAnnotations = map[string]string{ + "service.kubernetes.io/ibm-load-balancer-cloud-provider-enable-features": "nlb", + "service.kubernetes.io/ibm-load-balancer-cloud-provider-ip-type": "public", + "service.kubernetes.io/ibm-load-balancer-cloud-provider-vpc-health-check-protocol": "http", + } + default: + svcAnnotations = map[string]string{} + } + return &corev1.Service{ ObjectMeta: v1meta.ObjectMeta{ - Name: loadBalancerName, - Namespace: instance.Spec.Namespace, - Annotations: map[string]string{ - // AWS requires nlb Load Balancer for UDP - "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", - }, + Name: loadBalancerName, + Namespace: instance.Spec.Namespace, + Annotations: svcAnnotations, }, Spec: corev1.ServiceSpec{ ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go index 1f25b33672..fdf892f0e2 100644 --- a/controllers/submariner/submariner_controller_test.go +++ b/controllers/submariner/submariner_controller_test.go @@ -24,6 +24,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + v1config "github.com/openshift/api/config/v1" "github.com/submariner-io/admiral/pkg/fake" "github.com/submariner-io/submariner-operator/api/v1alpha1" "github.com/submariner-io/submariner-operator/controllers/test" @@ -236,14 +237,31 @@ func testReconciliation() { t.submariner.Spec.LoadBalancerEnabled = true }) - It("should create the load balancer service", func() { - t.AssertReconcileSuccess() + Context("and the Openshift platform type is AWS", func() { + BeforeEach(func() { + t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newInfrastructureCluster(v1config.AWSPlatformType)) + }) - service := &corev1.Service{} - err := t.ScopedClient.Get(context.TODO(), types.NamespacedName{Name: "submariner-gateway", Namespace: submarinerNamespace}, - service) - Expect(err).To(Succeed()) - Expect(service.Spec.Type).To(Equal(corev1.ServiceTypeLoadBalancer)) + It("should create the correct load balancer service", func() { + t.AssertReconcileSuccess() + + service := t.assertLoadBalancerService() + Expect(service.Annotations).To(HaveKeyWithValue("service.beta.kubernetes.io/aws-load-balancer-type", "nlb")) + }) + }) + + Context("and the Openshift platform type is IBMCloud", func() { + BeforeEach(func() { + t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newInfrastructureCluster(v1config.IBMCloudPlatformType)) + }) + + It("should create the correct load balancer service", func() { + t.AssertReconcileSuccess() + + service := t.assertLoadBalancerService() + Expect(service.Annotations).To(HaveKeyWithValue( + "service.kubernetes.io/ibm-load-balancer-cloud-provider-enable-features", "nlb")) + }) }) }) @@ -311,6 +329,16 @@ func testReconciliation() { }) } +func (t *testDriver) assertLoadBalancerService() *corev1.Service { + service := &corev1.Service{} + err := t.ScopedClient.Get(context.TODO(), types.NamespacedName{Name: "submariner-gateway", Namespace: submarinerNamespace}, + service) + Expect(err).To(Succeed()) + Expect(service.Spec.Type).To(Equal(corev1.ServiceTypeLoadBalancer)) + + return service +} + func testDeletion() { t := newTestDriver() @@ -511,3 +539,16 @@ func testDeletion() { }) }) } + +func newInfrastructureCluster(platformType v1config.PlatformType) *v1config.Infrastructure { + return &v1config.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: v1config.InfrastructureStatus{ + PlatformStatus: &v1config.PlatformStatus{ + Type: platformType, + }, + }, + } +} diff --git a/controllers/submariner/submariner_suite_test.go b/controllers/submariner/submariner_suite_test.go index 8a3e7bc39d..fe4f6e08c3 100644 --- a/controllers/submariner/submariner_suite_test.go +++ b/controllers/submariner/submariner_suite_test.go @@ -26,6 +26,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" "github.com/submariner-io/admiral/pkg/log/kzerolog" "github.com/submariner-io/admiral/pkg/syncer/broker" "github.com/submariner-io/submariner-operator/api/v1alpha1" @@ -46,6 +47,7 @@ var _ = BeforeSuite(func() { Expect(v1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) Expect(apiextensions.AddToScheme(scheme.Scheme)).To(Succeed()) Expect(submarinerv1.AddToScheme(scheme.Scheme)).To(Succeed()) + Expect(configv1.Install(scheme.Scheme)).To(Succeed()) }) var _ = Describe("", func() { diff --git a/go.mod b/go.mod index 04517e9636..fb2dda20c1 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/go-logr/logr v1.2.4 github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.8 - github.com/openshift/api v0.0.0-20211201215911-5a82bae32e46 + github.com/openshift/api v0.0.0-20230714214528-de6ad7979b00 github.com/operator-framework/operator-lib v0.11.1-0.20230306195046-28cadc6b6055 github.com/operator-framework/operator-sdk v1.30.0 github.com/pkg/errors v0.9.1 @@ -155,6 +155,7 @@ require ( github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b // indirect + github.com/openshift/client-go v0.0.0-20230705133330-7f808ad59404 // indirect github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 // indirect github.com/operator-framework/helm-operator-plugins v0.0.12-0.20230413193425-4632388adc61 // indirect github.com/operator-framework/java-operator-plugins v0.7.1-0.20230306190439-0eed476d2b75 // indirect diff --git a/go.sum b/go.sum index 2a6e4d6f85..496160f64d 100644 --- a/go.sum +++ b/go.sum @@ -812,7 +812,13 @@ github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b/go github.com/opencontainers/runtime-spec v1.1.0-rc.1 h1:wHa9jroFfKGQqFHj0I1fMRKLl0pfj+ynAqBxo3v6u9w= github.com/openshift/api v0.0.0-20211201215911-5a82bae32e46 h1:/EZ9QZVWoh4ygjZOGvw88yef3UjDNm0+JkGAy4ddJ+Q= github.com/openshift/api v0.0.0-20211201215911-5a82bae32e46/go.mod h1:RsQCVJu4qhUawxxDP7pGlwU3IA4F01wYm3qKEu29Su8= +github.com/openshift/api v0.0.0-20230703134140-1c2204a0195c h1:b1aPWPgW3MyK4BEKFjKBIHTV2LrdwGwErITEUw//xl4= +github.com/openshift/api v0.0.0-20230703134140-1c2204a0195c/go.mod h1:4VWG+W22wrB4HfBL88P40DxLEpSOaiBVxUnfalfJo9k= +github.com/openshift/api v0.0.0-20230714214528-de6ad7979b00 h1:sYXq/GVWN0Un+6eEGd3vft4dY+M3i0RSL3GJhvQBOGY= +github.com/openshift/api v0.0.0-20230714214528-de6ad7979b00/go.mod h1:yimSGmjsI+XF1mr+AKBs2//fSXIOhhetHGbMlBEfXbs= github.com/openshift/build-machinery-go v0.0.0-20210712174854-1bb7fd1518d3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= +github.com/openshift/client-go v0.0.0-20230705133330-7f808ad59404 h1:7Q/RkeK4UHpB25nu7z03tSFlWgWjcnAUbjwI1Ud22H4= +github.com/openshift/client-go v0.0.0-20230705133330-7f808ad59404/go.mod h1:8Hq3t7Ba02Z0sjDGtTCARPXylxbOyzFrbwiqb1ViWMA= github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 h1:d/Pnr19TnmIq3zQ6ebewC+5jt5zqYbRkvYd37YZENQY= github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42/go.mod h1:l/cuwtPxkVUY7fzYgdust2m9tlmb8I4pOvbsUufRb24= github.com/operator-framework/helm-operator-plugins v0.0.12-0.20230413193425-4632388adc61 h1:FPO2hS4HNIU2pzWeX2KusKxqDFeGIURRMkxRtn/i570= diff --git a/main.go b/main.go index 041dbc4301..c0d40e2797 100644 --- a/main.go +++ b/main.go @@ -25,6 +25,7 @@ import ( "os" "runtime" + configv1 "github.com/openshift/api/config/v1" "github.com/operator-framework/operator-lib/leader" "github.com/submariner-io/admiral/pkg/log/kzerolog" "github.com/submariner-io/submariner-operator/api/v1alpha1" @@ -144,6 +145,8 @@ func main() { utilruntime.Must(apiextensions.AddToScheme(scheme)) // These are required so that we can retrieve Gateway objects using the dynamic client utilruntime.Must(submv1.AddToScheme(scheme)) + // These are required so that we can retrieve OCP infrastructure objects using the dynamic client + utilruntime.Must(configv1.Install(scheme)) // +kubebuilder:scaffold:scheme // Create a new Cmd to provide shared dependencies and start components diff --git a/pkg/embeddedyamls/yamls.go b/pkg/embeddedyamls/yamls.go index 2bc6fa9908..0ebf6e4111 100644 --- a/pkg/embeddedyamls/yamls.go +++ b/pkg/embeddedyamls/yamls.go @@ -2674,6 +2674,12 @@ rules: - securitycontextconstraints verbs: - use + - apiGroups: + - config.openshift.io + resources: + - infrastructures + verbs: + - get ` Config_rbac_submariner_operator_ocp_cluster_role_binding_yaml = `--- apiVersion: rbac.authorization.k8s.io/v1