chore: decouple webhook logic from cloud provider API #2079
Conversation
✅ Deploy Preview for karpenter-docs-prod ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
cmd/webhook/main.go
Outdated
| // Webhooks | ||
| { | ||
| certificates.NewController, | ||
| newCRDDefaultingWebhook, |
There was a problem hiding this comment.
Aren't newCRDDefaultingWebhook and newAWSDefaultingWebhook both defaulting webhooks for v1alpha5.Provisioner. Do both get called, or are we lucky because of registration order?
There was a problem hiding this comment.
IIUC, they're called in a random ordering
You can use reinvocation to enforce ordering, but we don't have interdependence.
https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy
| hasLabel := false | ||
| if _, ok := p.Spec.Labels[key]; ok { | ||
| hasLabel = true | ||
| } | ||
| for _, requirement := range p.Spec.Requirements { | ||
| if requirement.Key == key { | ||
| hasLabel = true | ||
| } | ||
| } | ||
| if !hasLabel { | ||
| p.Spec.Requirements = append(p.Spec.Requirements, v1.NodeSelectorRequirement{ | ||
| Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}, | ||
| }) | ||
| } |
There was a problem hiding this comment.
| hasLabel := false | |
| if _, ok := p.Spec.Labels[key]; ok { | |
| hasLabel = true | |
| } | |
| for _, requirement := range p.Spec.Requirements { | |
| if requirement.Key == key { | |
| hasLabel = true | |
| } | |
| } | |
| if !hasLabel { | |
| p.Spec.Requirements = append(p.Spec.Requirements, v1.NodeSelectorRequirement{ | |
| Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}, | |
| }) | |
| } | |
| _, hasLabel := p.Spec.Labels[key] | |
| for _, requirement := range p.Spec.Requirements { | |
| if requirement.Key == key || hasLabel { | |
| hasLabel = true | |
| break | |
| } | |
| } | |
| if !hasLabel { | |
| p.Spec.Requirements = append(p.Spec.Requirements, v1.NodeSelectorRequirement{ | |
| Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}, | |
| }) | |
| } |
There was a problem hiding this comment.
I'm just coping this, but will check out this cleanup.
| - provisioners | ||
| - provisioners/status |
There was a problem hiding this comment.
what about AWSNodeTemplate?
There was a problem hiding this comment.
I want to move this as part of a separate change. These APIs really need to be owned by pkg/cloudprovider/aws, but they're not currently.
…)" This reverts commit 836f854.
…)" This reverts commit 836f854.
…)" This reverts commit 836f854.
Fixes #2052
Description
Separate aws specific webhook logic into a separate webhook that's hosted from the same webhook binary.
How was this change tested?
make testmake applyand manual verificationDoes this change impact docs?
Release Note
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.