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

Add pod level inject webhook #716

Merged
merged 9 commits into from
Aug 28, 2019
108 changes: 108 additions & 0 deletions pkg/webhook/v1alpha2/pod/inject_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
Copyright 2019 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 pod

import (
"context"
"net/http"

logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"

"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"

v1 "k8s.io/api/core/v1"
// "github.com/kubeflow/katib/pkg/webhook/v1alpha2/experiment/injector"
)

// For debug
var log = logf.Log.WithName("injector-webhook")

// sidecarInjector that inject metrics collect sidecar into master pod
type sidecarInjector struct {
client client.Client
decoder types.Decoder
// injector.Injector
}

var _ admission.Handler = &sidecarInjector{}

func (s *sidecarInjector) Handle(ctx context.Context, req types.Request) types.Response {
pod := &v1.Pod{}
err := s.decoder.Decode(req, pod)
if err != nil {
return admission.ErrorResponse(http.StatusBadRequest, err)
}

// Check whether the pod need to be mutated
if !s.MutationRequired(pod) {
log.Info("Skipping mutation for " + pod.Name + " due to policy check")
return admission.ValidationResponse(true, "")
}

// Do mutation
mutatedPod := s.Mutate(pod)
// if err != nil {
// return admission.ErrorResponse(http.StatusBadRequest, err)
// }

return admission.PatchResponse(pod, mutatedPod)
}

var _ inject.Client = &sidecarInjector{}

func (s *sidecarInjector) InjectClient(c client.Client) error {
s.client = c
return nil
}

var _ inject.Decoder = &sidecarInjector{}

func (s *sidecarInjector) InjectDecoder(d types.Decoder) error {
s.decoder = d
return nil
}

func NewSidecarInjector(c client.Client) *sidecarInjector {
return &sidecarInjector{
client: c,
}
}

func (s *sidecarInjector) MutationRequired(pod *v1.Pod) bool {
labels := pod.Labels
for k, v := range labels {
log.Info("FOR TEST: pod label {" + k + ": " + v + "}")
}

return true
}

func (s *sidecarInjector) Mutate(pod *v1.Pod) *v1.Pod {
mutatedPod := pod.DeepCopy()

// Hard code container, just for test
injectContainer := v1.Container{
Name: "sidecar-nginxk",
Image: "nginx:1.12.2",
}
mutatedPod.Spec.Containers = append(mutatedPod.Spec.Containers, injectContainer)

return mutatedPod
}
15 changes: 14 additions & 1 deletion pkg/webhook/v1alpha2/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import (

experimentsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/experiment/v1alpha2"
"github.com/kubeflow/katib/pkg/webhook/v1alpha2/experiment"
"github.com/kubeflow/katib/pkg/webhook/v1alpha2/pod"

admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"

"sigs.k8s.io/controller-runtime/pkg/manager"
Expand Down Expand Up @@ -84,5 +86,16 @@ func register(manager manager.Manager, server *webhook.Server) error {
if err != nil {
return err
}
return server.Register(mutatingWebhook, validatingWebhook)
injectWebhook, err := builder.NewWebhookBuilder().
Name("mutating.pod.kubeflow.org").
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe we can rename it to one which is katib related

Copy link
Member

Choose a reason for hiding this comment

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

Yes, do you have any suggestion about the name?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should follow xx.$group.$domain style (group should be katib and domain is kubeflow.org here), we'd better make all others consistent with this style.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM, it is a historical problem. Can we do the change in another PR? I think we could keep the name here and update it after the PR is merged, with other changes.

Copy link
Member

Choose a reason for hiding this comment

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

sure

Mutating().
Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
WithManager(manager).
ForType(&v1.Pod{}).
Handlers(pod.NewSidecarInjector(manager.GetClient())).
Build()
if err != nil {
return err
}
return server.Register(mutatingWebhook, validatingWebhook, injectWebhook)
}