Skip to content

fix: add kubeadmRbac to soot triggers slice #1169

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

Open
HarshavardhanK wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: add kubeadmRbac to soot triggers slice #1169

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cea9d4e
fix(soot): add kubeadmRbac to soot triggers slice
HarshavardhanK Jun 4, 2026
b163f2a
fix(soot): buffer kubeadmRbac channel and pre-seed on startup
HarshavardhanK Jun 4, 2026
86872f1
style: trim verbose comments in soot manager fix
HarshavardhanK Jun 5, 2026
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
9 changes: 8 additions & 1 deletion controllers/soot/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (m *Manager) Reconcile(ctx context.Context, request reconcile.Request) (res
Client: m.AdminClient,
Phase: resources.PhaseClusterAdminRBAC,
},
TriggerChannel: make(chan event.GenericEvent),
TriggerChannel: make(chan event.GenericEvent, 1), // buffered: pre-seeded below to run on startup

@prometherion prometherion Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't need to change the channel's buffering: what's the rationale?

I get the point of the initial seeding, but I think this is not required.

ControllerName: fmt.Sprintf("%s-kubeadmrbac", controllerNamePrefix),
}
if err = kubeadmRbac.SetupWithManager(mgr); err != nil {
Expand Down Expand Up @@ -393,6 +393,12 @@ func (m *Manager) Reconcile(ctx context.Context, request reconcile.Request) (res
close(completedCh)
}()

// Pre-seed so PhaseClusterAdminRBAC runs on startup rather than waiting for a kubeadm: CRB watch event.
var initTCP kamajiv1alpha1.TenantControlPlane
initTCP.Name = tcp.Name
initTCP.Namespace = tcp.Namespace
kubeadmRbac.TriggerChannel <- event.GenericEvent{Object: &initTCP}
Comment on lines +396 to +400

@prometherion prometherion Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't get the point of running this by triggering it, except to add an exception considering the eventually consistent nature of Kubernetes.

When the Manager is configured, it will start the Watch against the kubeadm:* Cluster Role, which is created by the UploadConfiguration's Kubeadm function that will be triggered immediately upon soot manager setup.


m.sootMap[request.NamespacedName.String()] = sootItem{
triggers: []chan event.GenericEvent{
writePermissions.TriggerChannel,
Expand All @@ -403,6 +409,7 @@ func (m *Manager) Reconcile(ctx context.Context, request reconcile.Request) (res
uploadKubeadmConfig.TriggerChannel,
uploadKubeletConfig.TriggerChannel,
bootstrapToken.TriggerChannel,
kubeadmRbac.TriggerChannel,

@prometherion prometherion Jun 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is absolutely a bug, and this is required.

},
cancelFn: tcpCancelFn,
completedCh: completedCh,
Expand Down
Loading