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.

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

V1.32.0+k3s1 #11478

Merged
merged 3 commits into from
Dec 20, 2024
Merged

V1.32.0+k3s1 #11478

merged 3 commits into from
Dec 20, 2024

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.

manifests/rolebindings.yaml Outdated Show resolved Hide resolved
pkg/agent/run.go Outdated Show resolved Hide resolved
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
go.mod Outdated Show resolved Hide resolved
@brandond brandond force-pushed the v1.32.0+k3s1 branch 2 times, most recently from d0b2136 to 9762e92 Compare December 20, 2024 04:36
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

go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
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]>
@galal-hussein galal-hussein merged commit 763188d into k3s-io:master Dec 20, 2024
38 checks passed
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