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

Adds provisioner limits based on cpu and mem #817

Merged
merged 27 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
632e737
Limit provisioner by cpu and memory
suket22 Nov 19, 2021
db8dbb1
Update pkg/apis/provisioning/v1alpha5/provisioner.go
suket22 Nov 19, 2021
5a7759a
Update pkg/controllers/provisioning/provisioner.go
suket22 Nov 19, 2021
5c5b3cf
Update pkg/controllers/provisioning/resourcecounter.go
suket22 Nov 19, 2021
e00778a
Separate controller for counting resources
suket22 Nov 22, 2021
4a6cd5e
Separate controller for counting resources
suket22 Nov 22, 2021
7633fc9
Etarn fixes
ellistarn Nov 23, 2021
9310ccd
Merge pull request #1 from ellistarn/limitsImpl
suket22 Nov 23, 2021
217942f
Some more fixes - don't default the status
suket22 Nov 23, 2021
2f291aa
Remove extra logging statement
suket22 Nov 23, 2021
9d67667
Fix defaults, fix binding errors
suket22 Nov 23, 2021
3285786
More minor fixes
suket22 Nov 23, 2021
4531d68
Remove extra patch from rbac
suket22 Nov 23, 2021
00a10d2
Addressing comments on the PR
suket22 Nov 23, 2021
2e20056
Compare provisionerSpecs before stopping existing Provisioners
suket22 Nov 23, 2021
7134769
Fix failing build!
suket22 Nov 23, 2021
64c1714
Merge branch 'main' into limitsImpl
suket22 Nov 23, 2021
555d18d
Don't reassign the provisioner in the launcher
suket22 Nov 24, 2021
f866907
Addressing more comments on the PR
suket22 Nov 24, 2021
74d6bd9
Adds basic unit test
suket22 Nov 24, 2021
afa1796
Update pkg/controllers/counter/controller.go
suket22 Nov 24, 2021
9974847
Update pkg/apis/provisioning/v1alpha5/provisioner.go
suket22 Nov 24, 2021
dfbc043
Update pkg/controllers/counter/controller.go
suket22 Nov 24, 2021
3036eea
More refactoring
suket22 Nov 24, 2021
ca36e42
Merge branch 'main' into limitsImpl
suket22 Nov 24, 2021
26c1831
Update pkg/controllers/provisioning/suite_test.go
suket22 Nov 24, 2021
2591f0a
Apply suggestions from code review
suket22 Nov 24, 2021
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
Prev Previous commit
Next Next commit
Fix defaults, fix binding errors
  • Loading branch information
suket22 committed Nov 23, 2021
commit 9d676676ff07fc4d73daf4eb2536074196c71db3
2 changes: 1 addition & 1 deletion pkg/apis/provisioning/v1alpha5/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
)

var DefaultCPULimits *resource.Quantity = resource.NewScaledQuantity(100, 0)
suket22 marked this conversation as resolved.
Show resolved Hide resolved
var DefaultMemoryLimits *resource.Quantity = resource.NewScaledQuantity(100, 0)
var DefaultMemoryLimits *resource.Quantity = resource.NewScaledQuantity(400, resource.Giga)

// Limits define bounds on the resources being provisioned by Karpenter
type Limits struct {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/counter/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (c *Controller) resourceCountsFor(ctx context.Context, provisionerName stri
}

var cpu = resource.NewScaledQuantity(0, 0)
var memory = resource.NewScaledQuantity(0, 0)
var memory = resource.NewScaledQuantity(0, resource.Giga)

for _, node := range nodes.Items {
suket22 marked this conversation as resolved.
Show resolved Hide resolved
cpu.Add(*node.Status.Capacity.Cpu())
Expand Down
15 changes: 15 additions & 0 deletions pkg/controllers/provisioning/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import (
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha5"
Expand Down Expand Up @@ -129,6 +131,19 @@ func (c *Controller) Register(_ context.Context, m manager.Manager) error {
NewControllerManagedBy(m).
Named("provisioning").
For(&v1alpha5.Provisioner{}).
WithEventFilter(predicate.Funcs{
suket22 marked this conversation as resolved.
Show resolved Hide resolved
UpdateFunc: func(e event.UpdateEvent) bool {
// If the only change to the provisioner resource has been in the status field, then we shouldn't be reconciling.
// This is important because otherwise you can end up in an odd loop. Consider this -
// 1. The launcher adds new nodes -> which leads to provisioner.status.resources being updated.
// 2. We then think a new provisioner is needed and cancel the context for the old provisioner.
// 3. If the old provisioner was doing things (say binding pods after the node was spun up) - it might then fail.
//
// When there's a change in just the status field (and not the spec), the generation values will remain the same so use that to
// determine if we need to reconcile.
return e.ObjectNew.GetGeneration() != e.ObjectOld.GetGeneration()
suket22 marked this conversation as resolved.
Show resolved Hide resolved
},
}).
WithOptions(controller.Options{MaxConcurrentReconciles: 10}).
Complete(c)
}