Skip to content

Set up defaulting webhook #51

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

Merged
merged 10 commits into from
Apr 20, 2023
Merged
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ vet: ## Run go vet against code.

.PHONY: test
test: manifests generate fmt vet envtest ## Run tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./pkg/... -coverprofile cover.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./pkg/... ./api/... -coverprofile cover.out

##@ Build

Expand Down
4 changes: 4 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,8 @@ resources:
kind: JobSet
path: sigs.k8s.io/jobset/api/v1alpha1
version: v1alpha1
webhooks:
defaulting: true
validation: true
webhookVersion: v1
version: "3"
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,19 @@ JobSet: An API for managing a group of Jobs as a unit.

# Installation

To install the CRD and deploy the controller on the cluster selected on your `~/.kubeconfig`, run the following commands:
### Prerequisites
[cert-manager](https://cert-manager.io/) is required to create certificates for the webhook. To install
it on your cluster, run the following command:
```
kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.11.0/cert-manager.yaml
```
See more details about [cert-manager installation](https://cert-manager.io/docs/installation/).

To install the JobSet CRD and deploy the controller on the cluster selected on your `~/.kubeconfig`, run the following commands:
```
git clone https://github.com/kubernetes-sigs/jobset.git
cd jobset

IMAGE_REGISTRY=<registry>/<project> make image-push deploy
```

Expand Down
65 changes: 65 additions & 0 deletions api/v1alpha1/jobset_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"

batchv1 "k8s.io/api/batch/v1"
)

func (r *JobSet) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(r).
Complete()
}

//+kubebuilder:webhook:path=/mutate-batch-x-k8s-io-v1alpha1-jobset,mutating=true,failurePolicy=fail,sideEffects=None,groups=batch.x-k8s.io,resources=jobsets,verbs=create;update,versions=v1alpha1,name=mjobset.kb.io,admissionReviewVersions=v1

