Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speaker: hide the L2ServiceStatus generation behind a flag #2312

Merged
merged 2 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ jobs:
HELM_FLAGS=""
echo '/etc/frr/core-%e.%p.%h.%t' | sudo tee /proc/sys/kernel/core_pattern
if [ ${{ matrix.deployment }} = "helm" ]; then export SPEAKER_SELECTOR="app.kubernetes.io/component=speaker" && export CONTROLLER_SELECTOR="app.kubernetes.io/component=controller"; fi
SKIP="none"
# TODO restore to NONE when https://github.com/metallb/metallb/issues/2311 is fixed
SKIP="L2ServiceStatus"
WITH_VRF="--with-vrf"
FOCUS=""
if [ "${{ matrix.bgp-type }}" == "native" ]; then SKIP="$SKIP|FRR|FRR-MODE|FRRK8S-MODE|BFD|VRF|DUALSTACK"; WITH_VRF=""; fi
Expand Down Expand Up @@ -330,7 +331,8 @@ jobs:
spec:
logLevel: debug
EOF
sudo -E env "PATH=$PATH" inv e2etest --bgp-mode frr --skip "IPV6|DUALSTACK|metrics|L2-interface selector|FRRK8S-MODE" -e /tmp/kind_logs
# TOOD remove skipping L2ServiceStatus once https://github.com/metallb/metallb/issues/2311 is fixed
sudo -E env "PATH=$PATH" inv e2etest --bgp-mode frr --skip "IPV6|DUALSTACK|metrics|L2-interface selector|FRRK8S-MODE|L2ServiceStatus" -e /tmp/kind_logs

- name: Collect Logs
if: ${{ failure() }}
Expand Down Expand Up @@ -366,7 +368,7 @@ jobs:
- name: E2E
run: |
FOCUS="L2.*should work for ExternalTrafficPolicy=Cluster|BGP.*A service of protocol load balancer should work with.*IPV4 - ExternalTrafficPolicyCluster$|validate FRR running configuration"
sudo -E env "PATH=$PATH" inv e2etest --bgp-mode frr --focus "$FOCUS" -e /tmp/kind_logs
sudo -E env "PATH=$PATH" inv e2etest --bgp-mode frr --focus "$FOCUS" --skip L2ServiceStatus -e /tmp/kind_logs

- name: Collect Logs
if: ${{ failure() }}
Expand Down
2 changes: 1 addition & 1 deletion e2etest/l2tests/interface_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ var _ = ginkgo.Describe("L2-interface selector", func() {
framework.ExpectNoError(err)
})

ginkgo.It("Validate ServiceL2Status interface", func() {
ginkgo.It("Validate L2ServiceStatus interface", func() {
ginkgo.By("use the 1st interface for announcing")
resources := config.Resources{
L2Advs: []metallbv1beta1.L2Advertisement{
Expand Down
12 changes: 10 additions & 2 deletions e2etest/l2tests/l2.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ var _ = ginkgo.Describe("L2", func() {
gomega.Eventually(func() error {
return service.ValidateL2(svc)
}, 2*time.Minute, 1*time.Second).ShouldNot(gomega.HaveOccurred())
})

ginkgo.It("should expose the status as L2ServiceStatus", func() {
svc, _ := service.CreateWithBackend(cs, f.Namespace.Name, "external-local-lb", service.TrafficPolicyCluster)

defer func() {
err := cs.CoreV1().Services(svc.Namespace).Delete(context.TODO(), svc.Name, metav1.DeleteOptions{})
framework.ExpectNoError(err)
}()

ginkgo.By("checking correct serviceL2Status object is populated")

Expand Down Expand Up @@ -800,9 +809,8 @@ var _ = ginkgo.Describe("L2", func() {
Name: fmt.Sprintf("test-addresspool%d", i+1),
},
Spec: metallbv1beta1.IPAddressPoolSpec{
Addresses: []string{addressesRange},
Addresses: []string{addressesRange},
AutoAssign: &autoAssign,

},
}
pools = append(pools, pool)
Expand Down
3 changes: 2 additions & 1 deletion internal/k8s/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ type Config struct {
Listener
Layer2StatusChan <-chan event.GenericEvent
Layer2StatusFetcher controllers.StatusFetcher
EnableL2Status bool
}

// New connects to masterAddr, using kubeconfig to authenticate.
Expand Down Expand Up @@ -278,7 +279,7 @@ func New(cfg *Config) (*Client, error) {
}

// metallb controller doesn't need this reconciler
if cfg.Layer2StatusChan != nil {
if cfg.EnableL2Status && cfg.Layer2StatusChan != nil {
if err = (&controllers.Layer2StatusReconciler{
Client: mgr.GetClient(),
Logger: cfg.Logger,
Expand Down
7 changes: 7 additions & 0 deletions speaker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ func main() {
enablePprof = flag.Bool("enable-pprof", false, "Enable pprof profiling")
loadBalancerClass = flag.String("lb-class", "", "load balancer class. When enabled, metallb will handle only services whose spec.loadBalancerClass matches the given lb class")
ignoreLBExclude = flag.Bool("ignore-exclude-lb", false, "ignore the exclude-from-external-load-balancers label")
// TODO: we are hiding the feature behind a feature flag because of https://github.com/metallb/metallb/issues/2311
// This flag can be removed once the issue is fixed.
enableL2ServiceStatus = flag.Bool("enable-l2-service-status", false, "enables the experimental l2 service status feature")
)
flag.Parse()

Expand Down Expand Up @@ -162,6 +165,9 @@ func main() {
InterfaceExcludeRegexp: interfacesToExclude,
IgnoreExcludeLB: *ignoreLBExclude,
Layer2StatusChange: func(namespacedName types.NamespacedName) {
if !*enableL2ServiceStatus {
return
}
statusNotifyChan <- controllers.NewL2StatusEvent(namespacedName.Namespace, namespacedName.Name)
},
})
Expand Down Expand Up @@ -201,6 +207,7 @@ func main() {
LoadBalancerClass: *loadBalancerClass,
WithFRRK8s: listenFRRK8s,

EnableL2Status: *enableL2ServiceStatus,
Layer2StatusChan: statusNotifyChan,
Layer2StatusFetcher: ctrl.layer2StatusFetchFunc,
})
Expand Down
Loading