Skip to content

Allow SPIRE agent on control plane#28947

Merged
dylandreimerink merged 1 commit intocilium:mainfrom
meyskens:meyskens/spire-on-control-plane
Jan 9, 2024
Merged

Allow SPIRE agent on control plane#28947
dylandreimerink merged 1 commit intocilium:mainfrom
meyskens:meyskens/spire-on-control-plane

Conversation

@meyskens
Copy link
Copy Markdown
Contributor

@meyskens meyskens commented Nov 2, 2023

This sets the default toleration for SPIRE agent to be allowed on the control plane nodes.
This allows Cilium Agent on these nodes to get attested by SPIRE for Mutual Authentication to work.

Fixes: #28694

Add default toleration for SPIRE agent on control plane nodes

@meyskens meyskens requested review from a team as code owners November 2, 2023 14:22
@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 Nov 2, 2023
@meyskens meyskens marked this pull request as draft November 2, 2023 14:22
@meyskens meyskens added release-note/bug This PR fixes an issue in a previous release of Cilium. area/servicemesh GH issues or PRs regarding servicemesh feature/authentication labels Nov 2, 2023
@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 Nov 2, 2023
@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch from 77fb368 to 55feacf Compare November 2, 2023 14:30
@rauanmayemir
Copy link
Copy Markdown
Contributor

rauanmayemir commented Nov 2, 2023

I think it should set tolerations same as cilium-agent, which is to tolerate pretty much anything.

Otherwise, nodes with other taints will schedule cilium, but not spire agent.

squeed
squeed reviewed Nov 3, 2023
View reviewed changes
@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch 3 times, most recently from f7458e8 to 75f4949 Compare November 6, 2023 10:14
@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Nov 6, 2023

/test

@meyskens meyskens marked this pull request as ready for review November 6, 2023 10:59
@meyskens meyskens requested a review from a team as a code owner November 6, 2023 10:59
@meyskens meyskens requested a review from mhofstetter November 6, 2023 11:00
mhofstetter
mhofstetter reviewed Nov 6, 2023
View reviewed changes
mhofstetter
mhofstetter reviewed Nov 6, 2023
View reviewed changes
@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch from 75f4949 to e9b25b6 Compare November 6, 2023 12:03
@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch from e9b25b6 to b3daa0b Compare November 8, 2023 08:56
mhofstetter
mhofstetter approved these changes Nov 8, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

Thanks for the changes - LGTM!

@mhofstetter mhofstetter requested a review from squeed November 8, 2023 09:24
@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Nov 8, 2023

/test

@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Nov 9, 2023

E2E tests are failing because of this change. They all have a node without cilium set via the CLI, this adds a CLI added nodeAffinity rule to not schedule. In the past this wasn't an issue as the node never became ready so it wasn't scheduled.
I think it is best to also apply the affinity in cilium-cli for the SPIRE deployment, otherwise the flag might break things in combination with mutual auth.

@squeed squeed mentioned this pull request Nov 10, 2023
Closed
8 tasks
@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch from c0780b6 to e3d66ef Compare November 22, 2023 13:59
@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Nov 22, 2023

/test

@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Nov 22, 2023

As feared/kinda expected the pod affinity does't play well with the daemonset and leaves one pending

NAMESPACE            NAME                                         READY   STATUS    RESTARTS   AGE     IP             NODE                 NOMINATED NODE   READINESS GATES
cilium-spire         spire-agent-7s22n                            1/1     Running   0          6m30s   172.18.0.3     kind-control-plane   <none>           <none>
cilium-spire         spire-agent-b2pjj                            1/1     Running   0          6m30s   172.18.0.2     kind-worker2         <none>           <none>
cilium-spire         spire-agent-kwcr7                            0/1     Pending   0          6m30s   <none>         <none>               <none>           <none>
cilium-spire         spire-agent-lgnvt                            1/1     Running   0          6m30s   172.18.0.4     kind-worker          <none>           <none>
cilium-spire         spire-server-0                               2/2     Running   0          6m30s   10.244.2.136   kind-worker2         <none>           <none>

@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Nov 22, 2023

Will remove that commit...

@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch 2 times, most recently from 3ec9de6 to 20137ce Compare November 22, 2023 16:21
@joestringer
Copy link
Copy Markdown
Member

joestringer commented Dec 12, 2023

Should this also be backported to v1.14 branch?

@meyskens
Allow SPIRE agent on control plane
c510954 
This sets the default toleration for SPIRE agent to be allowed on the
control plane nodes.
This allows Cilium Agent on these nodes to get attested by SPIRE for
Mutual Authentication to work.

Signed-off-by: Maartje Eyskens <[email protected]>
@meyskens meyskens force-pushed the meyskens/spire-on-control-plane branch from 20137ce to c510954 Compare December 18, 2023 12:57
@meyskens
Copy link
Copy Markdown
Contributor Author

meyskens commented Dec 18, 2023

/test

@meyskens meyskens marked this pull request as ready for review January 8, 2024 13:13
@meyskens meyskens added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 9, 2024
@dylandreimerink dylandreimerink added this pull request to the merge queue Jan 9, 2024
Merged via the queue into cilium:main with commit b193daa Jan 9, 2024
@jibi jibi mentioned this pull request Jan 11, 2024
Merged
9 tasks
@jibi jibi mentioned this pull request Jan 12, 2024
Merged
32 tasks
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 labels Jan 15, 2024
@aanm aanm mentioned this pull request Jan 16, 2024
Merged
@gentoo-root gentoo-root mentioned this pull request Jan 17, 2024
Merged
@giorio94 giorio94 added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 labels Jan 29, 2024
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

@christarazi christarazi christarazi approved these changes

@mhofstetter mhofstetter mhofstetter approved these changes

Assignees

No one assigned

Labels

area/servicemesh GH issues or PRs regarding servicemesh backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. feature/authentication ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

No open projects
cilium v1.14.6
Status: Released

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cilium Operator failing to connect to SPIRE server

10 participants

@meyskens @rauanmayemir @squeed @joestringer @christarazi @mhofstetter @jibi @gentoo-root @dylandreimerink @giorio94