Skip to content

bgpv2: Introduce MissingPeerConfig condition to the ClusterConfig #35527

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
Merged
merged 2 commits into from
Oct 28, 2024
Merged

bgpv2: Introduce MissingPeerConfig condition to the ClusterConfig #35527

merged 2 commits into from
Oct 28, 2024

Conversation

YutaroHayakawa
Copy link
Member

Introduce a new condition that tells the missing PeerConfigs in the ClusterConfig.

apiVersion: cilium.io/v2alpha1
kind: CiliumBGPClusterConfig
metadata:
  name: cilium-bgp
spec:
  bgpInstances:
  - localASN: 65000
    name: instance0
    peers:
    - name: peer0
      peerConfigRef:
        group: cilium.io
        kind: CiliumBGPPeerConfig
        name: peer-config0
    - name: peer1
      peerConfigRef:
        group: cilium.io
        kind: CiliumBGPPeerConfig
        name: peer-config0
  nodeSelector:
    matchLabels:
      kubernetes.io/hostname: kind-worker
status:
  conditions:
  - lastTransitionTime: "2024-10-24T14:50:20Z"
    message: 'Referenced CiliumBGPPeerConfig(s) are missing: [peer-config0]'
    observedGeneration: 5
    reason: MissingPeerConfigs
    status: "True"
    type: cilium.io/MissingPeerConfigs

bgpv2: Introduce MissingPeerConfig condition to the ClusterConfig

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 24, 2024
@YutaroHayakawa YutaroHayakawa added the release-note/misc This PR makes changes that have no direct user impact. label Oct 24, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 24, 2024
@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review October 24, 2024 15:23
@YutaroHayakawa YutaroHayakawa requested review from a team as code owners October 24, 2024 15:23
@YutaroHayakawa YutaroHayakawa requested review from youngnick, rastislavs and squeed and removed request for a team October 24, 2024 15:23
@YutaroHayakawa
Copy link
Member Author

/test

youngnick
youngnick approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice.

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cluster-config-condition branch from 812070d to 6add65e Compare October 25, 2024 05:33
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgpv2-missing-peer-config-cond branch from 204c95f to 3861df1 Compare October 25, 2024 05:39
@YutaroHayakawa
Copy link
Member Author

/test

rastislavs
rastislavs approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@rastislavs rastislavs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@YutaroHayakawa YutaroHayakawa added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Oct 25, 2024
Base automatically changed from yutaro/bgp-cluster-config-condition to main October 25, 2024 10:42
@YutaroHayakawa
bgpv2: Add PeerConfig client to the test fixture
41609ac 
Add PeerConfig client to the test fixture as a preparation for the
subsequent commits. Also, cleaned up some watch reactor logic for
maintainability (including fixes for missing resource watchers).

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa
bgpv2: Introduce MissingPeerConfig condition to the ClusterConfig
175a107 
Introduce a new error reporting condition MissingPeerConfig which
indicates the referenced peer config doesn't exist. This helps users to
notice typo in the peer config reference for example.

We need to introduce a new PeerConfig watcher (and corresponding cluster
role) to the operator for this feature.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgpv2-missing-peer-config-cond branch from 3861df1 to 175a107 Compare October 25, 2024 10:56
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

Cilium E2E Upgrade: #35554

@YutaroHayakawa YutaroHayakawa added this pull request to the merge queue Oct 28, 2024
Merged via the queue into main with commit 3025827 Oct 28, 2024
270 checks passed
@YutaroHayakawa YutaroHayakawa deleted the yutaro/bgpv2-missing-peer-config-cond branch October 28, 2024 05:40
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Oct 28, 2024
@YutaroHayakawa YutaroHayakawa mentioned this pull request Oct 29, 2024
Merged
@harsimran-pabla harsimran-pabla mentioned this pull request Nov 4, 2024
Closed
3 tasks
@YutaroHayakawa YutaroHayakawa added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. and removed release-note/misc This PR makes changes that have no direct user impact. labels Nov 5, 2024
@YutaroHayakawa YutaroHayakawa mentioned this pull request Nov 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@squeed squeed squeed approved these changes

@youngnick youngnick youngnick approved these changes

@rastislavs rastislavs rastislavs approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@YutaroHayakawa @squeed @youngnick @rastislavs