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.

Sign up for GitHub

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

Jump to bottom

CiliumBGPNodeConfigOverride has no effect #35066

Closed
2 of 3 tasks
starcraft66 opened this issue Sep 27, 2024 · 7 comments · Fixed by #35552
Closed
2 of 3 tasks

CiliumBGPNodeConfigOverride has no effect #35066

starcraft66 opened this issue Sep 27, 2024 · 7 comments · Fixed by #35552
Assignees
@harsimran-pabla
Labels
area/bgp info-completed The GH issue has received a reply from the author kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack.

Comments

@starcraft66
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Version

equal or higher than v1.16.0 and lower than v1.17.0

What happened?

I created a CiliumBGPNodeConfigOverride to override the BGP source address of a node in order to prefer a static IPv6 address instead of the one assigned via SLAAC.

I created the following resources:

---
apiVersion: cilium.io/v2alpha1
kind: CiliumBGPClusterConfig
metadata:
  name: bgp-spike
spec:
  bgpInstances:
  - name: spike
    localASN: 64513
    peers:
    - name: 305-1700-gw-v4
      peerASN: 64512
      peerAddress: 172.17.51.1
      peerConfigRef:
        name: cilium-peer
    - name: 305-1700-gw-v6
      peerASN: 64512
      peerAddress: 2a10:4741:37:51::1
      peerConfigRef:
        name: cilium-peer
---
# The name of CiliumBGPNodeConfigOverride resource must match the name of the node for which the configuration is intended.
# Similarly, the names of the BGP instance and peers must match with what is defined under CiliumBGPClusterConfig.
apiVersion: cilium.io/v2alpha1
kind: CiliumBGPNodeConfigOverride
metadata:
  name: spike
spec:
  bgpInstances:
  - name: spike
    peers:
    - name: 305-1700-gw-v4
      localAddress: 172.17.51.16
    - name: 305-1700-gw-v6
      localAddress: 2a10:4741:37:51::16
---
apiVersion: cilium.io/v2alpha1
kind: CiliumBGPPeerConfig
metadata:
  name: cilium-peer
spec:  
  gracefulRestart:
    enabled: true
    restartTimeSeconds: 15

My node is named spike:

❯ kgno             
NAME    STATUS   ROLES    AGE   VERSION
spike   Ready    <none>   9h    v1.31.0

Despite this, I get messages on my router that the session can't be established because it's expecting the static ip and getting bgp connections from the slaac ip.

How can we reproduce the issue?

Install cilium and apply the manifests above, run cilium bgp peers to validate the establishment of sessions. Check router logs if there's an error.

Cilium Version

Client: 1.17.0-pre.0 d41440c 2024-09-04T23:25:39 00:00 go version go1.23.0 linux/amd64
Daemon: 1.17.0-pre.0 d41440c 2024-09-04T23:25:39 00:00 go version go1.23.0 linux/amd64

Kernel Version

Linux spike 6.6.52 #1-NixOS SMP PREEMPT_DYNAMIC Wed Sep 18 17:24:10 UTC 2024 x86_64 GNU/Linux

Kubernetes Version

Client Version: v1.31.0
Kustomize Version: v5.4.2
Server Version: v1.31.0

Regression

No response

Sysdump

No response

Relevant log output

No response

Anything else?

No response

Cilium Users Document

  • Are you a user of Cilium? Please add yourself to the Users doc

Code of Conduct

  • I agree to follow this project's Code of Conduct
@starcraft66 starcraft66 added kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. needs/triage This issue requires triaging to establish severity and next steps. labels Sep 27, 2024
@youngnick youngnick added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/bgp labels Sep 30, 2024
@Lattyware
Copy link

I also saw this trying to set up in an IPv6-only environment, where setting the router ID is required, as a workaround for my specific case I was able to use the annotation kubectl annotate node [node] cilium.io/bgp-virtual-router.[asn]="router-id=[id]" from the old system and that appears to work, but seems to be deprecated and isn't mentioned in the docs for the new version, which implies the override should be able to do this.

@markusnasholm
Copy link

I also saw this trying to set up in an IPv6-only environment, where setting the router ID is required, as a workaround for my specific case I was able to use the annotation kubectl annotate node [node] cilium.io/bgp-virtual-router.[asn]="router-id=[id]" from the old system and that appears to work, but seems to be deprecated and isn't mentioned in the docs for the new version, which implies the override should be able to do this.

1 on this. Couldn't get IPv6-only peering to work until I tried the old way of annotating the node with router-id. This workaround works for me but it would be nice if this bug could be fixed, as the documentation only mentions this under BGPv1.

@harsimran-pabla harsimran-pabla self-assigned this Oct 25, 2024
@harsimran-pabla
Copy link
Contributor

Thanks for reporting this issue. I am looking into it.

@harsimran-pabla harsimran-pabla added info-completed The GH issue has received a reply from the author and removed sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. needs/triage This issue requires triaging to establish severity and next steps. labels Oct 25, 2024
@harsimran-pabla harsimran-pabla mentioned this issue Oct 25, 2024
Merged
@harsimran-pabla
Copy link
Contributor

Hi @Lattyware, @markusnasholm, Issue reported was for not setting local-address when using CiliumBGPNodeConfigOverride configuration. That was indeed a bug, it is fixed with attached PR.

I am not sure why router-id would not work when using CiliumBGPNodeConfigOverride, can you please check CiliumBGPNodeConfig resource and see if router-id is accurately reflected in that CRD ?

@Lattyware
Copy link

Lattyware commented Oct 25, 2024 •
edited
Loading

Hi @Lattyware, @markusnasholm, Issue reported was for not setting local-address when using CiliumBGPNodeConfigOverride configuration. That was indeed a bug, it is fixed with attached PR.

I am not sure why router-id would not work when using CiliumBGPNodeConfigOverride, can you please check CiliumBGPNodeConfig resource and see if router-id is accurately reflected in that CRD ?

Yes, I can see it reflected in router-id in CiliumBGPNodeConfig (although I'm currently using the old annotation to make my setup work, I'm assuming it wouldn't pull that from the annotation into the CRD somehow).

The error I got at the time was failed to get cilium node IP <nil>: router id not specified by annotation, cannot resolve router id, so it may be that it's a separate bug where something is only looking for the annotation and not at that CRD for the router id when using the IPv4 fails?

@harsimran-pabla
Copy link
Contributor

@Lattyware, hmm I'd suggest opening a separate issue for router-id problem and attach a sysdump to the issue. I can go through the logs and see what could be an issue.

@Lattyware
Copy link

@Lattyware, hmm I'd suggest opening a separate issue for router-id problem and attach a sysdump to the issue. I can go through the logs and see what could be an issue.

I'll see when I can get a moment to recreate the issue in a test environment to get a sysdump, as I need to leave the workaround live in the environment I originally encountered it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@harsimran-pabla harsimran-pabla

Labels
area/bgp info-completed The GH issue has received a reply from the author kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bgpv2: set local peering address when specified harsimran-pabla/cilium
5 participants
@Lattyware @starcraft66 @youngnick @markusnasholm @harsimran-pabla