Skip to content

Commit

Permalink
Provision GW LB service in accordance with OCP platform
Browse files Browse the repository at this point in the history
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: submariner-io#2603

Signed-off-by: Yossi Boaron <[email protected]>
  • Loading branch information
yboaron authored and tpantelis committed Jul 19, 2023
1 parent edf6e41 commit 5d51dea
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 15 deletions.
6 changes: 6 additions & 0 deletions config/rbac/submariner-operator/ocp_cluster_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,9 @@ rules:
- securitycontextconstraints
verbs:
- use
- apiGroups:
- config.openshift.io
resources:
- infrastructures
verbs:
- get
77 changes: 69 additions & 8 deletions controllers/submariner/loadbalancer_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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 "", nil
}

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,
Expand Down
56 changes: 51 additions & 5 deletions controllers/submariner/submariner_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"))
})
})
})

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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,
},
},
}
}
2 changes: 2 additions & 0 deletions controllers/submariner/submariner_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
7 changes: 6 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
6 changes: 6 additions & 0 deletions pkg/embeddedyamls/yamls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5d51dea

Please sign in to comment.