var _ webhook.Defaulter = &JobSet{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (r *JobSet) Default() {
// Default job completion mode to indexed.
for i, rjob := range r.Spec.Jobs {
if rjob.Template.Spec.CompletionMode == nil {
r.Spec.Jobs[i].Template.Spec.CompletionMode = completionModePtr(batchv1.IndexedCompletion)
}
}
}

//+kubebuilder:webhook:path=/validate-batch-x-k8s-io-v1alpha1-jobset,mutating=false,failurePolicy=fail,sideEffects=None,groups=batch.x-k8s.io,resources=jobsets,verbs=create;update,versions=v1alpha1,name=vjobset.kb.io,admissionReviewVersions=v1

var _ webhook.Validator = &JobSet{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *JobSet) ValidateCreate() error {
return nil
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *JobSet) ValidateUpdate(old runtime.Object) error {
return nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *JobSet) ValidateDelete() error {
return nil
}

func completionModePtr(mode batchv1.CompletionMode) *batchv1.CompletionMode {
return &mode
}
82 changes: 82 additions & 0 deletions api/v1alpha1/jobset_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package v1alpha1

import (
"testing"

"github.com/google/go-cmp/cmp"
batchv1 "k8s.io/api/batch/v1"
)

func TestJobSetDefaulting(t *testing.T) {
testCases := []struct {
name string
js *JobSet
want *JobSet
}{
{
name: "job completion mode is unset",
js: &JobSet{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not able to use wrappers here because it would create a circular import dependency (testing package imports v1alpha1, so v1alpha1 cannot import testing).

We could consider a refactor to solve this (kueue uses a separate webhooks package and sets up a CustomerDefaulter, which allows for more flexibility, contextual logging, etc. but this would require creating a separate webhook struct rather than adding methods to the JobSet struct to implement the Defaulter interface, and it becomes more complicated and didn't seem like it was worth it for this very simple defaulting we are doing.

Spec: JobSetSpec{
Jobs: []ReplicatedJob{
{
Template: batchv1.JobTemplateSpec{
Spec: batchv1.JobSpec{},
},
},
},
},
},
want: &JobSet{
Spec: JobSetSpec{
Jobs: []ReplicatedJob{
{
Template: batchv1.JobTemplateSpec{
Spec: batchv1.JobSpec{
CompletionMode: completionModePtr(batchv1.IndexedCompletion),
},
},
},
},
},
},
},
{
name: "job completion mode is set to non-indexed",
js: &JobSet{
Spec: JobSetSpec{
Jobs: []ReplicatedJob{
{
Template: batchv1.JobTemplateSpec{
Spec: batchv1.JobSpec{
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
},
},
},
},
},
},
want: &JobSet{
Spec: JobSetSpec{
Jobs: []ReplicatedJob{
{
Template: batchv1.JobTemplateSpec{
Spec: batchv1.JobSpec{
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
},
},
},
},
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tc.js.Default()
if diff := cmp.Diff(tc.want, tc.js); diff != "" {
t.Errorf("unexpected jobset defaulting: (-want/+got): %s", diff)
}
})
}
}
2 changes: 1 addition & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 39 additions & 0 deletions config/certmanager/certificate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# The following manifests contain a self-signed issuer CR and a certificate CR.
# More document can be found at https://docs.cert-manager.io
# WARNING: Targets CertManager v1.0. Check https://cert-manager.io/docs/installation/upgrading/ for breaking changes.
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
labels:
app.kubernetes.io/name: issuer
app.kubernetes.io/instance: selfsigned-issuer
app.kubernetes.io/component: certificate
app.kubernetes.io/created-by: jobset
app.kubernetes.io/part-of: jobset
app.kubernetes.io/managed-by: kustomize
name: selfsigned-issuer
namespace: system
spec:
selfSigned: {}
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
labels:
app.kubernetes.io/name: certificate
app.kubernetes.io/instance: serving-cert
app.kubernetes.io/component: certificate
app.kubernetes.io/created-by: jobset
app.kubernetes.io/part-of: jobset
app.kubernetes.io/managed-by: kustomize
name: serving-cert # this name should match the one appeared in kustomizeconfig.yaml
namespace: system
spec:
# $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize
dnsNames:
- $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc
- $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc.cluster.local
issuerRef:
kind: Issuer
name: selfsigned-issuer
secretName: webhook-server-cert # this secret will not be prefixed, since it's not managed by kustomize
5 changes: 5 additions & 0 deletions config/certmanager/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
resources:
- certificate.yaml

configurations:
- kustomizeconfig.yaml
16 changes: 16 additions & 0 deletions config/certmanager/kustomizeconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# This configuration is for teaching kustomize how to update name ref and var substitution
nameReference:
- kind: Issuer
group: cert-manager.io
fieldSpecs:
- kind: Certificate
group: cert-manager.io
path: spec/issuerRef/name

varReference:
- kind: Certificate
group: cert-manager.io
path: spec/commonName
- kind: Certificate
group: cert-manager.io
path: spec/dnsNames
4 changes: 2 additions & 2 deletions config/crd/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ resources:
patchesStrategicMerge:
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
# patches here are for enabling the conversion webhook for each CRD
#- patches/webhook_in_jobsets.yaml
- patches/webhook_in_jobsets.yaml
#+kubebuilder:scaffold:crdkustomizewebhookpatch

# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
# patches here are for enabling the CA injection for each CRD
#- patches/cainjection_in_jobsets.yaml
- patches/cainjection_in_jobsets.yaml
#+kubebuilder:scaffold:crdkustomizecainjectionpatch

# the following config is for teaching kustomize how to do kustomization for CRDs.
Expand Down
60 changes: 30 additions & 30 deletions config/default/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ bases:
- ../manager
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
#- ../webhook
- ../webhook
# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required.
#- ../certmanager
- ../certmanager
# [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
#- ../prometheus

Expand All @@ -34,39 +34,39 @@ patchesStrategicMerge:

# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
#- manager_webhook_patch.yaml
- manager_webhook_patch.yaml

# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'.
# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks.
# 'CERTMANAGER' needs to be enabled to use ca injection
#- webhookcainjection_patch.yaml
- webhookcainjection_patch.yaml

# the following config is for teaching kustomize how to do var substitution
vars:
# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix.
#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR
# objref:
# kind: Certificate
# group: cert-manager.io
# version: v1
# name: serving-cert # this name should match the one in certificate.yaml
# fieldref:
# fieldpath: metadata.namespace
#- name: CERTIFICATE_NAME
# objref:
# kind: Certificate
# group: cert-manager.io
# version: v1
# name: serving-cert # this name should match the one in certificate.yaml
#- name: SERVICE_NAMESPACE # namespace of the service
# objref:
# kind: Service
# version: v1
# name: webhook-service
# fieldref:
# fieldpath: metadata.namespace
#- name: SERVICE_NAME
# objref:
# kind: Service
# version: v1
# name: webhook-service
- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR
objref:
kind: Certificate
group: cert-manager.io
version: v1
name: serving-cert # this name should match the one in certificate.yaml
fieldref:
fieldpath: metadata.namespace
- name: CERTIFICATE_NAME
objref:
kind: Certificate
group: cert-manager.io
version: v1
name: serving-cert # this name should match the one in certificate.yaml
- name: SERVICE_NAMESPACE # namespace of the service
objref:
kind: Service
version: v1
name: webhook-service
fieldref:
fieldpath: metadata.namespace
- name: SERVICE_NAME
objref:
kind: Service
version: v1
name: webhook-service
23 changes: 23 additions & 0 deletions config/default/manager_webhook_patch.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: controller-manager
namespace: system
spec:
template:
spec:
containers:
- name: manager
ports:
- containerPort: 9443
name: webhook-server
protocol: TCP
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
readOnly: true
volumes:
- name: cert
secret:
defaultMode: 420
secretName: webhook-server-cert
Loading