From 715955cb82a8150d7ab28ef964f602446c0ad18b 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 | 77 +++++++++++++++++-- .../submariner/submariner_controller_test.go | 56 ++++++++++++-- .../submariner/submariner_suite_test.go | 2 + go.mod | 3 +- go.sum | 6 ++ main.go | 7 +- pkg/embeddedyamls/yamls.go | 6 ++ 8 files changed, 148 insertions(+), 15 deletions(-) diff --git a/config/rbac/submariner-operator/ocp_cluster_role.yaml b/config/rbac/submariner-operator/ocp_cluster_role.yaml index b1842dfb0..6948228cf 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 8f02d61b5..2097e4aad 100644 --- a/controllers/submariner/loadbalancer_resources.go +++ b/controllers/submariner/loadbalancer_resources.go @@ -20,15 +20,20 @@ package submariner import ( "context" + "fmt" "github.com/go-logr/logr" + configv1 "github.com/openshift/api/config/v1" + "github.com/pkg/errors" "github.com/submariner-io/submariner-operator/api/v1alpha1" "github.com/submariner-io/submariner-operator/controllers/apply" "github.com/submariner-io/submariner-operator/pkg/names" submv1 "github.com/submariner-io/submariner/pkg/apis/submariner.io/v1" "github.com/submariner-io/submariner/pkg/port" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -43,18 +48,74 @@ 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 newLoadBalancerService(instance *v1alpha1.Submariner) *corev1.Service { +func (r *Reconciler) getOCPPlatformType(ctx context.Context) (string, error) { + clusterInfra := &configv1.Infrastructure{} + err := r.config.GeneralClient.Get(ctx, types.NamespacedName{Name: "cluster"}, clusterInfra) + + if apierrors.IsNotFound(err) { + return "", nil + } + + if err != nil { + return "", errors.Wrap(err, "error retrieving cluster Infrastructure resource") + } + + if clusterInfra.Status.PlatformStatus == nil { + return string(clusterInfra.Status.Platform), nil //nolint:staticcheck //Purposely using deprecated field for backwards compatibility + } + + return string(clusterInfra.Status.PlatformStatus.Type), nil +} + +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 1f25b3367..d71ad665b 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" @@ -238,12 +239,34 @@ func testReconciliation() { It("should create the load balancer service", func() { t.AssertReconcileSuccess() + t.assertLoadBalancerService() + }) - 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)) + Context("and the Openshift platform type is AWS", func() { + BeforeEach(func() { + t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newInfrastructureCluster(v1config.AWSPlatformType)) + }) + + 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 +334,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 +544,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 8a3e7bc39..fe4f6e08c 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 de2e313d3..e82530e44 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 293e57fca..68b7cbf6a 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 041dbc430..f9e4f4f79 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 @@ -190,7 +193,9 @@ func main() { os.Exit(1) } - generalClient, _ := client.New(mgr.GetConfig(), client.Options{}) + generalClient, _ := client.New(mgr.GetConfig(), client.Options{ + Scheme: scheme, + }) if err = submariner.NewReconciler(&submariner.Config{ ScopedClient: mgr.GetClient(), diff --git a/pkg/embeddedyamls/yamls.go b/pkg/embeddedyamls/yamls.go index 2662c9f6e..cd98b72fd 100644 --- a/pkg/embeddedyamls/yamls.go +++ b/pkg/embeddedyamls/yamls.go @@ -2675,6 +2675,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