Skip to content

Commit

Permalink
BGPBackend: make frr-k8s the default backend in openshift
Browse files Browse the repository at this point in the history 
We want to have frr-k8s activated by default when the platform is
openshift.

Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
  • Loading branch information
@fedepaol
fedepaol committed Jul 1, 2024
1 parent ad91938 commit 613974a
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 63 deletions.
18 changes: 9 additions & 9 deletions api/v1beta1/metallb_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package v1beta1

import (
"github.com/metallb/metallb-operator/pkg/params"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand All @@ -37,6 +36,14 @@ const (
LogLevelNone MetalLBLogLevel = "none"
)

const (
FRRMode BGPType = "frr"
NativeMode BGPType = "native"
FRRK8sMode BGPType = "frr-k8s"
)

type BGPType string

// MetalLBSpec defines the desired state of MetalLB
type MetalLBSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
Expand Down Expand Up @@ -87,7 +94,7 @@ type MetalLBSpec struct {

// The type of BGP implementation deployed with MetalLB
// +optional
BGPBackend params.BGPType `json:"bgpBackend,omitempty"`
BGPBackend BGPType `json:"bgpBackend,omitempty"`

// The specific frr-k8s configuration
FRRK8SConfig *FRRK8SConfig `json:"frrk8sConfig,omitempty"`
Expand Down Expand Up @@ -142,13 +149,6 @@ type MetalLB struct {
Status MetalLBStatus `json:"status,omitempty"`
}

func (m *MetalLB) BGPBackend() params.BGPType {
if m.Spec.BGPBackend == "" {
return params.FRRMode
}
return m.Spec.BGPBackend
}

// +kubebuilder:object:root=true

// MetalLBList contains a list of MetalLB
Expand Down
9 changes: 4 additions & 5 deletions api/v1beta1/metallb_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"net"
"strings"

"github.com/metallb/metallb-operator/pkg/params"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -108,13 +107,13 @@ func (metallb *MetalLB) Validate() error {
}

if metallb.Spec.BGPBackend != "" &&
metallb.Spec.BGPBackend != params.NativeMode &&
metallb.Spec.BGPBackend != params.FRRK8sMode &&
metallb.Spec.BGPBackend != params.FRRMode {
metallb.Spec.BGPBackend != NativeMode &&
metallb.Spec.BGPBackend != FRRK8sMode &&
metallb.Spec.BGPBackend != FRRMode {
return errors.New("Invalid BGP Backend, must be one of native, frr, frr-k8s")
}

if metallb.Spec.BGPBackend != params.FRRK8sMode &&
if metallb.Spec.BGPBackend != FRRK8sMode &&
metallb.Spec.FRRK8SConfig != nil {
return fmt.Errorf("can't apply frrk8s config while running in %s mode", metallb.Spec.BGPBackend)
}
Expand Down
10 changes: 5 additions & 5 deletions controllers/metallb_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (r *MetalLBReconciler) syncMetalLBResources(config *metallbv1beta1.MetalLB)
return err
}

if config.BGPBackend() == params.FRRK8sMode {
if params.BGPType(config, r.EnvConfig.IsOpenshift) == metallbv1beta1.FRRK8sMode {
objs = append(objs, frrk8sObjs...)
} else {
toDel = append(toDel, frrk8sObjs...)
Expand Down Expand Up @@ -217,12 +217,12 @@ func (r *MetalLBReconciler) syncMetalLBResources(config *metallbv1beta1.MetalLB)

func validateBGPMode(config *metallbv1beta1.MetalLB, isOpenshift bool) error {
if config.Spec.BGPBackend != "" &&
config.Spec.BGPBackend != params.NativeMode &&
config.Spec.BGPBackend != params.FRRK8sMode &&
config.Spec.BGPBackend != params.FRRMode {
config.Spec.BGPBackend != metallbv1beta1.NativeMode &&
config.Spec.BGPBackend != metallbv1beta1.FRRK8sMode &&
config.Spec.BGPBackend != metallbv1beta1.FRRMode {
return fmt.Errorf("unsupported bgp backend %s", config.Spec.BGPBackend)
}
if isOpenshift && config.Spec.BGPBackend == params.NativeMode {
if isOpenshift && config.Spec.BGPBackend == metallbv1beta1.NativeMode {
return fmt.Errorf("native mode is not supported on openshift")
}
return nil
Expand Down
37 changes: 18 additions & 19 deletions controllers/metallb_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"time"

metallbv1beta1 "github.com/metallb/metallb-operator/api/v1beta1"
"github.com/metallb/metallb-operator/pkg/params"
"github.com/metallb/metallb-operator/test/consts"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -30,7 +29,7 @@ var _ = Describe("MetalLB Controller", func() {
reconciler.EnvConfig = defaultEnvConfig
})

DescribeTable("Should create manifests with images and namespace overriden", func(bgpType params.BGPType) {
DescribeTable("Should create manifests with images and namespace overriden", func(bgpType metallbv1beta1.BGPType) {

metallb := &metallbv1beta1.MetalLB{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -203,12 +202,12 @@ var _ = Describe("MetalLB Controller", func() {
})
Expect(err).NotTo(HaveOccurred())
},
Entry("Native Mode", params.NativeMode),
Entry("FRR Mode", params.FRRMode),
Entry("FRR-K8s Mode", params.FRRK8sMode),
Entry("Native Mode", metallbv1beta1.NativeMode),
Entry("FRR Mode", metallbv1beta1.FRRMode),
Entry("FRR-K8s Mode", metallbv1beta1.FRRK8sMode),
)

DescribeTable("Should forward logLevel to containers", func(bgpType params.BGPType) {
DescribeTable("Should forward logLevel to containers", func(bgpType metallbv1beta1.BGPType) {

metallb := &metallbv1beta1.MetalLB{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -260,9 +259,9 @@ var _ = Describe("MetalLB Controller", func() {
WithTransform(argsGetter, ContainElement("--log-level=warn")),
)))
},
Entry("Native Mode", params.NativeMode),
Entry("FRR Mode", params.FRRMode),
Entry("FRR-K8s Mode", params.FRRK8sMode),
Entry("Native Mode", metallbv1beta1.NativeMode),
Entry("FRR Mode", metallbv1beta1.FRRMode),
Entry("FRR-K8s Mode", metallbv1beta1.FRRK8sMode),
)

It("Should create manifests for frr-k8s", func() {
Expand All @@ -272,7 +271,7 @@ var _ = Describe("MetalLB Controller", func() {
Namespace: MetalLBTestNameSpace,
},
Spec: metallbv1beta1.MetalLBSpec{
BGPBackend: params.FRRK8sMode,
BGPBackend: metallbv1beta1.FRRK8sMode,
},
}

Expand Down Expand Up @@ -321,11 +320,11 @@ var _ = Describe("MetalLB Controller", func() {

})
It("Should switch between modes", func() {
checkSpeakerBGPMode := func(mode params.BGPType) {
checkSpeakerBGPMode := func(mode metallbv1beta1.BGPType) {
bgpTypeMatcher := ContainElement(v1.EnvVar{Name: "METALLB_BGP_TYPE", Value: string(mode)})
// Since when running in native mode the helm chart doesn't set the type, here we
// check for the absence of the env variable instead of having it set with a given value.
if mode == params.NativeMode {
if mode == metallbv1beta1.NativeMode {
bgpTypeMatcher = Not(ContainElement(HaveField("Name", "METALLB_BGP_TYPE")))
}

Expand Down Expand Up @@ -354,7 +353,7 @@ var _ = Describe("MetalLB Controller", func() {
Namespace: MetalLBTestNameSpace,
},
Spec: metallbv1beta1.MetalLBSpec{
BGPBackend: params.FRRK8sMode,
BGPBackend: metallbv1beta1.FRRK8sMode,
},
}

Expand All @@ -370,13 +369,13 @@ var _ = Describe("MetalLB Controller", func() {
}, 2*time.Second, 200*time.Millisecond).ShouldNot((HaveOccurred()))

By("Checking the speaker is running in frr k8s mode")
checkSpeakerBGPMode(params.FRRK8sMode)
checkSpeakerBGPMode(metallbv1beta1.FRRK8sMode)

By("Updating to frr mode")
toUpdate := &metallbv1beta1.MetalLB{}
err = k8sClient.Get(context.Background(), client.ObjectKey{Name: "metallb", Namespace: MetalLBTestNameSpace}, toUpdate)
Expect(err).ToNot(HaveOccurred())
toUpdate.Spec.BGPBackend = params.FRRMode
toUpdate.Spec.BGPBackend = metallbv1beta1.FRRMode
err = k8sClient.Update(context.Background(), toUpdate)
Expect(err).ToNot(HaveOccurred())

Expand All @@ -387,18 +386,18 @@ var _ = Describe("MetalLB Controller", func() {
}, 5*time.Second, 200*time.Millisecond).Should(BeTrue())

By("Checking the speaker is running in frr mode")
checkSpeakerBGPMode(params.FRRMode)
checkSpeakerBGPMode(metallbv1beta1.FRRMode)

By("Updating to native mode")
toUpdate = &metallbv1beta1.MetalLB{}
err = k8sClient.Get(context.Background(), client.ObjectKey{Name: "metallb", Namespace: MetalLBTestNameSpace}, toUpdate)
Expect(err).ToNot(HaveOccurred())
toUpdate.Spec.BGPBackend = params.NativeMode
toUpdate.Spec.BGPBackend = metallbv1beta1.NativeMode
err = k8sClient.Update(context.Background(), toUpdate)
Expect(err).ToNot(HaveOccurred())

By("Checking the speaker is running in native mode")
checkSpeakerBGPMode(params.NativeMode)
checkSpeakerBGPMode(metallbv1beta1.NativeMode)

By("Leaving the bgp backend empty")
toUpdate = &metallbv1beta1.MetalLB{}
Expand All @@ -409,7 +408,7 @@ var _ = Describe("MetalLB Controller", func() {
Expect(err).ToNot(HaveOccurred())

By("Checking the speaker is running in frr mode")
checkSpeakerBGPMode(params.FRRMode)
checkSpeakerBGPMode(metallbv1beta1.FRRMode)
})
})
})
Expand Down
3 changes: 1 addition & 2 deletions pkg/helm/frrk8s_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

metallbv1beta1 "github.com/metallb/metallb-operator/api/v1beta1"
"github.com/metallb/metallb-operator/pkg/params"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -123,7 +122,7 @@ func TestParseFRRK8SOCPSecureMetrics(t *testing.T) {
Namespace: MetalLBTestNameSpace,
},
Spec: metallbv1beta1.MetalLBSpec{
BGPBackend: params.FRRMode,
BGPBackend: metallbv1beta1.FRRMode,
},
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/helm/metallb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,10 @@ func controllerValues(envConfig params.EnvConfig, crdConfig *metallbv1beta1.Meta

func speakerValues(envConfig params.EnvConfig, crdConfig *metallbv1beta1.MetalLB) map[string]interface{} {
frrEnabled := false
if crdConfig.BGPBackend() == params.FRRMode {
if params.BGPType(crdConfig, envConfig.IsOpenshift) == metallbv1beta1.FRRMode {
frrEnabled = true
}

speakerValueMap := map[string]interface{}{
"image": map[string]interface{}{
"repository": envConfig.SpeakerImage.Repo,
Expand Down Expand Up @@ -333,7 +334,7 @@ func speakerValues(envConfig params.EnvConfig, crdConfig *metallbv1beta1.MetalLB

func metalLBFrrk8sValues(envConfig params.EnvConfig, crdConfig *metallbv1beta1.MetalLB) map[string]interface{} {
enabled := false
if crdConfig.BGPBackend() == params.FRRK8sMode {
if params.BGPType(crdConfig, envConfig.IsOpenshift) == metallbv1beta1.FRRK8sMode {
enabled = true
}
frrk8sValuesMap := map[string]interface{}{
Expand Down
6 changes: 3 additions & 3 deletions pkg/helm/metallb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func TestParseMetalLBChartWithCustomValues(t *testing.T) {
ControllerConfig: controllerConfig,
SpeakerConfig: speakerConfig,
LoadBalancerClass: loadBalancerClass,
BGPBackend: params.NativeMode,
BGPBackend: metallbv1beta1.NativeMode,
},
}

Expand Down Expand Up @@ -239,7 +239,7 @@ func TestParseOCPSecureMetrics(t *testing.T) {
Namespace: MetalLBTestNameSpace,
},
Spec: metallbv1beta1.MetalLBSpec{
BGPBackend: params.FRRMode,
BGPBackend: metallbv1beta1.FRRMode,
},
}

Expand Down Expand Up @@ -285,7 +285,7 @@ func TestParseSecureMetrics(t *testing.T) {
Namespace: MetalLBTestNameSpace,
},
Spec: metallbv1beta1.MetalLBSpec{
BGPBackend: params.FRRMode,
BGPBackend: metallbv1beta1.FRRMode,
},
}

Expand Down
18 changes: 11 additions & 7 deletions pkg/params/params.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,24 @@ import (
"os"
"strconv"
"strings"

"github.com/metallb/metallb-operator/api/v1beta1"
)

type ImageInfo struct {
Repo string
Tag string
}

const (
FRRMode BGPType = "frr"
NativeMode BGPType = "native"
FRRK8sMode BGPType = "frr-k8s"
)

type BGPType string
func BGPType(m *v1beta1.MetalLB, isOpenshift bool) v1beta1.BGPType {
if m.Spec.BGPBackend == "" {
if isOpenshift {
return v1beta1.FRRK8sMode
}
return v1beta1.FRRMode
}
return m.Spec.BGPBackend
}

type EnvConfig struct {
Namespace string
Expand Down
21 changes: 10 additions & 11 deletions test/e2e/functional/tests/e2e.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
. "github.com/onsi/gomega"

metallbv1beta1 "github.com/metallb/metallb-operator/api/v1beta1"
"github.com/metallb/metallb-operator/pkg/params"
"github.com/metallb/metallb-operator/pkg/status"
"github.com/metallb/metallb-operator/test/consts"
testclient "github.com/metallb/metallb-operator/test/e2e/client"
Expand Down Expand Up @@ -81,13 +80,13 @@ var _ = Describe("metallb", func() {
}
})

DescribeTable("with BGP type", func(bgpType params.BGPType) {
if isOpenshift && bgpType == params.NativeMode {
DescribeTable("with BGP type", func(bgpType metallbv1beta1.BGPType) {
if isOpenshift && bgpType == metallbv1beta1.NativeMode {
Skip("Native mode not supported with openshift")
}
checkControllerBGPMode := func(mode params.BGPType) {
checkControllerBGPMode := func(mode metallbv1beta1.BGPType) {
bgpTypeMatcher := ContainElement(corev1.EnvVar{Name: "METALLB_BGP_TYPE", Value: string(mode)})
if mode == params.NativeMode {
if mode == metallbv1beta1.NativeMode {
bgpTypeMatcher = Not(ContainElement(HaveField("Name", "METALLB_BGP_TYPE")))
}

Expand All @@ -110,9 +109,9 @@ var _ = Describe("metallb", func() {
)))
}

checkSpeakerBGPMode := func(mode params.BGPType) {
checkSpeakerBGPMode := func(mode metallbv1beta1.BGPType) {
bgpTypeMatcher := ContainElement(corev1.EnvVar{Name: "METALLB_BGP_TYPE", Value: string(mode)})
if mode == params.NativeMode {
if mode == metallbv1beta1.NativeMode {
bgpTypeMatcher = Not(ContainElement(HaveField("Name", "METALLB_BGP_TYPE")))
}

Expand Down Expand Up @@ -251,7 +250,7 @@ var _ = Describe("metallb", func() {
return true
}, 5*time.Minute, 5*time.Second).Should(BeTrue())

if bgpType != params.FRRK8sMode {
if bgpType != metallbv1beta1.FRRK8sMode {
return
}
By("checking frr-k8s daemonset is in running state")
Expand Down Expand Up @@ -307,9 +306,9 @@ var _ = Describe("metallb", func() {
}, metallbutils.DeployTimeout, metallbutils.Interval).ShouldNot(HaveOccurred())

},
Entry("Native Mode", params.NativeMode),
Entry("FRR Mode", params.FRRMode),
Entry("FRR-K8s Mode", params.FRRK8sMode),
Entry("Native Mode", metallbv1beta1.NativeMode),
Entry("FRR Mode", metallbv1beta1.FRRMode),
Entry("FRR-K8s Mode", metallbv1beta1.FRRK8sMode),
)

})
Expand Down

0 comments on commit 613974a

Please sign in to comment.