WIP: add a namespace selector to the provisioner spec #1496
Conversation
|
✔️ Deploy Preview for karpenter-docs-prod ready! 🔨 Explore the source changes: 39aed08 🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/622a392f435d310008394fc5 😎 Browse the preview: https://deploy-preview-1496--karpenter-docs-prod.netlify.app |
|
This likely requires more docs related to upgrading Karpenter versions. We needed the ability to list namespaces on the cluster role that current users likely won't have. |
tzneal
force-pushed
the
allow-provisioners-to-filter-on-namespace
branch
from
a6cb9c8 to
c433c76
Compare
| @@ -96,13 +101,30 @@ func (c *Controller) selectProvisioner(ctx context.Context, pod *v1.Pod) (errs e | |||
| if len(provisioners) == 0 { | |||
| return nil | |||
| } | |||
|
|
|||
| // lookup the pod namespace for matching against the provisioner | |||
| podNamespace, err := c.getPodNamespace(ctx, pod) | |||
There was a problem hiding this comment.
Can we encapsulate this inside provisionerCanProvision?
There was a problem hiding this comment.
You could, but then you're looking up the namespace n times. Maybe it doesn't matter?
This is modeled after the NamespaceSelector on pod affinity terms and works the same way. Fixes aws#1493
tzneal
force-pushed
the
allow-provisioners-to-filter-on-namespace
branch
from
c433c76 to
39aed08
Compare
| @@ -52,6 +52,7 @@ apply: ## Deploy the controller into your ~/.kube/config cluster | |||
| $(HELM_OPTS) \ | |||
| --set controller.image=$(shell $(WITH_GOFLAGS) ko build -B github.com/aws/karpenter/cmd/controller) \ | |||
| --set webhook.image=$(shell $(WITH_GOFLAGS) ko build -B github.com/aws/karpenter/cmd/webhook) | |||
| kubectl apply -f charts/karpenter/crds/ | |||
There was a problem hiding this comment.
Totally minor nit, but I just realized that our CRD should be installed first, since our webhooks reference them. I believe it only matters on first installation.
| @@ -96,13 +100,29 @@ func (c *Controller) selectProvisioner(ctx context.Context, pod *v1.Pod) (errs e | |||
| if len(provisioners) == 0 { | |||
| return nil | |||
| } | |||
|
|
|||
| // lookup the pod namespace for matching against the provisioner | |||
| var podNamespace v1.Namespace | |||
There was a problem hiding this comment.
I'd leave things unqualified unless it's unclear
| var podNamespace v1.Namespace | |
| var namespace v1.Namespace |
|
Holy cow!!! This PR was so quick!!! ❤️ @tzneal & @ellistarn |
tzneal
changed the title
|
This solution has several issues, will update docs to further clarify. |
1. Issue, if available:
Fixes #1493
2. Description of changes:
This is modeled after the NamespaceSelector on pod affinity terms
and works the same way.
3. How was this change tested?
Unit testing and creating/editing namespaces on an EKS cluster to watch pods be rejected or accepted by Karpenter.
4. Does this change impact docs?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.