Skip to content

Conversation

@galal-hussein
Copy link
Contributor

Proposed Changes

  • Change the coreclient to use k3s-controller-client kubeconfig
  • update rbac for k3s-controller to include list/watch/update for nodes
  • remove unneeded feature gates
  • update the embedded scheduler calls

Types of Changes

Verification

Testing

Linked Issues

User-Facing Change


Further Comments

Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

lgtm, nits on RBAC changes.

resources:
- nodes/status
verbs:
- patch
Copy link
Member

Choose a reason for hiding this comment

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

This matches the RBAC from upstream flannel: https://github.com/flannel-io/flannel/blob/master/chart/kube-flannel/templates/rbac.yaml#L6-L21

I'm not super stoked on all agents being able to patch each others status, but it seems like this is how flannel works at the moment.

logrus.Infof("Starting flannel with backend %s", nodeConfig.FlannelBackend)
if err := waitForPodCIDR(ctx, nodeConfig.AgentConfig.NodeName, nodes); err != nil {

if err := util.WaitForRBACReady(ctx, nodeConfig.AgentConfig.KubeConfigK3sController, util.DefaultAPIServerReadyTimeout, authorizationv1.ResourceAttributes{
Copy link
Member

Choose a reason for hiding this comment

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

This avoids having flannel spam the log with errors while we wait for the RBAC manifest to be applied.

}
}

// If we're running the embedded cloud controller, wait for it to untaint at least one
Copy link
Member

@brandond brandond Dec 19, 2024

Choose a reason for hiding this comment

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

Moved this here from pkg/daemons/executor/embed.go so that we can use the kube-scheduler's own kubeconfig to wait for ready nodes. Without parsing component args, executor only knows the location of agent kubeconfigs.

@brandond brandond self-requested a review December 20, 2024 00:25
@brandond brandond force-pushed the v1.32.0+k3s1 branch 3 times, most recently from 6aa0c7c to 853d502 Compare December 20, 2024 04:03
@brandond brandond force-pushed the v1.32.0+k3s1 branch 2 times, most recently from d0b2136 to 9762e92 Compare December 20, 2024 04:36
@codecov
Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 64.36782% with 31 lines in your changes missing coverage. Please review.

Project coverage is 39.16%. Comparing base (83a3e85) to head (a30ce2f).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
pkg/daemons/control/server.go 67.27% 14 Missing and 4 partials ⚠️
pkg/agent/flannel/setup.go 52.94% 5 Missing and 3 partials ⚠️
pkg/agent/run.go 33.33% 0 Missing and 2 partials ⚠️
pkg/daemons/control/deps/deps.go 80.00% 2 Missing ⚠️
pkg/cloudprovider/servicelb.go 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (83a3e85) and HEAD (a30ce2f). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (83a3e85) HEAD (a30ce2f)
unittests 1 0
e2etests 7 6
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11478      +/-   ##
==========================================
- Coverage   47.63%   39.16%   -8.48%     
==========================================
  Files         181      164      -17     
  Lines       18794    18219     -575     
==========================================
- Hits         8953     7135    -1818     
- Misses       8490     9885    +1395     
+ Partials     1351     1199     -152     
Flag Coverage Δ
e2etests 35.50% <64.36%> (-7.45%) ⬇️
inttests 34.85% <64.36%> (+16.12%) ⬆️
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

blocked on wrangler/gengo updates

galal-hussein and others added 3 commits December 20, 2024 20:38
Signed-off-by: galal-hussein <[email protected]>
Signed-off-by: Brad Davidson <[email protected]>
Signed-off-by: galal-hussein <[email protected]>
Signed-off-by: Brad Davidson <[email protected]>
These are broken by AuthorizeNodeWithSelectors being on by default. All
agents must be upgraded to v1.32 or newer to work properly, until we
backport RBAC changes to older branches.

Signed-off-by: Brad Davidson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants