From 3f9a825d856857ea747e535f30712a1d65eba707 Mon Sep 17 00:00:00 2001 From: Shintaro Murakami Date: Mon, 15 Apr 2019 19:38:47 +0900 Subject: [PATCH 01/11] Pass appropriate context to leaderelector.Run --- pkg/manager/internal.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 60138b904c..1ca1f08d9c 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -309,7 +309,16 @@ func (cm *controllerManager) startLeaderElection() (err error) { return err } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + select { + case <-cm.internalStop: + cancel() + case <-ctx.Done(): + } + }() + // Start the leader elector process - go l.Run(context.Background()) + go l.Run(ctx) return nil } From ce521f8367a3210040d6107848f3389fea81ffa5 Mon Sep 17 00:00:00 2001 From: Andy Bursavich Date: Sun, 19 May 2019 11:49:27 -0700 Subject: [PATCH 02/11] metrics: migrate workqueue provider to v1.14 removing deprecated metrics Migration work was already done to move controller-runtime from k8s v1.13 to v1.14, however this introduced certain issues. Unfortunately, workqueue_queue_duration_seconds and workqueue_work_duration_seconds were included in v0.1.10, but were recorded with microsecond values. Moving to k8s v1.14 corrects those values to seconds. Primarily this change removes the metrics that were deprecated in k8s v1.14, but were never previously included in v0.1 of controller-runtime. The one metric that was deprecated and is retained is longest_running_processor_microseconds, as it was included in v0.1.10. --- pkg/metrics/client_go_adapter.go | 189 ------------------------------- pkg/metrics/workqueue.go | 173 ++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 189 deletions(-) create mode 100644 pkg/metrics/workqueue.go diff --git a/pkg/metrics/client_go_adapter.go b/pkg/metrics/client_go_adapter.go index 3b2c316280..dd9e50b3fd 100644 --- a/pkg/metrics/client_go_adapter.go +++ b/pkg/metrics/client_go_adapter.go @@ -20,23 +20,9 @@ import ( "net/url" "time" - "k8s.io/apimachinery/pkg/util/runtime" - "github.com/prometheus/client_golang/prometheus" reflectormetrics "k8s.io/client-go/tools/cache" clientmetrics "k8s.io/client-go/tools/metrics" - workqueuemetrics "k8s.io/client-go/util/workqueue" -) - -const ( - workQueueSubsystem = "workqueue" - depthKey = "depth" - addsKey = "adds_total" - queueLatencyKey = "queue_duration_seconds" - workDurationKey = "work_duration_seconds" - unfinishedWorkKey = "unfinished_work_seconds" - longestRunningProcessorKey = "longest_running_processor_seconds" - retriesKey = "retries_total" ) // this file contains setup logic to initialize the myriad of places @@ -117,62 +103,11 @@ var ( Name: "last_resource_version", Help: "Last resource version seen for the reflectors", }, []string{"name"}) - - // workqueue metrics - - depth = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Subsystem: workQueueSubsystem, - Name: depthKey, - Help: "Current depth of workqueue", - }, []string{"name"}) - - adds = prometheus.NewCounterVec(prometheus.CounterOpts{ - Subsystem: workQueueSubsystem, - Name: addsKey, - Help: "Total number of adds handled by workqueue", - }, []string{"name"}) - - latency = prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Subsystem: workQueueSubsystem, - Name: queueLatencyKey, - Help: "How long in seconds an item stays in workqueue before being requested.", - Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), - }, []string{"name"}) - - workDuration = prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Subsystem: workQueueSubsystem, - Name: workDurationKey, - Help: "How long in seconds processing an item from workqueue takes.", - Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), - }, []string{"name"}) - - unfinishedWork = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Subsystem: workQueueSubsystem, - Name: unfinishedWorkKey, - Help: "How many seconds of work has done that " + - "is in progress and hasn't been observed by work_duration. Large " + - "values indicate stuck threads. One can deduce the number of stuck " + - "threads by observing the rate at which this increases.", - }, []string{"name"}) - - longestRunning = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Subsystem: workQueueSubsystem, - Name: longestRunningProcessorKey, - Help: "How many seconds has the longest running " + - "processor for workqueue been running.", - }, []string{"name"}) - - retries = prometheus.NewCounterVec(prometheus.CounterOpts{ - Subsystem: workQueueSubsystem, - Name: retriesKey, - Help: "Total number of retries handled by workqueue", - }, []string{"name"}) ) func init() { registerClientMetrics() registerReflectorMetrics() - registerWorkqueueMetrics() } // registerClientMetrics sets up the client latency metrics from client-go @@ -199,19 +134,6 @@ func registerReflectorMetrics() { reflectormetrics.SetReflectorMetricsProvider(reflectorMetricsProvider{}) } -// registerWorkQueueMetrics sets up workqueue (other reconcile) metrics -func registerWorkqueueMetrics() { - Registry.MustRegister(depth) - Registry.MustRegister(adds) - Registry.MustRegister(latency) - Registry.MustRegister(workDuration) - Registry.MustRegister(retries) - Registry.MustRegister(longestRunning) - Registry.MustRegister(unfinishedWork) - - workqueuemetrics.SetProvider(workqueueMetricsProvider{}) -} - // this section contains adapters, implementations, and other sundry organic, artisinally // hand-crafted syntax trees required to convince client-go that it actually wants to let // someone use its metrics. @@ -273,114 +195,3 @@ func (reflectorMetricsProvider) NewItemsInWatchMetric(name string) reflectormetr func (reflectorMetricsProvider) NewLastResourceVersionMetric(name string) reflectormetrics.GaugeMetric { return lastResourceVersion.WithLabelValues(name) } - -// Workqueue metrics (method #3 for client-go metrics), -// copied (more-or-less directly) from k8s.io/kubernetes setup code -// (which isn't anywhere in an easily-importable place). -// TODO(directxman12): stop "cheating" and calling histograms summaries when we pull in the latest deps - -type workqueueMetricsProvider struct{} - -func (workqueueMetricsProvider) NewDepthMetric(name string) workqueuemetrics.GaugeMetric { - return depth.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewAddsMetric(name string) workqueuemetrics.CounterMetric { - return adds.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewLatencyMetric(name string) workqueuemetrics.HistogramMetric { - return latency.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewWorkDurationMetric(name string) workqueuemetrics.HistogramMetric { - return workDuration.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewUnfinishedWorkSecondsMetric(name string) workqueuemetrics.SettableGaugeMetric { - return unfinishedWork.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewLongestRunningProcessorSecondsMetric(name string) workqueuemetrics.SettableGaugeMetric { - return longestRunning.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewRetriesMetric(name string) workqueuemetrics.CounterMetric { - return retries.WithLabelValues(name) -} - -func (workqueueMetricsProvider) NewDeprecatedDepthMetric(name string) workqueuemetrics.GaugeMetric { - depth := prometheus.NewGauge(prometheus.GaugeOpts{ - Subsystem: name, - Name: "depth", - Help: "Current depth of workqueue: " + name, - }) - runtime.HandleError(Registry.Register(depth)) - return depth -} - -func (workqueueMetricsProvider) NewDeprecatedAddsMetric(name string) workqueuemetrics.CounterMetric { - adds := prometheus.NewCounter(prometheus.CounterOpts{ - Subsystem: name, - Name: "adds", - Help: "Total number of adds handled by workqueue: " + name, - }) - runtime.HandleError(Registry.Register(adds)) - return adds -} - -func (workqueueMetricsProvider) NewDeprecatedLatencyMetric(name string) workqueuemetrics.SummaryMetric { - latency := prometheus.NewSummary(prometheus.SummaryOpts{ - Subsystem: name, - Name: "queue_latency", - Help: "How long an item stays in workqueue" + name + " before being requested.", - ConstLabels: prometheus.Labels{"name": name}, - }) - runtime.HandleError(Registry.Register(latency)) - return latency -} - -func (workqueueMetricsProvider) NewDeprecatedWorkDurationMetric(name string) workqueuemetrics.SummaryMetric { - workDuration := prometheus.NewSummary(prometheus.SummaryOpts{ - Subsystem: name, - Name: "work_duration", - Help: "How long processing an item from workqueue" + name + " takes.", - ConstLabels: prometheus.Labels{"name": name}, - }) - runtime.HandleError(Registry.Register(workDuration)) - return workDuration -} - -func (workqueueMetricsProvider) NewDeprecatedUnfinishedWorkSecondsMetric(name string) workqueuemetrics.SettableGaugeMetric { - unfinishedWork := prometheus.NewGauge(prometheus.GaugeOpts{ - Subsystem: name, - Name: "unfinished_work_seconds", - Help: "How many seconds of work " + name + " has done that " + - "is in progress and hasn't been observed by work_duration. Large " + - "values indicate stuck threads. One can deduce the number of stuck " + - "threads by observing the rate at which this increases.", - }) - runtime.HandleError(Registry.Register(unfinishedWork)) - return unfinishedWork -} - -func (workqueueMetricsProvider) NewDeprecatedLongestRunningProcessorMicrosecondsMetric(name string) workqueuemetrics.SettableGaugeMetric { - longestRunning := prometheus.NewGauge(prometheus.GaugeOpts{ - Subsystem: name, - Name: "longest_running_processor_microseconds", - Help: "How many microseconds has the longest running " + - "processor for " + name + " been running.", - }) - runtime.HandleError(Registry.Register(longestRunning)) - return longestRunning -} - -func (workqueueMetricsProvider) NewDeprecatedRetriesMetric(name string) workqueuemetrics.CounterMetric { - retries := prometheus.NewCounter(prometheus.CounterOpts{ - Subsystem: name, - Name: "retries", - Help: "Total number of retries handled by workqueue: " + name, - }) - runtime.HandleError(Registry.Register(retries)) - return retries -} diff --git a/pkg/metrics/workqueue.go b/pkg/metrics/workqueue.go new file mode 100644 index 0000000000..6381f0c14a --- /dev/null +++ b/pkg/metrics/workqueue.go @@ -0,0 +1,173 @@ +/* +Copyright 2018 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 metrics + +import ( + "github.com/prometheus/client_golang/prometheus" + "k8s.io/client-go/util/workqueue" + logf "sigs.k8s.io/controller-runtime/pkg/internal/log" +) + +var log = logf.RuntimeLog.WithName("metrics") + +// This file is copied and adapted from k8s.io/kubernetes/pkg/util/workqueue/prometheus +// which registers metrics to the default prometheus Registry. We require very +// similar functionality, but must register metrics to a different Registry. + +func init() { + workqueue.SetProvider(workqueueMetricsProvider{}) +} + +func registerWorkqueueMetric(c prometheus.Collector, name, queue string) { + if err := Registry.Register(c); err != nil { + log.Error(err, "failed to register metric", "name", name, "queue", queue) + } +} + +type workqueueMetricsProvider struct{} + +func (workqueueMetricsProvider) NewDepthMetric(queue string) workqueue.GaugeMetric { + const name = "workqueue_depth" + m := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: name, + Help: "Current depth of workqueue", + ConstLabels: prometheus.Labels{"name": queue}, + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +func (workqueueMetricsProvider) NewAddsMetric(queue string) workqueue.CounterMetric { + const name = "workqueue_adds_total" + m := prometheus.NewCounter(prometheus.CounterOpts{ + Name: name, + Help: "Total number of adds handled by workqueue", + ConstLabels: prometheus.Labels{"name": queue}, + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +func (workqueueMetricsProvider) NewLatencyMetric(queue string) workqueue.HistogramMetric { + const name = "workqueue_queue_duration_seconds" + m := prometheus.NewHistogram(prometheus.HistogramOpts{ + Name: name, + Help: "How long in seconds an item stays in workqueue before being requested.", + ConstLabels: prometheus.Labels{"name": queue}, + Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +func (workqueueMetricsProvider) NewWorkDurationMetric(queue string) workqueue.HistogramMetric { + const name = "workqueue_work_duration_seconds" + m := prometheus.NewHistogram(prometheus.HistogramOpts{ + Name: name, + Help: "How long in seconds processing an item from workqueue takes.", + ConstLabels: prometheus.Labels{"name": queue}, + Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +func (workqueueMetricsProvider) NewUnfinishedWorkSecondsMetric(queue string) workqueue.SettableGaugeMetric { + const name = "workqueue_unfinished_work_seconds" + m := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: name, + Help: "How many seconds of work has done that " + + "is in progress and hasn't been observed by work_duration. Large " + + "values indicate stuck threads. One can deduce the number of stuck " + + "threads by observing the rate at which this increases.", + ConstLabels: prometheus.Labels{"name": queue}, + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +func (workqueueMetricsProvider) NewLongestRunningProcessorSecondsMetric(queue string) workqueue.SettableGaugeMetric { + const name = "workqueue_longest_running_processor_seconds" + m := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: name, + Help: "How many seconds has the longest running " + + "processor for workqueue been running.", + ConstLabels: prometheus.Labels{"name": queue}, + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +func (workqueueMetricsProvider) NewRetriesMetric(queue string) workqueue.CounterMetric { + const name = "workqueue_retries_total" + m := prometheus.NewCounter(prometheus.CounterOpts{ + Name: name, + Help: "Total number of retries handled by workqueue", + ConstLabels: prometheus.Labels{"name": queue}, + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +// TODO(abursavich): Remove the following deprecated metrics when they are +// removed from k8s.io/client-go/util/workqueue. + +func (workqueueMetricsProvider) NewDeprecatedLongestRunningProcessorMicrosecondsMetric(queue string) workqueue.SettableGaugeMetric { + const name = "workqueue_longest_running_processor_microseconds" + m := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: name, + Help: "(Deprecated) How many microseconds has the longest running " + + "processor for workqueue been running.", + ConstLabels: prometheus.Labels{"name": queue}, + }) + registerWorkqueueMetric(m, name, queue) + return m +} + +// NOTE: The following deprecated metrics are noops because they were never +// included in controller-runtime. + +func (workqueueMetricsProvider) NewDeprecatedDepthMetric(queue string) workqueue.GaugeMetric { + return noopMetric{} +} + +func (workqueueMetricsProvider) NewDeprecatedAddsMetric(queue string) workqueue.CounterMetric { + return noopMetric{} +} + +func (workqueueMetricsProvider) NewDeprecatedLatencyMetric(queue string) workqueue.SummaryMetric { + return noopMetric{} +} + +func (workqueueMetricsProvider) NewDeprecatedWorkDurationMetric(queue string) workqueue.SummaryMetric { + return noopMetric{} +} + +func (workqueueMetricsProvider) NewDeprecatedUnfinishedWorkSecondsMetric(queue string) workqueue.SettableGaugeMetric { + return noopMetric{} +} + +func (workqueueMetricsProvider) NewDeprecatedRetriesMetric(queue string) workqueue.CounterMetric { + return noopMetric{} +} + +type noopMetric struct{} + +func (noopMetric) Inc() {} +func (noopMetric) Dec() {} +func (noopMetric) Set(float64) {} +func (noopMetric) Observe(float64) {} From df54457f6c307b74d8cdf1ed7743bd9b82cb9091 Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Mon, 10 Jun 2019 16:30:20 -0700 Subject: [PATCH 03/11] :book: minor updates to the examples --- examples/builtins/main.go | 13 +++---------- examples/builtins/mutatingwebhook.go | 2 ++ examples/builtins/validatingwebhook.go | 2 ++ examples/crd/pkg/resource.go | 4 ++-- go.mod | 1 + 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/examples/builtins/main.go b/examples/builtins/main.go index 70ce1a43e1..19879ab65c 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -78,18 +78,11 @@ func main() { // Setup webhooks entryLog.Info("setting up webhook server") - hookServer := &webhook.Server{ - Port: 9876, - CertDir: "/tmp/cert", - } - if err := mgr.Add(hookServer); err != nil { - entryLog.Error(err, "unable register webhook server with manager") - os.Exit(1) - } + hookServer := mgr.GetWebhookServer() entryLog.Info("registering webhooks to the webhook server") - hookServer.Register("/mutate-pods", &webhook.Admission{Handler: &podAnnotator{}}) - hookServer.Register("/validate-pods", &webhook.Admission{Handler: &podValidator{}}) + hookServer.Register("/mutate-v1-pod", &webhook.Admission{Handler: &podAnnotator{}}) + hookServer.Register("/validate-v1-pod", &webhook.Admission{Handler: &podValidator{}}) entryLog.Info("starting manager") if err := mgr.Start(signals.SetupSignalHandler()); err != nil { diff --git a/examples/builtins/mutatingwebhook.go b/examples/builtins/mutatingwebhook.go index 56b5cd2a2a..3de24f1a68 100644 --- a/examples/builtins/mutatingwebhook.go +++ b/examples/builtins/mutatingwebhook.go @@ -26,6 +26,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) +// +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.kb.io + // podAnnotator annotates Pods type podAnnotator struct { client client.Client diff --git a/examples/builtins/validatingwebhook.go b/examples/builtins/validatingwebhook.go index e185180e2b..a51e5b2179 100644 --- a/examples/builtins/validatingwebhook.go +++ b/examples/builtins/validatingwebhook.go @@ -26,6 +26,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) +// +kubebuilder:webhook:path=/validate-v1-pod,mutating=false,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=vpod.kb.io + // podValidator validates Pods type podValidator struct { client client.Client diff --git a/examples/crd/pkg/resource.go b/examples/crd/pkg/resource.go index 67bbc57d29..6637303f5b 100644 --- a/examples/crd/pkg/resource.go +++ b/examples/crd/pkg/resource.go @@ -61,7 +61,7 @@ type ChaosPodList struct { Items []ChaosPod `json:"items"` } -// +kubebuilder:webhook:failurePolicy=fail,groups=chaosapps.metamagical.io,resources=chaospods,verbs=create;update,versions=v1,name=vchaospod.kb.io,path=/validate-chaosapps-metamagical-io-v1-chaospod,mutating=false +// +kubebuilder:webhook:path=/validate-chaosapps-metamagical-io-v1-chaospod,mutating=false,failurePolicy=fail,groups=chaosapps.metamagical.io,resources=chaospods,verbs=create;update,versions=v1,name=vchaospod.kb.io var _ webhook.Validator = &ChaosPod{} @@ -93,7 +93,7 @@ func (c *ChaosPod) ValidateUpdate(old runtime.Object) error { return nil } -// +kubebuilder:webhook:failurePolicy=fail,groups=chaosapps.metamagical.io,resources=chaospods,verbs=create;update,versions=v1,name=mchaospod.kb.io,path=/mutate-chaosapps-metamagical-io-v1-chaospod,mutating=true +// +kubebuilder:webhook:path=/mutate-chaosapps-metamagical-io-v1-chaospod,mutating=true,failurePolicy=fail,groups=chaosapps.metamagical.io,resources=chaospods,verbs=create;update,versions=v1,name=mchaospod.kb.io var _ webhook.Defaulter = &ChaosPod{} diff --git a/go.mod b/go.mod index 568ccb60da..c63910855c 100644 --- a/go.mod +++ b/go.mod @@ -38,6 +38,7 @@ require ( golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect google.golang.org/appengine v1.1.0 // indirect gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect + gopkg.in/fsnotify.v1 v1.4.7 gopkg.in/inf.v0 v0.9.1 // indirect k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b k8s.io/apiextensions-apiserver v0.0.0-20190409022649-727a075fdec8 From 0d3fbbc45eddb4b47e33924bffbc5df1b66901ca Mon Sep 17 00:00:00 2001 From: adohe Date: Tue, 11 Jun 2019 15:23:05 +0800 Subject: [PATCH 04/11] :sparkles: make UseExistingCluster a pointer to support explicitly opt-out in the code --- pkg/envtest/server.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/envtest/server.go b/pkg/envtest/server.go index 2cfedd1e06..7c4d8cfff7 100644 --- a/pkg/envtest/server.go +++ b/pkg/envtest/server.go @@ -90,7 +90,7 @@ type Environment struct { // UseExisting indicates that this environments should use an // existing kubeconfig, instead of trying to stand up a new control plane. // This is useful in cases that need aggregated API servers and the like. - UseExistingCluster bool + UseExistingCluster *bool // ControlPlaneStartTimeout is the maximum duration each controlplane component // may take to start. It defaults to the KUBEBUILDER_CONTROLPLANE_START_TIMEOUT @@ -113,7 +113,7 @@ type Environment struct { // Stop stops a running server func (te *Environment) Stop() error { - if te.UseExistingCluster { + if te.useExistingCluster() { return nil } return te.ControlPlane.Stop() @@ -130,11 +130,7 @@ func (te Environment) getAPIServerFlags() []string { // Start starts a local Kubernetes server and updates te.ApiserverPort with the port it is listening on func (te *Environment) Start() (*rest.Config, error) { - if !te.UseExistingCluster { - // Check USE_EXISTING_CLUSTER env then - te.UseExistingCluster = strings.ToLower(os.Getenv(envUseExistingCluster)) == "true" - } - if te.UseExistingCluster { + if te.useExistingCluster() { log.V(1).Info("using existing cluster") if te.Config == nil { // we want to allow people to pass in their own config, so @@ -256,3 +252,10 @@ func (te *Environment) defaultTimeouts() error { } return nil } + +func (te *Environment) useExistingCluster() bool { + if te.UseExistingCluster == nil { + return strings.ToLower(os.Getenv(envUseExistingCluster)) == "true" + } + return *te.UseExistingCluster +} From 3547c6c4f1f86dca33095a9c702a7e9c56e06f5d Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Tue, 11 Jun 2019 16:06:02 -0700 Subject: [PATCH 05/11] :bug: stop registering webhooks with same path when adding multiple controllers If a kind has implemented Defaulter and(or) Validator interface(s), multiple controllers for this kind should be able to be added to the same controller manager. --- pkg/builder/build.go | 35 +++++++++++++++++++++++++++-------- pkg/builder/build_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/pkg/builder/build.go b/pkg/builder/build.go index 487786586b..5bb70e8ceb 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -18,6 +18,8 @@ package builder import ( "fmt" + "net/http" + "net/url" "strings" "k8s.io/apimachinery/pkg/runtime" @@ -280,11 +282,15 @@ func (blder *Builder) doWebhook() error { mwh := admission.DefaultingWebhookFor(defaulter) if mwh != nil { path := generateMutatePath(gvk) - log.Info("Registering a mutating webhook", - "GVK", gvk, - "path", path) - blder.mgr.GetWebhookServer().Register(path, mwh) + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log.Info("Registering a mutating webhook", + "GVK", gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, mwh) + } } } @@ -292,10 +298,15 @@ func (blder *Builder) doWebhook() error { vwh := admission.ValidatingWebhookFor(validator) if vwh != nil { path := generateValidatePath(gvk) - log.Info("Registering a validating webhook", - "GVK", gvk, - "path", path) - blder.mgr.GetWebhookServer().Register(path, vwh) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log.Info("Registering a validating webhook", + "GVK", gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, vwh) + } } } @@ -306,6 +317,14 @@ func (blder *Builder) doWebhook() error { return nil } +func (blder *Builder) isAlreadyHandled(path string) bool { + h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}}) + if p == path && h != nil { + return true + } + return false +} + func generateMutatePath(gvk schema.GroupVersionKind) string { return "/mutate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + gvk.Version + "-" + strings.ToLower(gvk.Kind) diff --git a/pkg/builder/build_test.go b/pkg/builder/build_test.go index 241f51c3f1..545f222a9e 100644 --- a/pkg/builder/build_test.go +++ b/pkg/builder/build_test.go @@ -350,6 +350,34 @@ var _ = Describe("application", func() { Expect(w.Body).To(ContainSubstring(`"allowed":true`)) Expect(w.Body).To(ContainSubstring(`"code":200`)) }) + + It("should allow multiple controllers for the same kind", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()} + builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + Expect(err).NotTo(HaveOccurred()) + + By("creating the 1st controller") + ctrl1, err := ControllerManagedBy(m). + For(&TestDefaultValidator{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(ctrl1).NotTo(BeNil()) + + By("creating the 2nd controller") + ctrl2, err := ControllerManagedBy(m). + For(&TestDefaultValidator{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(ctrl2).NotTo(BeNil()) + }) }) Describe("Start with SimpleController", func() { From 8ad94f4c4bcd1442f5b1086f8bbe4a2a8b69576a Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Wed, 19 Jun 2019 13:17:58 -0700 Subject: [PATCH 06/11] Fix runtime/scheme type aliases pkg/runtime/scheme was supposed to maintain a type alias to make it easier for people to upgrade and/or interact with projects written around older versions, but apparently, I forgot to actually leave the type alias around. This fixes that. --- pkg/runtime/scheme/scheme.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/runtime/scheme/scheme.go b/pkg/runtime/scheme/scheme.go index 889308c1e0..5b6c9465bf 100644 --- a/pkg/runtime/scheme/scheme.go +++ b/pkg/runtime/scheme/scheme.go @@ -20,3 +20,10 @@ limitations under the License. // // Deprecated: use pkg/scheme instead. package scheme + +import ( + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +// Builder builds a new Scheme for mapping go types to Kubernetes GroupVersionKinds. +type Builder = scheme.Builder From 4c27c48a5b5b534fe9df5336f6b738fc1314af0d Mon Sep 17 00:00:00 2001 From: ialidzhikov Date: Mon, 24 Jun 2019 00:11:12 +0300 Subject: [PATCH 07/11] Fix typo Signed-off-by: ialidzhikov --- .gitignore | 3 +++ pkg/client/client_test.go | 20 ++++++++++---------- pkg/client/options.go | 2 -- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/.gitignore b/.gitignore index c25057f685..f749874ce0 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,6 @@ *.swp *.swo *~ + +# Vscode files +.vscode diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index ad96f4ccb4..c492aec7a0 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -475,7 +475,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -578,7 +578,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -672,7 +672,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -791,7 +791,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -860,7 +860,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -956,7 +956,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1024,7 +1024,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1136,7 +1136,7 @@ var _ = Describe("Client", func() { err = cl.Patch(context.TODO(), u, client.ConstantPatch(types.MergePatchType, mergePatch)) Expect(err).NotTo(HaveOccurred()) - By("validating pathed Node has new annotation") + By("validating patched Node has new annotation") actual, err := clientset.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) @@ -1240,7 +1240,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1345,7 +1345,7 @@ var _ = Describe("Client", func() { close(done) }) - It("should fail if the object does not exists", func(done Done) { + It("should fail if the object does not exist", func(done Done) { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) diff --git a/pkg/client/options.go b/pkg/client/options.go index 46ddf66f97..395ce7838f 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -101,7 +101,6 @@ type DeleteOptions struct { // AsDeleteOptions returns these options as a metav1.DeleteOptions. // This may mutate the Raw field. func (o *DeleteOptions) AsDeleteOptions() *metav1.DeleteOptions { - if o == nil { return &metav1.DeleteOptions{} } @@ -309,7 +308,6 @@ type UpdateOptions struct { // AsUpdateOptions returns these options as a metav1.UpdateOptions. // This may mutate the Raw field. func (o *UpdateOptions) AsUpdateOptions() *metav1.UpdateOptions { - if o == nil { return &metav1.UpdateOptions{} } From cc64c08bf2fd6c8e9c3a2c0f9b63da4598827c81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=83=AD=E7=BB=B4?= Date: Tue, 25 Jun 2019 19:14:05 +0800 Subject: [PATCH 08/11] update jsonpatch --- Gopkg.lock | 18 +++++++++--------- go.mod | 5 ++--- go.sum | 10 ++++------ pkg/webhook/admission/multi.go | 2 +- pkg/webhook/admission/multi_test.go | 2 +- pkg/webhook/admission/response.go | 2 +- pkg/webhook/admission/response_test.go | 2 +- pkg/webhook/admission/webhook.go | 2 +- pkg/webhook/admission/webhook_test.go | 2 +- pkg/webhook/alias.go | 2 +- 10 files changed, 22 insertions(+), 25 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index dbd73bbbc9..5907ab15d1 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -40,14 +40,6 @@ pruneopts = "UT" revision = "de5bf2ad457846296e2031421a34e2568e304e35" -[[projects]] - digest = "1:464aef731a5f82ded547c62e249a2e9ec59fbbc9ddab53cda7b9857852630a61" - name = "github.com/appscode/jsonpatch" - packages = ["."] - pruneopts = "UT" - revision = "7c0e3b262f30165a8ec3d0b4c6059fd92703bfb2" - version = "1.0.0" - [[projects]] branch = "master" digest = "1:d6afaeed1502aa28e80a4ed0981d570ad91b2579193404256ce672ed0a609e0d" @@ -524,6 +516,14 @@ pruneopts = "UT" revision = "fbb02b2291d28baffd63558aa44b4b56f178d650" +[[projects]] + digest = "1:7d4fd782f2a710d08834b2c01742b3bba7fb0248383f9cc6c3dc95b3025689d7" + name = "gomodules.xyz/jsonpatch" + packages = ["v2"] + pruneopts = "UT" + revision = "e8422f09d27ee2c8cfb2c7f8089eb9eeb0764849" + version = "v2.0.0" + [[projects]] digest = "1:c8907869850adaa8bd7631887948d0684f3787d0912f1c01ab72581a6c34432e" name = "google.golang.org/appengine" @@ -912,7 +912,6 @@ analyzer-name = "dep" analyzer-version = 1 input-imports = [ - "github.com/appscode/jsonpatch", "github.com/emicklei/go-restful", "github.com/evanphx/json-patch", "github.com/go-logr/logr", @@ -930,6 +929,7 @@ "go.uber.org/zap", "go.uber.org/zap/buffer", "go.uber.org/zap/zapcore", + "gomodules.xyz/jsonpatch/v2", "gopkg.in/fsnotify.v1", "k8s.io/api/admission/v1beta1", "k8s.io/api/apps/v1", diff --git a/go.mod b/go.mod index c63910855c..928622f224 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,8 @@ go 1.11 require ( cloud.google.com/go v0.26.0 // indirect - github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect - github.com/evanphx/json-patch v4.1.0+incompatible + github.com/evanphx/json-patch v4.5.0+incompatible github.com/go-logr/logr v0.1.0 github.com/go-logr/zapr v0.1.0 github.com/gogo/protobuf v1.1.1 // indirect @@ -23,7 +22,6 @@ require ( github.com/onsi/ginkgo v1.6.0 github.com/onsi/gomega v1.4.2 github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c // indirect - github.com/pkg/errors v0.8.1 // indirect github.com/prometheus/client_golang v0.9.0 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910 github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e // indirect @@ -36,6 +34,7 @@ require ( golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be // indirect golang.org/x/sync v0.0.0-20190423024810-112230192c58 // indirect golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect + gomodules.xyz/jsonpatch/v2 v2.0.0 google.golang.org/appengine v1.1.0 // indirect gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect gopkg.in/fsnotify.v1 v1.4.7 diff --git a/go.sum b/go.sum index b5e1bdd0c3..fa3b3a5209 100644 --- a/go.sum +++ b/go.sum @@ -1,15 +1,12 @@ cloud.google.com/go v0.26.0 h1:e0WKqKTd5BnrG8aKH3J3h+QvEIQtSUcf2n5UZ5ZgLtQ= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= -github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30 h1:Kn3rqvbUFqSepE2OqVu0Pn1CbDw9IuMlONapol0zuwk= -github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30/go.mod h1:4AJxUpXUhv4N+ziTvIcWWXgeorXpxPZOfk9HdEVr96M= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 h1:xJ4a3vCFaGF/jqvzLMYoU8P317H5OQ+Via4RmuPwCS0= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/evanphx/json-patch v4.0.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= -github.com/evanphx/json-patch v4.1.0+incompatible h1:K1MDoo4AZ4wU0GIU/fPmtZg7VpzLjCxu+UwBD1FvwOc= -github.com/evanphx/json-patch v4.1.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= +github.com/evanphx/json-patch v4.5.0+incompatible h1:ouOWdg56aJriqS0huScTkVXPC5IcNrDCXZ6OoTAWu7M= +github.com/evanphx/json-patch v4.5.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/go-logr/logr v0.1.0 h1:M1Tv3VzNlEHg6uyACnRdtrploV2P7wZqH8BoQMtz0cg= @@ -68,7 +65,6 @@ github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTd github.com/spf13/pflag v1.0.2 h1:Fy0orTDgHdbnzHcsOgfCN4LtHf0ec3wwtiwJqwvf3Gc= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4= @@ -92,6 +88,8 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2IVY3KZs6p9mix0ziNYJM= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= +gomodules.xyz/jsonpatch/v2 v2.0.0 h1:lHNQverf0+Gm1TbSbVIDWVXOhZ2FpZopxRqpr2uIjs4= +gomodules.xyz/jsonpatch/v2 v2.0.0/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/pkg/webhook/admission/multi.go b/pkg/webhook/admission/multi.go index a65be69f68..2ffdb57b00 100644 --- a/pkg/webhook/admission/multi.go +++ b/pkg/webhook/admission/multi.go @@ -22,7 +22,7 @@ import ( "fmt" "net/http" - "github.com/appscode/jsonpatch" + "gomodules.xyz/jsonpatch/v2" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" diff --git a/pkg/webhook/admission/multi_test.go b/pkg/webhook/admission/multi_test.go index 8501e62f62..b6a8bb2b9c 100644 --- a/pkg/webhook/admission/multi_test.go +++ b/pkg/webhook/admission/multi_test.go @@ -22,7 +22,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/appscode/jsonpatch" + "gomodules.xyz/jsonpatch/v2" admissionv1beta1 "k8s.io/api/admission/v1beta1" ) diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go index bb625ed0d3..da8e336788 100644 --- a/pkg/webhook/admission/response.go +++ b/pkg/webhook/admission/response.go @@ -19,7 +19,7 @@ package admission import ( "net/http" - "github.com/appscode/jsonpatch" + "gomodules.xyz/jsonpatch/v2" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/webhook/admission/response_test.go b/pkg/webhook/admission/response_test.go index eb7ec832b6..accf3d6b26 100644 --- a/pkg/webhook/admission/response_test.go +++ b/pkg/webhook/admission/response_test.go @@ -20,9 +20,9 @@ import ( "errors" "net/http" - "github.com/appscode/jsonpatch" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "gomodules.xyz/jsonpatch/v2" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/webhook/admission/webhook.go b/pkg/webhook/admission/webhook.go index 2f6cd9a88f..9804aa9ba0 100644 --- a/pkg/webhook/admission/webhook.go +++ b/pkg/webhook/admission/webhook.go @@ -21,8 +21,8 @@ import ( "errors" "net/http" - "github.com/appscode/jsonpatch" "github.com/go-logr/logr" + "gomodules.xyz/jsonpatch/v2" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" diff --git a/pkg/webhook/admission/webhook_test.go b/pkg/webhook/admission/webhook_test.go index 2097414de0..89d890e49c 100644 --- a/pkg/webhook/admission/webhook_test.go +++ b/pkg/webhook/admission/webhook_test.go @@ -23,7 +23,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/appscode/jsonpatch" + "gomodules.xyz/jsonpatch/v2" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index 27ed60ad18..276784efb2 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -17,7 +17,7 @@ limitations under the License. package webhook import ( - "github.com/appscode/jsonpatch" + "gomodules.xyz/jsonpatch/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) From d36a00e29f9cf452f31cf890e20611c4878acd8f Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Mon, 24 Jun 2019 14:24:08 -0700 Subject: [PATCH 09/11] :warning: split the webhook builder out as a separate builder. Controller builder now only build controller; while webhook builder only build webhook. --- alias.go | 3 + examples/crd/main.go | 9 +- pkg/builder/{build.go => controller.go} | 94 +----- pkg/builder/controller_test.go | 310 ++++++++++++++++++ pkg/builder/example_webhook_test.go | 61 ++++ pkg/builder/webhook.go | 150 +++++++++ .../{build_test.go => webhook_test.go} | 281 +--------------- 7 files changed, 541 insertions(+), 367 deletions(-) rename pkg/builder/{build.go => controller.go} (69%) create mode 100644 pkg/builder/controller_test.go create mode 100644 pkg/builder/example_webhook_test.go create mode 100644 pkg/builder/webhook.go rename pkg/builder/{build_test.go => webhook_test.go} (58%) diff --git a/alias.go b/alias.go index b41f51435b..af955ad301 100644 --- a/alias.go +++ b/alias.go @@ -94,6 +94,9 @@ var ( // NewControllerManagedBy returns a new controller builder that will be started by the provided Manager NewControllerManagedBy = builder.ControllerManagedBy + // NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager + NewWebhookManagedBy = builder.WebhookManagedBy + // NewManager returns a new Manager for creating Controllers. NewManager = manager.New diff --git a/examples/crd/main.go b/examples/crd/main.go index 82dcd75c5d..86339a535e 100644 --- a/examples/crd/main.go +++ b/examples/crd/main.go @@ -125,12 +125,19 @@ func main() { Client: mgr.GetClient(), scheme: mgr.GetScheme(), }) - if err != nil { setupLog.Error(err, "unable to create controller") os.Exit(1) } + err = ctrl.NewWebhookManagedBy(mgr). + For(&api.ChaosPod{}). + Complete() + if err != nil { + setupLog.Error(err, "unable to create webhook") + os.Exit(1) + } + setupLog.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") diff --git a/pkg/builder/build.go b/pkg/builder/controller.go similarity index 69% rename from pkg/builder/build.go rename to pkg/builder/controller.go index 5bb70e8ceb..d89f3696ff 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/controller.go @@ -18,12 +18,9 @@ package builder import ( "fmt" - "net/http" - "net/url" "strings" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/client/config" @@ -33,8 +30,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" ) // Supporting mocking out functions for testing @@ -71,8 +66,6 @@ func ControllerManagedBy(m manager.Manager) *Builder { // update events by *reconciling the object*. // This is the equivalent of calling // Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) -// If the passed in object has implemented the admission.Defaulter interface, a MutatingWebhook will be wired for this type. -// If the passed in object has implemented the admission.Validator interface, a ValidatingWebhook will be wired for this type. // // Deprecated: Use For func (blder *Builder) ForType(apiType runtime.Object) *Builder { @@ -83,8 +76,6 @@ func (blder *Builder) ForType(apiType runtime.Object) *Builder { // update events by *reconciling the object*. // This is the equivalent of calling // Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) -// If the passed in object has implemented the admission.Defaulter interface, a MutatingWebhook will be wired for this type. -// If the passed in object has implemented the admission.Validator interface, a ValidatingWebhook will be wired for this type. func (blder *Builder) For(apiType runtime.Object) *Builder { blder.apiType = apiType return blder @@ -159,7 +150,7 @@ func (blder *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { } // Set the Config - if err := blder.doConfig(); err != nil { + if err := blder.loadRestConfig(); err != nil { return nil, err } @@ -173,11 +164,6 @@ func (blder *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { return nil, err } - // Set the Webook if needed - if err := blder.doWebhook(); err != nil { - return nil, err - } - // Set the Watch if err := blder.doWatch(); err != nil { return nil, err @@ -217,7 +203,7 @@ func (blder *Builder) doWatch() error { return nil } -func (blder *Builder) doConfig() error { +func (blder *Builder) loadRestConfig() error { if blder.config != nil { return nil } @@ -258,79 +244,3 @@ func (blder *Builder) doController(r reconcile.Reconciler) error { blder.ctrl, err = newController(name, blder.mgr, controller.Options{Reconciler: r}) return err } - -func (blder *Builder) doWebhook() error { - // Create a webhook for each type - gvk, err := apiutil.GVKForObject(blder.apiType, blder.mgr.GetScheme()) - if err != nil { - return err - } - - // TODO: When the conversion webhook lands, we need to handle all registered versions of a given group-kind. - // A potential workflow for defaulting webhook - // 1) a bespoke (non-hub) version comes in - // 2) convert it to the hub version - // 3) do defaulting - // 4) convert it back to the same bespoke version - // 5) calculate the JSON patch - // - // A potential workflow for validating webhook - // 1) a bespoke (non-hub) version comes in - // 2) convert it to the hub version - // 3) do validation - if defaulter, isDefaulter := blder.apiType.(admission.Defaulter); isDefaulter { - mwh := admission.DefaultingWebhookFor(defaulter) - if mwh != nil { - path := generateMutatePath(gvk) - - // Checking if the path is already registered. - // If so, just skip it. - if !blder.isAlreadyHandled(path) { - log.Info("Registering a mutating webhook", - "GVK", gvk, - "path", path) - blder.mgr.GetWebhookServer().Register(path, mwh) - } - } - } - - if validator, isValidator := blder.apiType.(admission.Validator); isValidator { - vwh := admission.ValidatingWebhookFor(validator) - if vwh != nil { - path := generateValidatePath(gvk) - - // Checking if the path is already registered. - // If so, just skip it. - if !blder.isAlreadyHandled(path) { - log.Info("Registering a validating webhook", - "GVK", gvk, - "path", path) - blder.mgr.GetWebhookServer().Register(path, vwh) - } - } - } - - err = conversion.CheckConvertibility(blder.mgr.GetScheme(), blder.apiType) - if err != nil { - log.Error(err, "conversion check failed", "GVK", gvk) - } - return nil -} - -func (blder *Builder) isAlreadyHandled(path string) bool { - h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}}) - if p == path && h != nil { - return true - } - return false -} - -func generateMutatePath(gvk schema.GroupVersionKind) string { - return "/mutate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + - gvk.Version + "-" + strings.ToLower(gvk.Kind) -} - -func generateValidatePath(gvk schema.GroupVersionKind) string { - return "/validate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + - gvk.Version + "-" + strings.ToLower(gvk.Kind) -} diff --git a/pkg/builder/controller_test.go b/pkg/builder/controller_test.go new file mode 100644 index 0000000000..29917edbe5 --- /dev/null +++ b/pkg/builder/controller_test.go @@ -0,0 +1,310 @@ +/* +Copyright 2018 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 builder + +import ( + "context" + "fmt" + "strings" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/scheme" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +var _ = Describe("application", func() { + var stop chan struct{} + + BeforeEach(func() { + stop = make(chan struct{}) + getConfig = func() (*rest.Config, error) { return cfg, nil } + newController = controller.New + newManager = manager.New + }) + + AfterEach(func() { + close(stop) + }) + + noop := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { return reconcile.Result{}, nil }) + + Describe("New", func() { + It("should return success if given valid objects", func() { + instance, err := SimpleController(). + For(&appsv1.ReplicaSet{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(instance).NotTo(BeNil()) + }) + + It("should return an error if the Config is invalid", func() { + getConfig = func() (*rest.Config, error) { return cfg, fmt.Errorf("expected error") } + instance, err := SimpleController(). + For(&appsv1.ReplicaSet{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected error")) + Expect(instance).To(BeNil()) + }) + + It("should return an error if there is no GVK for an object", func() { + instance, err := SimpleController(). + For(&fakeType{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no kind is registered for the type builder.fakeType")) + Expect(instance).To(BeNil()) + + instance, err = SimpleController(). + For(&appsv1.ReplicaSet{}). + Owns(&fakeType{}). + Build(noop) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no kind is registered for the type builder.fakeType")) + Expect(instance).To(BeNil()) + }) + + It("should return an error if it cannot create the manager", func() { + newManager = func(config *rest.Config, options manager.Options) (manager.Manager, error) { + return nil, fmt.Errorf("expected error") + } + instance, err := SimpleController(). + For(&appsv1.ReplicaSet{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected error")) + Expect(instance).To(BeNil()) + }) + + It("should return an error if it cannot create the controller", func() { + newController = func(name string, mgr manager.Manager, options controller.Options) ( + controller.Controller, error) { + return nil, fmt.Errorf("expected error") + } + instance, err := SimpleController(). + For(&appsv1.ReplicaSet{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected error")) + Expect(instance).To(BeNil()) + }) + + It("should allow multiple controllers for the same kind", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()} + builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + Expect(err).NotTo(HaveOccurred()) + + By("creating the 1st controller") + ctrl1, err := ControllerManagedBy(m). + For(&TestDefaultValidator{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(ctrl1).NotTo(BeNil()) + + By("creating the 2nd controller") + ctrl2, err := ControllerManagedBy(m). + For(&TestDefaultValidator{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(ctrl2).NotTo(BeNil()) + }) + }) + + Describe("Start with SimpleController", func() { + It("should Reconcile Owns objects", func(done Done) { + bldr := SimpleController(). + ForType(&appsv1.Deployment{}). + WithConfig(cfg). + Owns(&appsv1.ReplicaSet{}) + doReconcileTest("1", stop, bldr, nil, false) + + close(done) + }, 10) + + It("should Reconcile Owns objects with a Manager", func(done Done) { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + bldr := SimpleController(). + WithManager(m). + For(&appsv1.Deployment{}). + Owns(&appsv1.ReplicaSet{}) + doReconcileTest("2", stop, bldr, m, false) + close(done) + }, 10) + }) + + Describe("Start with ControllerManagedBy", func() { + It("should Reconcile Owns objects", func(done Done) { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + bldr := ControllerManagedBy(m). + For(&appsv1.Deployment{}). + Owns(&appsv1.ReplicaSet{}) + doReconcileTest("3", stop, bldr, m, false) + close(done) + }, 10) + + It("should Reconcile Watches objects", func(done Done) { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + bldr := ControllerManagedBy(m). + For(&appsv1.Deployment{}). + Watches( // Equivalent of Owns + &source.Kind{Type: &appsv1.ReplicaSet{}}, + &handler.EnqueueRequestForOwner{OwnerType: &appsv1.Deployment{}, IsController: true}) + doReconcileTest("4", stop, bldr, m, true) + close(done) + }, 10) + }) +}) + +func doReconcileTest(nameSuffix string, stop chan struct{}, blder *Builder, mgr manager.Manager, complete bool) { + deployName := "deploy-name-" + nameSuffix + rsName := "rs-name-" + nameSuffix + + By("Creating the application") + ch := make(chan reconcile.Request) + fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { + defer GinkgoRecover() + if !strings.HasSuffix(req.Name, nameSuffix) { + // From different test, ignore this request. Etcd is shared across tests. + return reconcile.Result{}, nil + } + ch <- req + return reconcile.Result{}, nil + }) + + instance := mgr + if complete { + err := blder.Complete(fn) + Expect(err).NotTo(HaveOccurred()) + } else { + var err error + instance, err = blder.Build(fn) + Expect(err).NotTo(HaveOccurred()) + } + + // Manager should match + if mgr != nil { + Expect(instance).To(Equal(mgr)) + } + + By("Starting the application") + go func() { + defer GinkgoRecover() + Expect(instance.Start(stop)).NotTo(HaveOccurred()) + By("Stopping the application") + }() + + By("Creating a Deployment") + // Expect a Reconcile when the Deployment is managedObjects. + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: deployName, + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"foo": "bar"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "nginx", + Image: "nginx", + }, + }, + }, + }, + }, + } + err := instance.GetClient().Create(context.TODO(), dep) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for the Deployment Reconcile") + Expect(<-ch).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})) + + By("Creating a ReplicaSet") + // Expect a Reconcile when an Owned object is managedObjects. + t := true + rs := &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: rsName, + Labels: dep.Spec.Selector.MatchLabels, + OwnerReferences: []metav1.OwnerReference{ + { + Name: deployName, + Kind: "Deployment", + APIVersion: "apps/v1", + Controller: &t, + UID: dep.UID, + }, + }, + }, + Spec: appsv1.ReplicaSetSpec{ + Selector: dep.Spec.Selector, + Template: dep.Spec.Template, + }, + } + err = instance.GetClient().Create(context.TODO(), rs) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for the ReplicaSet Reconcile") + Expect(<-ch).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})) + +} + +var _ runtime.Object = &fakeType{} + +type fakeType struct{} + +func (*fakeType) GetObjectKind() schema.ObjectKind { return nil } +func (*fakeType) DeepCopyObject() runtime.Object { return nil } diff --git a/pkg/builder/example_webhook_test.go b/pkg/builder/example_webhook_test.go new file mode 100644 index 0000000000..63333a2478 --- /dev/null +++ b/pkg/builder/example_webhook_test.go @@ -0,0 +1,61 @@ +/* +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 builder_test + +import ( + "os" + + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client/config" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/manager/signals" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + examplegroup "sigs.k8s.io/controller-runtime/examples/crd/pkg" +) + +// examplegroup.ChaosPod has implemented both admission.Defaulter and +// admission.Validator interfaces. +var _ admission.Defaulter = &examplegroup.ChaosPod{} +var _ admission.Validator = &examplegroup.ChaosPod{} + +// This example use webhook builder to create a simple webhook that is managed +// by a manager for CRD ChaosPod. And then start the manager. +func ExampleWebhookBuilder() { + var log = logf.Log.WithName("webhookbuilder-example") + + mgr, err := manager.New(config.GetConfigOrDie(), manager.Options{}) + if err != nil { + log.Error(err, "could not create manager") + os.Exit(1) + } + + err = builder. + WebhookManagedBy(mgr). // Create the WebhookManagedBy + For(&examplegroup.ChaosPod{}). // ChaosPod is a CRD. + Complete() + if err != nil { + log.Error(err, "could not create webhook") + os.Exit(1) + } + + if err := mgr.Start(signals.SetupSignalHandler()); err != nil { + log.Error(err, "could not start manager") + os.Exit(1) + } +} diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go new file mode 100644 index 0000000000..d807552a02 --- /dev/null +++ b/pkg/builder/webhook.go @@ -0,0 +1,150 @@ +/* +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 builder + +import ( + "net/http" + "net/url" + "strings" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" +) + +// WebhookBuilder builds a Webhook. +type WebhookBuilder struct { + apiType runtime.Object + gvk schema.GroupVersionKind + mgr manager.Manager + config *rest.Config +} + +func WebhookManagedBy(m manager.Manager) *WebhookBuilder { + return &WebhookBuilder{mgr: m} +} + +// TODO(droot): update the GoDoc for conversion. + +// For takes a runtime.Object which should be a CR. +// If the given object implements the admission.Defaulter interface, a MutatingWebhook will be wired for this type. +// If the given object implements the admission.Validator interface, a ValidatingWebhook will be wired for this type. +func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder { + blder.apiType = apiType + return blder +} + +// Complete builds the webhook. +func (blder *WebhookBuilder) Complete() error { + // Set the Config + if err := blder.loadRestConfig(); err != nil { + return err + } + + // Set the Webook if needed + return blder.registerWebhooks() +} + +func (blder *WebhookBuilder) loadRestConfig() error { + if blder.config != nil { + return nil + } + if blder.mgr != nil { + blder.config = blder.mgr.GetConfig() + return nil + } + var err error + blder.config, err = getConfig() + return err +} + +func (blder *WebhookBuilder) registerWebhooks() error { + // Create webhook(s) for each type + var err error + blder.gvk, err = apiutil.GVKForObject(blder.apiType, blder.mgr.GetScheme()) + if err != nil { + return err + } + + blder.registerDefaultingWebhook() + blder.registerValidatingWebhook() + + err = conversion.CheckConvertibility(blder.mgr.GetScheme(), blder.apiType) + if err != nil { + log.Error(err, "conversion check failed", "GVK", blder.gvk) + } + return nil +} + +// registerDefaultingWebhook registers a defaulting webhook if th +func (blder *WebhookBuilder) registerDefaultingWebhook() { + if defaulter, isDefaulter := blder.apiType.(admission.Defaulter); isDefaulter { + mwh := admission.DefaultingWebhookFor(defaulter) + if mwh != nil { + path := generateMutatePath(blder.gvk) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log.Info("Registering a mutating webhook", + "GVK", blder.gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, mwh) + } + } + } +} + +func (blder *WebhookBuilder) registerValidatingWebhook() { + if validator, isValidator := blder.apiType.(admission.Validator); isValidator { + vwh := admission.ValidatingWebhookFor(validator) + if vwh != nil { + path := generateValidatePath(blder.gvk) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log.Info("Registering a validating webhook", + "GVK", blder.gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, vwh) + } + } + } +} + +func (blder *WebhookBuilder) isAlreadyHandled(path string) bool { + h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}}) + if p == path && h != nil { + return true + } + return false +} + +func generateMutatePath(gvk schema.GroupVersionKind) string { + return "/mutate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + + gvk.Version + "-" + strings.ToLower(gvk.Kind) +} + +func generateValidatePath(gvk schema.GroupVersionKind) string { + return "/validate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + + gvk.Version + "-" + strings.ToLower(gvk.Kind) +} diff --git a/pkg/builder/build_test.go b/pkg/builder/webhook_test.go similarity index 58% rename from pkg/builder/build_test.go rename to pkg/builder/webhook_test.go index 545f222a9e..9554ecbced 100644 --- a/pkg/builder/build_test.go +++ b/pkg/builder/webhook_test.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +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. @@ -17,7 +17,6 @@ limitations under the License. package builder import ( - "context" "errors" "fmt" "net/http" @@ -27,19 +26,12 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/scheme" - "sigs.k8s.io/controller-runtime/pkg/source" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -57,74 +49,7 @@ var _ = Describe("application", func() { close(stop) }) - noop := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { return reconcile.Result{}, nil }) - Describe("New", func() { - It("should return success if given valid objects", func() { - instance, err := SimpleController(). - For(&appsv1.ReplicaSet{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).NotTo(HaveOccurred()) - Expect(instance).NotTo(BeNil()) - }) - - It("should return an error if the Config is invalid", func() { - getConfig = func() (*rest.Config, error) { return cfg, fmt.Errorf("expected error") } - instance, err := SimpleController(). - For(&appsv1.ReplicaSet{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("expected error")) - Expect(instance).To(BeNil()) - }) - - It("should return an error if there is no GVK for an object", func() { - instance, err := SimpleController(). - For(&fakeType{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("no kind is registered for the type builder.fakeType")) - Expect(instance).To(BeNil()) - - instance, err = SimpleController(). - For(&appsv1.ReplicaSet{}). - Owns(&fakeType{}). - Build(noop) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("no kind is registered for the type builder.fakeType")) - Expect(instance).To(BeNil()) - }) - - It("should return an error if it cannot create the manager", func() { - newManager = func(config *rest.Config, options manager.Options) (manager.Manager, error) { - return nil, fmt.Errorf("expected error") - } - instance, err := SimpleController(). - For(&appsv1.ReplicaSet{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("expected error")) - Expect(instance).To(BeNil()) - }) - - It("should return an error if it cannot create the controller", func() { - newController = func(name string, mgr manager.Manager, options controller.Options) ( - controller.Controller, error) { - return nil, fmt.Errorf("expected error") - } - instance, err := SimpleController(). - For(&appsv1.ReplicaSet{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("expected error")) - Expect(instance).To(BeNil()) - }) - It("should scaffold a defaulting webhook if the type implements the Defaulter interface", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) @@ -136,12 +61,10 @@ var _ = Describe("application", func() { err = builder.AddToScheme(m.GetScheme()) Expect(err).NotTo(HaveOccurred()) - instance, err := ControllerManagedBy(m). + err = WebhookManagedBy(m). For(&TestDefaulter{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) + Complete() Expect(err).NotTo(HaveOccurred()) - Expect(instance).NotTo(BeNil()) svr := m.GetWebhookServer() Expect(svr).NotTo(BeNil()) @@ -210,12 +133,10 @@ var _ = Describe("application", func() { err = builder.AddToScheme(m.GetScheme()) Expect(err).NotTo(HaveOccurred()) - instance, err := ControllerManagedBy(m). + err = WebhookManagedBy(m). For(&TestValidator{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) + Complete() Expect(err).NotTo(HaveOccurred()) - Expect(instance).NotTo(BeNil()) svr := m.GetWebhookServer() Expect(svr).NotTo(BeNil()) @@ -285,12 +206,10 @@ var _ = Describe("application", func() { err = builder.AddToScheme(m.GetScheme()) Expect(err).NotTo(HaveOccurred()) - instance, err := ControllerManagedBy(m). + err = WebhookManagedBy(m). For(&TestDefaultValidator{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) + Complete() Expect(err).NotTo(HaveOccurred()) - Expect(instance).NotTo(BeNil()) svr := m.GetWebhookServer() Expect(svr).NotTo(BeNil()) @@ -350,195 +269,9 @@ var _ = Describe("application", func() { Expect(w.Body).To(ContainSubstring(`"allowed":true`)) Expect(w.Body).To(ContainSubstring(`"code":200`)) }) - - It("should allow multiple controllers for the same kind", func() { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - Expect(err).NotTo(HaveOccurred()) - - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()} - builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - Expect(err).NotTo(HaveOccurred()) - - By("creating the 1st controller") - ctrl1, err := ControllerManagedBy(m). - For(&TestDefaultValidator{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).NotTo(HaveOccurred()) - Expect(ctrl1).NotTo(BeNil()) - - By("creating the 2nd controller") - ctrl2, err := ControllerManagedBy(m). - For(&TestDefaultValidator{}). - Owns(&appsv1.ReplicaSet{}). - Build(noop) - Expect(err).NotTo(HaveOccurred()) - Expect(ctrl2).NotTo(BeNil()) - }) - }) - - Describe("Start with SimpleController", func() { - It("should Reconcile Owns objects", func(done Done) { - bldr := SimpleController(). - ForType(&appsv1.Deployment{}). - WithConfig(cfg). - Owns(&appsv1.ReplicaSet{}) - doReconcileTest("1", stop, bldr, nil, false) - - close(done) - }, 10) - - It("should Reconcile Owns objects with a Manager", func(done Done) { - m, err := manager.New(cfg, manager.Options{}) - Expect(err).NotTo(HaveOccurred()) - - bldr := SimpleController(). - WithManager(m). - For(&appsv1.Deployment{}). - Owns(&appsv1.ReplicaSet{}) - doReconcileTest("2", stop, bldr, m, false) - close(done) - }, 10) - }) - - Describe("Start with ControllerManagedBy", func() { - It("should Reconcile Owns objects", func(done Done) { - m, err := manager.New(cfg, manager.Options{}) - Expect(err).NotTo(HaveOccurred()) - - bldr := ControllerManagedBy(m). - For(&appsv1.Deployment{}). - Owns(&appsv1.ReplicaSet{}) - doReconcileTest("3", stop, bldr, m, false) - close(done) - }, 10) - - It("should Reconcile Watches objects", func(done Done) { - m, err := manager.New(cfg, manager.Options{}) - Expect(err).NotTo(HaveOccurred()) - - bldr := ControllerManagedBy(m). - For(&appsv1.Deployment{}). - Watches( // Equivalent of Owns - &source.Kind{Type: &appsv1.ReplicaSet{}}, - &handler.EnqueueRequestForOwner{OwnerType: &appsv1.Deployment{}, IsController: true}) - doReconcileTest("4", stop, bldr, m, true) - close(done) - }, 10) }) }) -func doReconcileTest(nameSuffix string, stop chan struct{}, blder *Builder, mgr manager.Manager, complete bool) { - deployName := "deploy-name-" + nameSuffix - rsName := "rs-name-" + nameSuffix - - By("Creating the application") - ch := make(chan reconcile.Request) - fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { - defer GinkgoRecover() - if !strings.HasSuffix(req.Name, nameSuffix) { - // From different test, ignore this request. Etcd is shared across tests. - return reconcile.Result{}, nil - } - ch <- req - return reconcile.Result{}, nil - }) - - instance := mgr - if complete { - err := blder.Complete(fn) - Expect(err).NotTo(HaveOccurred()) - } else { - var err error - instance, err = blder.Build(fn) - Expect(err).NotTo(HaveOccurred()) - } - - // Manager should match - if mgr != nil { - Expect(instance).To(Equal(mgr)) - } - - By("Starting the application") - go func() { - defer GinkgoRecover() - Expect(instance.Start(stop)).NotTo(HaveOccurred()) - By("Stopping the application") - }() - - By("Creating a Deployment") - // Expect a Reconcile when the Deployment is managedObjects. - dep := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: deployName, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"foo": "bar"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "nginx", - Image: "nginx", - }, - }, - }, - }, - }, - } - err := instance.GetClient().Create(context.TODO(), dep) - Expect(err).NotTo(HaveOccurred()) - - By("Waiting for the Deployment Reconcile") - Expect(<-ch).To(Equal(reconcile.Request{ - NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})) - - By("Creating a ReplicaSet") - // Expect a Reconcile when an Owned object is managedObjects. - t := true - rs := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: rsName, - Labels: dep.Spec.Selector.MatchLabels, - OwnerReferences: []metav1.OwnerReference{ - { - Name: deployName, - Kind: "Deployment", - APIVersion: "apps/v1", - Controller: &t, - UID: dep.UID, - }, - }, - }, - Spec: appsv1.ReplicaSetSpec{ - Selector: dep.Spec.Selector, - Template: dep.Spec.Template, - }, - } - err = instance.GetClient().Create(context.TODO(), rs) - Expect(err).NotTo(HaveOccurred()) - - By("Waiting for the ReplicaSet Reconcile") - Expect(<-ch).To(Equal(reconcile.Request{ - NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})) - -} - -var _ runtime.Object = &fakeType{} - -type fakeType struct{} - -func (*fakeType) GetObjectKind() schema.ObjectKind { return nil } -func (*fakeType) DeepCopyObject() runtime.Object { return nil } - // TestDefaulter var _ runtime.Object = &TestDefaulter{} From 37842afde8145260f859f5d64b9a5be6ee8dc37a Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Thu, 27 Jun 2019 11:52:05 -0700 Subject: [PATCH 10/11] :running: fix go modules checksum mismatch --- go.sum | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go.sum b/go.sum index fa3b3a5209..e092650230 100644 --- a/go.sum +++ b/go.sum @@ -60,7 +60,6 @@ github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e h1:n/3MEhJQjQxrO github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273 h1:agujYaXJSxSo18YNX3jzl+4G6Bstwt+kqv47GS12uL0= github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= -github.com/spf13/afero v1.2.2 h1:5jhuqJyZCZf2JRofRvN/nIFgIWNzPa3/Vz8mYylgbWc= github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/pflag v1.0.2 h1:Fy0orTDgHdbnzHcsOgfCN4LtHf0ec3wwtiwJqwvf3Gc= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= @@ -88,7 +87,7 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2IVY3KZs6p9mix0ziNYJM= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= -gomodules.xyz/jsonpatch/v2 v2.0.0 h1:lHNQverf0+Gm1TbSbVIDWVXOhZ2FpZopxRqpr2uIjs4= +gomodules.xyz/jsonpatch/v2 v2.0.0 h1:OyHbl+7IOECpPKfVK42oFr6N7+Y2dR+Jsb/IiDV3hOo= gomodules.xyz/jsonpatch/v2 v2.0.0/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= From 0f2e574d471454928fafffcaaafeed581ef89aae Mon Sep 17 00:00:00 2001 From: Sunil Arora Date: Thu, 27 Jun 2019 13:23:59 -0700 Subject: [PATCH 11/11] :sparkles: enable conversion webhook in webhook builder Main changes: - Disable conversion webhook by default and enable only if a type implements convertible - Enhanced the test coverage to include spoke to spoke conversion --- .../pkg/apis/addtoscheme_jobs_v1.go | 25 ---- .../pkg/apis/addtoscheme_jobs_v2.go | 25 ---- examples/conversion/pkg/apis/apis.go | 32 ---- examples/conversion/pkg/apis/jobs/v1/doc.go | 22 --- examples/conversion/pkg/apis/jobs/v2/doc.go | 22 --- pkg/builder/webhook.go | 23 ++- pkg/builder/webhook_test.go | 24 ++- pkg/manager/internal.go | 2 - pkg/webhook/conversion/conversion.go | 48 +++--- pkg/webhook/conversion/conversion_test.go | 139 +++++++++++++++--- pkg/webhook/conversion/testdata/.gitignore | 24 +++ pkg/webhook/conversion/testdata/Makefile | 64 ++++++++ pkg/webhook/conversion/testdata/PROJECT | 13 ++ .../testdata/api}/v1/externaljob_types.go | 10 +- .../testdata/api/v1/groupversion_info.go | 22 +-- .../testdata/api}/v1/zz_generated.deepcopy.go | 7 +- .../testdata/api}/v2/externaljob_types.go | 6 +- .../testdata/api/v2/groupversion_info.go | 22 +-- .../testdata/api}/v2/zz_generated.deepcopy.go | 7 +- .../testdata/api/v3/externaljob_types.go | 92 ++++++++++++ .../testdata/api/v3/groupversion_info.go | 35 +++++ .../testdata/api/v3/zz_generated.deepcopy.go | 113 ++++++++++++++ .../testdata/hack/boilerplate.go.txt | 5 +- pkg/webhook/conversion/testdata/main.go | 72 +++++++++ 24 files changed, 629 insertions(+), 225 deletions(-) delete mode 100644 examples/conversion/pkg/apis/addtoscheme_jobs_v1.go delete mode 100644 examples/conversion/pkg/apis/addtoscheme_jobs_v2.go delete mode 100644 examples/conversion/pkg/apis/apis.go delete mode 100644 examples/conversion/pkg/apis/jobs/v1/doc.go delete mode 100644 examples/conversion/pkg/apis/jobs/v2/doc.go create mode 100644 pkg/webhook/conversion/testdata/.gitignore create mode 100644 pkg/webhook/conversion/testdata/Makefile create mode 100644 pkg/webhook/conversion/testdata/PROJECT rename {examples/conversion/pkg/apis/jobs => pkg/webhook/conversion/testdata/api}/v1/externaljob_types.go (91%) rename examples/conversion/pkg/apis/jobs/v1/register.go => pkg/webhook/conversion/testdata/api/v1/groupversion_info.go (54%) rename {examples/conversion/pkg/apis/jobs => pkg/webhook/conversion/testdata/api}/v1/zz_generated.deepcopy.go (97%) rename {examples/conversion/pkg/apis/jobs => pkg/webhook/conversion/testdata/api}/v2/externaljob_types.go (92%) rename examples/conversion/pkg/apis/jobs/v2/register.go => pkg/webhook/conversion/testdata/api/v2/groupversion_info.go (54%) rename {examples/conversion/pkg/apis/jobs => pkg/webhook/conversion/testdata/api}/v2/zz_generated.deepcopy.go (97%) create mode 100644 pkg/webhook/conversion/testdata/api/v3/externaljob_types.go create mode 100644 pkg/webhook/conversion/testdata/api/v3/groupversion_info.go create mode 100644 pkg/webhook/conversion/testdata/api/v3/zz_generated.deepcopy.go rename examples/conversion/pkg/apis/jobs/group.go => pkg/webhook/conversion/testdata/hack/boilerplate.go.txt (89%) create mode 100644 pkg/webhook/conversion/testdata/main.go diff --git a/examples/conversion/pkg/apis/addtoscheme_jobs_v1.go b/examples/conversion/pkg/apis/addtoscheme_jobs_v1.go deleted file mode 100644 index eca32666cc..0000000000 --- a/examples/conversion/pkg/apis/addtoscheme_jobs_v1.go +++ /dev/null @@ -1,25 +0,0 @@ -/* - -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 apis - -import ( - "sigs.k8s.io/controller-runtime/examples/conversion/pkg/apis/jobs/v1" -) - -func init() { - // Register the types with the Scheme so the components can map objects to GroupVersionKinds and back - AddToSchemes = append(AddToSchemes, v1.SchemeBuilder.AddToScheme) -} diff --git a/examples/conversion/pkg/apis/addtoscheme_jobs_v2.go b/examples/conversion/pkg/apis/addtoscheme_jobs_v2.go deleted file mode 100644 index 769e79230a..0000000000 --- a/examples/conversion/pkg/apis/addtoscheme_jobs_v2.go +++ /dev/null @@ -1,25 +0,0 @@ -/* - -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 apis - -import ( - "sigs.k8s.io/controller-runtime/examples/conversion/pkg/apis/jobs/v2" -) - -func init() { - // Register the types with the Scheme so the components can map objects to GroupVersionKinds and back - AddToSchemes = append(AddToSchemes, v2.SchemeBuilder.AddToScheme) -} diff --git a/examples/conversion/pkg/apis/apis.go b/examples/conversion/pkg/apis/apis.go deleted file mode 100644 index 15ccbf9c8a..0000000000 --- a/examples/conversion/pkg/apis/apis.go +++ /dev/null @@ -1,32 +0,0 @@ -/* - -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. -*/ - -// Generate deepcopy for apis -//go:generate go run ../../vendor/k8s.io/code-generator/cmd/deepcopy-gen/main.go -O zz_generated.deepcopy -i ./... -h ../../hack/boilerplate.go.txt - -// Package apis contains Kubernetes API groups. -package apis - -import ( - "k8s.io/apimachinery/pkg/runtime" -) - -// AddToSchemes may be used to add all resources defined in the project to a Scheme -var AddToSchemes runtime.SchemeBuilder - -// AddToScheme adds all Resources to the Scheme -func AddToScheme(s *runtime.Scheme) error { - return AddToSchemes.AddToScheme(s) -} diff --git a/examples/conversion/pkg/apis/jobs/v1/doc.go b/examples/conversion/pkg/apis/jobs/v1/doc.go deleted file mode 100644 index 6b06db1687..0000000000 --- a/examples/conversion/pkg/apis/jobs/v1/doc.go +++ /dev/null @@ -1,22 +0,0 @@ -/* - -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 v1 contains API Schema definitions for the jobs v1 API group -// +k8s:openapi-gen=true -// +k8s:deepcopy-gen=package,register -// +k8s:conversion-gen=github.com/droot/crd-conversion-example/pkg/apis/jobs -// +k8s:defaulter-gen=TypeMeta -// +groupName=jobs.example.org -package v1 diff --git a/examples/conversion/pkg/apis/jobs/v2/doc.go b/examples/conversion/pkg/apis/jobs/v2/doc.go deleted file mode 100644 index 2a1052c95e..0000000000 --- a/examples/conversion/pkg/apis/jobs/v2/doc.go +++ /dev/null @@ -1,22 +0,0 @@ -/* - -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 v2 contains API Schema definitions for the jobs v2 API group -// +k8s:openapi-gen=true -// +k8s:deepcopy-gen=package,register -// +k8s:conversion-gen=github.com/droot/crd-conversion-example/pkg/apis/jobs -// +k8s:defaulter-gen=TypeMeta -// +groupName=jobs.example.org -package v2 diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index d807552a02..84c59d0edf 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -87,9 +87,9 @@ func (blder *WebhookBuilder) registerWebhooks() error { blder.registerDefaultingWebhook() blder.registerValidatingWebhook() - err = conversion.CheckConvertibility(blder.mgr.GetScheme(), blder.apiType) + err = blder.registerConversionWebhook() if err != nil { - log.Error(err, "conversion check failed", "GVK", blder.gvk) + return err } return nil } @@ -131,7 +131,26 @@ func (blder *WebhookBuilder) registerValidatingWebhook() { } } +func (blder *WebhookBuilder) registerConversionWebhook() error { + ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType) + if err != nil { + log.Error(err, "conversion check failed", "object", blder.apiType) + return err + } + if ok { + if !blder.isAlreadyHandled("/convert") { + blder.mgr.GetWebhookServer().Register("/convert", &conversion.Webhook{}) + } + log.Info("conversion webhook enabled", "object", blder.apiType) + } + + return nil +} + func (blder *WebhookBuilder) isAlreadyHandled(path string) bool { + if blder.mgr.GetWebhookServer().WebhookMux == nil { + return false + } h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}}) if p == path && h != nil { return true diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 9554ecbced..79f0e79d90 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -281,13 +281,19 @@ type TestDefaulter struct { var testDefaulterGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaulter"} -func (*TestDefaulter) GetObjectKind() schema.ObjectKind { return nil } +func (d *TestDefaulter) GetObjectKind() schema.ObjectKind { return d } func (d *TestDefaulter) DeepCopyObject() runtime.Object { return &TestDefaulter{ Replica: d.Replica, } } +func (d *TestDefaulter) GroupVersionKind() schema.GroupVersionKind { + return testDefaulterGVK +} + +func (d *TestDefaulter) SetGroupVersionKind(gvk schema.GroupVersionKind) {} + var _ runtime.Object = &TestDefaulterList{} type TestDefaulterList struct{} @@ -310,13 +316,19 @@ type TestValidator struct { var testValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestValidator"} -func (*TestValidator) GetObjectKind() schema.ObjectKind { return nil } +func (v *TestValidator) GetObjectKind() schema.ObjectKind { return v } func (v *TestValidator) DeepCopyObject() runtime.Object { return &TestValidator{ Replica: v.Replica, } } +func (v *TestValidator) GroupVersionKind() schema.GroupVersionKind { + return testValidatorGVK +} + +func (v *TestValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {} + var _ runtime.Object = &TestValidatorList{} type TestValidatorList struct{} @@ -354,13 +366,19 @@ type TestDefaultValidator struct { var testDefaultValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaultValidator"} -func (*TestDefaultValidator) GetObjectKind() schema.ObjectKind { return nil } +func (dv *TestDefaultValidator) GetObjectKind() schema.ObjectKind { return dv } func (dv *TestDefaultValidator) DeepCopyObject() runtime.Object { return &TestDefaultValidator{ Replica: dv.Replica, } } +func (dv *TestDefaultValidator) GroupVersionKind() schema.GroupVersionKind { + return testDefaultValidatorGVK +} + +func (dv *TestDefaultValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {} + var _ runtime.Object = &TestDefaultValidatorList{} type TestDefaultValidatorList struct{} diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 652ba3080f..19857bdd03 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -38,7 +38,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/recorder" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" ) const ( @@ -218,7 +217,6 @@ func (cm *controllerManager) GetWebhookServer() *webhook.Server { Port: cm.port, Host: cm.host, } - cm.webhookServer.Register("/convert", &conversion.Webhook{}) if err := cm.Add(cm.webhookServer); err != nil { panic("unable to add webhookServer to the controller manager") } diff --git a/pkg/webhook/conversion/conversion.go b/pkg/webhook/conversion/conversion.go index 7a74a14892..f9bd5cb2db 100644 --- a/pkg/webhook/conversion/conversion.go +++ b/pkg/webhook/conversion/conversion.go @@ -174,23 +174,24 @@ func (wh *Webhook) convertViaHub(src, dst conversion.Convertible) error { // getHub returns an instance of the Hub for passed-in object's group/kind. func (wh *Webhook) getHub(obj runtime.Object) (conversion.Hub, error) { - gvks, _, err := wh.scheme.ObjectKinds(obj) - if err != nil { - return nil, fmt.Errorf("error retriving object kinds for given object : %v", err) + gvks := objectGVKs(wh.scheme, obj) + if len(gvks) == 0 { + return nil, fmt.Errorf("error retrieving gvks for object : %v", obj) } var hub conversion.Hub - var isHub, hubFoundAlready bool + var hubFoundAlready bool for _, gvk := range gvks { instance, err := wh.scheme.New(gvk) if err != nil { return nil, fmt.Errorf("failed to allocate an instance for gvk %v %v", gvk, err) } - if hub, isHub = instance.(conversion.Hub); isHub { + if val, isHub := instance.(conversion.Hub); isHub { if hubFoundAlready { return nil, fmt.Errorf("multiple hub version defined for %T", obj) } hubFoundAlready = true + hub = val } } return hub, nil @@ -216,21 +217,21 @@ func (wh *Webhook) allocateDstObject(apiVersion, kind string) (runtime.Object, e return obj, nil } -// CheckConvertibility determines if given type is convertible or not. For a type +// IsConvertible determines if given type is convertible or not. For a type // to be convertible, the group-kind needs to have a Hub type defined and all // non-hub types must be able to convert to/from Hub. -func CheckConvertibility(scheme *runtime.Scheme, obj runtime.Object) error { +func IsConvertible(scheme *runtime.Scheme, obj runtime.Object) (bool, error) { var hubs, spokes, nonSpokes []runtime.Object - gvks, _, err := scheme.ObjectKinds(obj) - if err != nil { - return fmt.Errorf("error retriving object kinds for given object : %v", err) + gvks := objectGVKs(scheme, obj) + if len(gvks) == 0 { + return false, fmt.Errorf("error retrieving gvks for object : %v", obj) } for _, gvk := range gvks { instance, err := scheme.New(gvk) if err != nil { - return fmt.Errorf("failed to allocate an instance for gvk %v %v", gvk, err) + return false, fmt.Errorf("failed to allocate an instance for gvk %v %v", gvk, err) } if isHub(instance) { @@ -247,13 +248,13 @@ func CheckConvertibility(scheme *runtime.Scheme, obj runtime.Object) error { } if len(gvks) == 1 { - return nil // single version + return false, nil // single version } if len(hubs) == 0 && len(spokes) == 0 { // multiple version detected with no conversion implementation. This is // true for multi-version built-in types. - return nil + return false, nil } if len(hubs) == 1 && len(nonSpokes) == 0 { // convertible @@ -261,18 +262,31 @@ func CheckConvertibility(scheme *runtime.Scheme, obj runtime.Object) error { for _, sp := range spokes { spokeVersions = append(spokeVersions, sp.GetObjectKind().GroupVersionKind().String()) } - log.V(1).Info("conversion enabled for kind", "kind", - gvks[0].GroupKind(), "hub", hubs[0], "spokes", spokeVersions) - return nil + return true, nil } - return PartialImplementationError{ + return false, PartialImplementationError{ hubs: hubs, nonSpokes: nonSpokes, spokes: spokes, } } +// objectGVKs returns all (Group,Version,Kind) for the Group/Kind of given object. +func objectGVKs(scheme *runtime.Scheme, obj runtime.Object) []schema.GroupVersionKind { + var gvks []schema.GroupVersionKind + + objGVK := obj.GetObjectKind().GroupVersionKind() + knownTypes := scheme.AllKnownTypes() + + for gvk := range knownTypes { + if objGVK.GroupKind() == gvk.GroupKind() { + gvks = append(gvks, gvk) + } + } + return gvks +} + // PartialImplementationError represents an error due to partial conversion // implementation such as hub without spokes, multiple hubs or spokes without hub. type PartialImplementationError struct { diff --git a/pkg/webhook/conversion/conversion_test.go b/pkg/webhook/conversion/conversion_test.go index 6b08a1ae03..1f2e1dc0e3 100644 --- a/pkg/webhook/conversion/conversion_test.go +++ b/pkg/webhook/conversion/conversion_test.go @@ -32,9 +32,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" kscheme "k8s.io/client-go/kubernetes/scheme" - jobsapis "sigs.k8s.io/controller-runtime/examples/conversion/pkg/apis" - jobsv1 "sigs.k8s.io/controller-runtime/examples/conversion/pkg/apis/jobs/v1" - jobsv2 "sigs.k8s.io/controller-runtime/examples/conversion/pkg/apis/jobs/v2" + jobsv1 "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata/api/v1" + jobsv2 "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata/api/v2" + jobsv3 "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata/api/v3" ) var _ = Describe("Conversion Webhook", func() { @@ -49,8 +49,11 @@ var _ = Describe("Conversion Webhook", func() { Body: bytes.NewBuffer(nil), } - scheme = kscheme.Scheme - Expect(jobsapis.AddToScheme(scheme)).To(Succeed()) + scheme = runtime.NewScheme() + Expect(kscheme.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv1.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv2.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) Expect(webhook.InjectScheme(scheme)).To(Succeed()) var err error @@ -77,7 +80,7 @@ var _ = Describe("Conversion Webhook", func() { return &jobsv1.ExternalJob{ TypeMeta: metav1.TypeMeta{ Kind: "ExternalJob", - APIVersion: "jobs.example.org/v1", + APIVersion: "jobs.testprojects.kb.io/v1", }, ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -89,14 +92,30 @@ var _ = Describe("Conversion Webhook", func() { } } - It("should convert objects successfully", func() { + makeV2Obj := func() *jobsv2.ExternalJob { + return &jobsv2.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v2", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv2.ExternalJobSpec{ + ScheduleAt: "every 2 seconds", + }, + } + } + + It("should convert spoke to hub successfully", func() { v1Obj := makeV1Obj() expected := &jobsv2.ExternalJob{ TypeMeta: metav1.TypeMeta{ Kind: "ExternalJob", - APIVersion: "jobs.example.org/v2", + APIVersion: "jobs.testprojects.kb.io/v2", }, ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -110,7 +129,85 @@ var _ = Describe("Conversion Webhook", func() { convReq := &apix.ConversionReview{ TypeMeta: metav1.TypeMeta{}, Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.example.org/v2", + DesiredAPIVersion: "jobs.testprojects.kb.io/v2", + Objects: []runtime.RawExtension{ + { + Object: v1Obj, + }, + }, + }, + } + + convReview := doRequest(convReq) + + Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) + Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) + got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(expected)) + }) + + It("should convert hub to spoke successfully", func() { + + v2Obj := makeV2Obj() + + expected := &jobsv1.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv1.ExternalJobSpec{ + RunAt: "every 2 seconds", + }, + } + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v1", + Objects: []runtime.RawExtension{ + { + Object: v2Obj, + }, + }, + }, + } + + convReview := doRequest(convReq) + + Expect(convReview.Response.ConvertedObjects).To(HaveLen(1)) + Expect(convReview.Response.Result.Status).To(Equal(metav1.StatusSuccess)) + got, _, err := decoder.Decode(convReview.Response.ConvertedObjects[0].Raw) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(expected)) + }) + + It("should convert spoke to spoke successfully", func() { + + v1Obj := makeV1Obj() + + expected := &jobsv3.ExternalJob{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExternalJob", + APIVersion: "jobs.testprojects.kb.io/v3", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "obj-1", + }, + Spec: jobsv3.ExternalJobSpec{ + DeferredAt: "every 2 seconds", + }, + } + + convReq := &apix.ConversionReview{ + TypeMeta: metav1.TypeMeta{}, + Request: &apix.ConversionRequest{ + DesiredAPIVersion: "jobs.testprojects.kb.io/v3", Objects: []runtime.RawExtension{ { Object: v1Obj, @@ -156,7 +253,7 @@ var _ = Describe("Conversion Webhook", func() { convReq := &apix.ConversionReview{ TypeMeta: metav1.TypeMeta{}, Request: &apix.ConversionRequest{ - DesiredAPIVersion: "jobs.example.org/v1", + DesiredAPIVersion: "jobs.testprojects.kb.io/v1", Objects: []runtime.RawExtension{ { Object: v1Obj, @@ -202,30 +299,33 @@ var _ = Describe("Conversion Webhook", func() { }) -var _ = Describe("Convertibility Check", func() { +var _ = Describe("IsConvertible", func() { var scheme *runtime.Scheme BeforeEach(func() { + scheme = runtime.NewScheme() - scheme = kscheme.Scheme - Expect(jobsapis.AddToScheme(scheme)).To(Succeed()) - + Expect(kscheme.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv1.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv2.AddToScheme(scheme)).To(Succeed()) + Expect(jobsv3.AddToScheme(scheme)).To(Succeed()) }) - It("should not return error for convertible types", func() { + It("should return true for convertible types", func() { obj := &jobsv2.ExternalJob{ TypeMeta: metav1.TypeMeta{ Kind: "ExternalJob", - APIVersion: "jobs.example.org/v2", + APIVersion: "jobs.testprojects.kb.io/v2", }, } - err := CheckConvertibility(scheme, obj) + ok, err := IsConvertible(scheme, obj) Expect(err).NotTo(HaveOccurred()) + Expect(ok).To(BeTrue()) }) - It("should not return error for a built-in multi-version type", func() { + It("should return false for a non convertible type", func() { obj := &appsv1beta1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: "Deployment", @@ -233,7 +333,8 @@ var _ = Describe("Convertibility Check", func() { }, } - err := CheckConvertibility(scheme, obj) + ok, err := IsConvertible(scheme, obj) Expect(err).NotTo(HaveOccurred()) + Expect(ok).ToNot(BeTrue()) }) }) diff --git a/pkg/webhook/conversion/testdata/.gitignore b/pkg/webhook/conversion/testdata/.gitignore new file mode 100644 index 0000000000..d97ffc5159 --- /dev/null +++ b/pkg/webhook/conversion/testdata/.gitignore @@ -0,0 +1,24 @@ + +# Binaries for programs and plugins +*.exe +*.exe~ +*.dll +*.so +*.dylib +bin + +# Test binary, build with `go test -c` +*.test + +# Output of the go coverage tool, specifically when used with LiteIDE +*.out + +# Kubernetes Generated files - skip generated files, except for vendored files + +!vendor/**/zz_generated.* + +# editor and IDE paraphernalia +.idea +*.swp +*.swo +*~ diff --git a/pkg/webhook/conversion/testdata/Makefile b/pkg/webhook/conversion/testdata/Makefile new file mode 100644 index 0000000000..2d9d3dda15 --- /dev/null +++ b/pkg/webhook/conversion/testdata/Makefile @@ -0,0 +1,64 @@ + +# Image URL to use all building/pushing image targets +IMG ?= controller:latest +# Produce CRDs that work back to Kubernetes 1.11 (no version conversion) +CRD_OPTIONS ?= "crd:trivialVersions=true" + +all: manager + +# Run tests +test: generate fmt vet manifests + go test ./api/... ./controllers/... -coverprofile cover.out + +# Build manager binary +manager: generate fmt vet + go build -o bin/manager main.go + +# Run against the configured Kubernetes cluster in ~/.kube/config +run: generate fmt vet + go run ./main.go + +# Install CRDs into a cluster +install: manifests + kubectl apply -f config/crd/bases + +# Deploy controller in the configured Kubernetes cluster in ~/.kube/config +deploy: manifests + kubectl apply -f config/crd/bases + kustomize build config/default | kubectl apply -f - + +# Generate manifests e.g. CRD, RBAC etc. +manifests: controller-gen + $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases + +# Run go fmt against code +fmt: + go fmt ./... + +# Run go vet against code +vet: + go vet ./... + +# Generate code +generate: controller-gen + $(CONTROLLER_GEN) object:headerFile=./hack/boilerplate.go.txt paths=./api/... + +# Build the docker image +docker-build: test + docker build . -t ${IMG} + @echo "updating kustomize image patch file for manager resource" + sed -i'' -e 's@image: .*@image: '"${IMG}"'@' ./config/default/manager_image_patch.yaml + +# Push the docker image +docker-push: + docker push ${IMG} + +# find or download controller-gen +# download controller-gen if necessary +controller-gen: +ifeq (, $(shell which controller-gen)) + go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.0-beta.2 +CONTROLLER_GEN=$(shell go env GOPATH)/bin/controller-gen +else +CONTROLLER_GEN=$(shell which controller-gen) +endif diff --git a/pkg/webhook/conversion/testdata/PROJECT b/pkg/webhook/conversion/testdata/PROJECT new file mode 100644 index 0000000000..6b168dcbc1 --- /dev/null +++ b/pkg/webhook/conversion/testdata/PROJECT @@ -0,0 +1,13 @@ +version: "2" +domain: testprojects.kb.io +repo: sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata +resources: +- group: jobs + version: v1 + kind: ExternalJob +- group: jobs + version: v2 + kind: ExternalJob +- group: jobs + version: v3 + kind: ExternalJob diff --git a/examples/conversion/pkg/apis/jobs/v1/externaljob_types.go b/pkg/webhook/conversion/testdata/api/v1/externaljob_types.go similarity index 91% rename from examples/conversion/pkg/apis/jobs/v1/externaljob_types.go rename to pkg/webhook/conversion/testdata/api/v1/externaljob_types.go index dcb137084d..bf99e2a204 100644 --- a/examples/conversion/pkg/apis/jobs/v1/externaljob_types.go +++ b/pkg/webhook/conversion/testdata/api/v1/externaljob_types.go @@ -17,10 +17,10 @@ package v1 import ( "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/examples/conversion/pkg/apis/jobs/v2" "sigs.k8s.io/controller-runtime/pkg/conversion" + + v2 "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata/api/v2" ) // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! @@ -39,11 +39,9 @@ type ExternalJobStatus struct { // Important: Run "make" to regenerate code after modifying this file } -// +genclient -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:object:root=true // ExternalJob is the Schema for the externaljobs API -// +k8s:openapi-gen=true type ExternalJob struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -52,7 +50,7 @@ type ExternalJob struct { Status ExternalJobStatus `json:"status,omitempty"` } -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:object:root=true // ExternalJobList contains a list of ExternalJob type ExternalJobList struct { diff --git a/examples/conversion/pkg/apis/jobs/v1/register.go b/pkg/webhook/conversion/testdata/api/v1/groupversion_info.go similarity index 54% rename from examples/conversion/pkg/apis/jobs/v1/register.go rename to pkg/webhook/conversion/testdata/api/v1/groupversion_info.go index 8181537beb..5bbef61786 100644 --- a/examples/conversion/pkg/apis/jobs/v1/register.go +++ b/pkg/webhook/conversion/testdata/api/v1/groupversion_info.go @@ -13,14 +13,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -// NOTE: Boilerplate only. Ignore this file. - // Package v1 contains API Schema definitions for the jobs v1 API group -// +k8s:openapi-gen=true -// +k8s:deepcopy-gen=package,register -// +k8s:conversion-gen=github.com/droot/crd-conversion-example/pkg/apis/jobs -// +k8s:defaulter-gen=TypeMeta -// +groupName=jobs.example.org +// +kubebuilder:object:generate=true +// +groupName=jobs.testprojects.kb.io package v1 import ( @@ -29,17 +24,12 @@ import ( ) var ( - // SchemeGroupVersion is group version used to register these objects - SchemeGroupVersion = schema.GroupVersion{Group: "jobs.example.org", Version: "v1"} + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "jobs.testprojects.kb.io", Version: "v1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion} + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} - // AddToScheme is required by pkg/client/... + // AddToScheme adds the types in this group-version to the given scheme. AddToScheme = SchemeBuilder.AddToScheme ) - -// Resource is required by pkg/client/listers/... -func Resource(resource string) schema.GroupResource { - return SchemeGroupVersion.WithResource(resource).GroupResource() -} diff --git a/examples/conversion/pkg/apis/jobs/v1/zz_generated.deepcopy.go b/pkg/webhook/conversion/testdata/api/v1/zz_generated.deepcopy.go similarity index 97% rename from examples/conversion/pkg/apis/jobs/v1/zz_generated.deepcopy.go rename to pkg/webhook/conversion/testdata/api/v1/zz_generated.deepcopy.go index dc2e649724..7208ba8c69 100644 --- a/examples/conversion/pkg/apis/jobs/v1/zz_generated.deepcopy.go +++ b/pkg/webhook/conversion/testdata/api/v1/zz_generated.deepcopy.go @@ -14,7 +14,8 @@ 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. */ -// Code generated by main. DO NOT EDIT. + +// autogenerated by controller-gen object, do not modify manually package v1 @@ -29,7 +30,6 @@ func (in *ExternalJob) DeepCopyInto(out *ExternalJob) { in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec out.Status = in.Status - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJob. @@ -62,7 +62,6 @@ func (in *ExternalJobList) DeepCopyInto(out *ExternalJobList) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobList. @@ -86,7 +85,6 @@ func (in *ExternalJobList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalJobSpec) DeepCopyInto(out *ExternalJobSpec) { *out = *in - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobSpec. @@ -102,7 +100,6 @@ func (in *ExternalJobSpec) DeepCopy() *ExternalJobSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalJobStatus) DeepCopyInto(out *ExternalJobStatus) { *out = *in - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobStatus. diff --git a/examples/conversion/pkg/apis/jobs/v2/externaljob_types.go b/pkg/webhook/conversion/testdata/api/v2/externaljob_types.go similarity index 92% rename from examples/conversion/pkg/apis/jobs/v2/externaljob_types.go rename to pkg/webhook/conversion/testdata/api/v2/externaljob_types.go index c6e75ee0e8..de5a03a212 100644 --- a/examples/conversion/pkg/apis/jobs/v2/externaljob_types.go +++ b/pkg/webhook/conversion/testdata/api/v2/externaljob_types.go @@ -35,11 +35,9 @@ type ExternalJobStatus struct { // Important: Run "make" to regenerate code after modifying this file } -// +genclient -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:object:root=true // ExternalJob is the Schema for the externaljobs API -// +k8s:openapi-gen=true type ExternalJob struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -48,7 +46,7 @@ type ExternalJob struct { Status ExternalJobStatus `json:"status,omitempty"` } -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:object:root=true // ExternalJobList contains a list of ExternalJob type ExternalJobList struct { diff --git a/examples/conversion/pkg/apis/jobs/v2/register.go b/pkg/webhook/conversion/testdata/api/v2/groupversion_info.go similarity index 54% rename from examples/conversion/pkg/apis/jobs/v2/register.go rename to pkg/webhook/conversion/testdata/api/v2/groupversion_info.go index 70e26e15b0..5019111a00 100644 --- a/examples/conversion/pkg/apis/jobs/v2/register.go +++ b/pkg/webhook/conversion/testdata/api/v2/groupversion_info.go @@ -13,14 +13,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -// NOTE: Boilerplate only. Ignore this file. - // Package v2 contains API Schema definitions for the jobs v2 API group -// +k8s:openapi-gen=true -// +k8s:deepcopy-gen=package,register -// +k8s:conversion-gen=github.com/droot/crd-conversion-example/pkg/apis/jobs -// +k8s:defaulter-gen=TypeMeta -// +groupName=jobs.example.org +// +kubebuilder:object:generate=true +// +groupName=jobs.testprojects.kb.io package v2 import ( @@ -29,17 +24,12 @@ import ( ) var ( - // SchemeGroupVersion is group version used to register these objects - SchemeGroupVersion = schema.GroupVersion{Group: "jobs.example.org", Version: "v2"} + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "jobs.testprojects.kb.io", Version: "v2"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion} + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} - // AddToScheme is required by pkg/client/... + // AddToScheme adds the types in this group-version to the given scheme. AddToScheme = SchemeBuilder.AddToScheme ) - -// Resource is required by pkg/client/listers/... -func Resource(resource string) schema.GroupResource { - return SchemeGroupVersion.WithResource(resource).GroupResource() -} diff --git a/examples/conversion/pkg/apis/jobs/v2/zz_generated.deepcopy.go b/pkg/webhook/conversion/testdata/api/v2/zz_generated.deepcopy.go similarity index 97% rename from examples/conversion/pkg/apis/jobs/v2/zz_generated.deepcopy.go rename to pkg/webhook/conversion/testdata/api/v2/zz_generated.deepcopy.go index cffbbf5577..53c9f758b1 100644 --- a/examples/conversion/pkg/apis/jobs/v2/zz_generated.deepcopy.go +++ b/pkg/webhook/conversion/testdata/api/v2/zz_generated.deepcopy.go @@ -14,7 +14,8 @@ 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. */ -// Code generated by main. DO NOT EDIT. + +// autogenerated by controller-gen object, do not modify manually package v2 @@ -29,7 +30,6 @@ func (in *ExternalJob) DeepCopyInto(out *ExternalJob) { in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec out.Status = in.Status - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJob. @@ -62,7 +62,6 @@ func (in *ExternalJobList) DeepCopyInto(out *ExternalJobList) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobList. @@ -86,7 +85,6 @@ func (in *ExternalJobList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalJobSpec) DeepCopyInto(out *ExternalJobSpec) { *out = *in - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobSpec. @@ -102,7 +100,6 @@ func (in *ExternalJobSpec) DeepCopy() *ExternalJobSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalJobStatus) DeepCopyInto(out *ExternalJobStatus) { *out = *in - return } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobStatus. diff --git a/pkg/webhook/conversion/testdata/api/v3/externaljob_types.go b/pkg/webhook/conversion/testdata/api/v3/externaljob_types.go new file mode 100644 index 0000000000..15c438f68a --- /dev/null +++ b/pkg/webhook/conversion/testdata/api/v3/externaljob_types.go @@ -0,0 +1,92 @@ +/* + +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 v3 + +import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/conversion" + + v2 "sigs.k8s.io/controller-runtime/pkg/webhook/conversion/testdata/api/v2" +) + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// ExternalJobSpec defines the desired state of ExternalJob +type ExternalJobSpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + DeferredAt string `json:"deferredAt"` +} + +// ExternalJobStatus defines the observed state of ExternalJob +type ExternalJobStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +// +kubebuilder:object:root=true + +// ExternalJob is the Schema for the externaljobs API +type ExternalJob struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ExternalJobSpec `json:"spec,omitempty"` + Status ExternalJobStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// ExternalJobList contains a list of ExternalJob +type ExternalJobList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ExternalJob `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ExternalJob{}, &ExternalJobList{}) +} + +// ConvertTo implements conversion logic to convert to Hub type (v2.ExternalJob +// in this case) +func (ej *ExternalJob) ConvertTo(dst conversion.Hub) error { + switch t := dst.(type) { + case *v2.ExternalJob: + jobv2 := dst.(*v2.ExternalJob) + jobv2.ObjectMeta = ej.ObjectMeta + jobv2.Spec.ScheduleAt = ej.Spec.DeferredAt + return nil + default: + return fmt.Errorf("unsupported type %v", t) + } +} + +// ConvertFrom implements conversion logic to convert from Hub type (v2.ExternalJob +// in this case) +func (ej *ExternalJob) ConvertFrom(src conversion.Hub) error { + switch t := src.(type) { + case *v2.ExternalJob: + jobv2 := src.(*v2.ExternalJob) + ej.ObjectMeta = jobv2.ObjectMeta + ej.Spec.DeferredAt = jobv2.Spec.ScheduleAt + return nil + default: + return fmt.Errorf("unsupported type %v", t) + } +} diff --git a/pkg/webhook/conversion/testdata/api/v3/groupversion_info.go b/pkg/webhook/conversion/testdata/api/v3/groupversion_info.go new file mode 100644 index 0000000000..1ae8269614 --- /dev/null +++ b/pkg/webhook/conversion/testdata/api/v3/groupversion_info.go @@ -0,0 +1,35 @@ +/* + +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 v3 contains API Schema definitions for the jobs v3 API group +// +kubebuilder:object:generate=true +// +groupName=jobs.testprojects.kb.io +package v3 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "jobs.testprojects.kb.io", Version: "v3"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) diff --git a/pkg/webhook/conversion/testdata/api/v3/zz_generated.deepcopy.go b/pkg/webhook/conversion/testdata/api/v3/zz_generated.deepcopy.go new file mode 100644 index 0000000000..a90942b427 --- /dev/null +++ b/pkg/webhook/conversion/testdata/api/v3/zz_generated.deepcopy.go @@ -0,0 +1,113 @@ +// +build !ignore_autogenerated + +/* + +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. +*/ + +// autogenerated by controller-gen object, do not modify manually + +package v3 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalJob) DeepCopyInto(out *ExternalJob) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJob. +func (in *ExternalJob) DeepCopy() *ExternalJob { + if in == nil { + return nil + } + out := new(ExternalJob) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ExternalJob) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalJobList) DeepCopyInto(out *ExternalJobList) { + *out = *in + out.TypeMeta = in.TypeMeta + out.ListMeta = in.ListMeta + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ExternalJob, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobList. +func (in *ExternalJobList) DeepCopy() *ExternalJobList { + if in == nil { + return nil + } + out := new(ExternalJobList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ExternalJobList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalJobSpec) DeepCopyInto(out *ExternalJobSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobSpec. +func (in *ExternalJobSpec) DeepCopy() *ExternalJobSpec { + if in == nil { + return nil + } + out := new(ExternalJobSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalJobStatus) DeepCopyInto(out *ExternalJobStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalJobStatus. +func (in *ExternalJobStatus) DeepCopy() *ExternalJobStatus { + if in == nil { + return nil + } + out := new(ExternalJobStatus) + in.DeepCopyInto(out) + return out +} diff --git a/examples/conversion/pkg/apis/jobs/group.go b/pkg/webhook/conversion/testdata/hack/boilerplate.go.txt similarity index 89% rename from examples/conversion/pkg/apis/jobs/group.go rename to pkg/webhook/conversion/testdata/hack/boilerplate.go.txt index 02b401aeb6..b92001fb4e 100644 --- a/examples/conversion/pkg/apis/jobs/group.go +++ b/pkg/webhook/conversion/testdata/hack/boilerplate.go.txt @@ -11,7 +11,4 @@ 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 jobs contains jobs API versions -package jobs +*/ \ No newline at end of file diff --git a/pkg/webhook/conversion/testdata/main.go b/pkg/webhook/conversion/testdata/main.go new file mode 100644 index 0000000000..2291dac0c3 --- /dev/null +++ b/pkg/webhook/conversion/testdata/main.go @@ -0,0 +1,72 @@ +/* + +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 main + +import ( + "flag" + "os" + + "k8s.io/apimachinery/pkg/runtime" + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + jobsv1 "testdata.kb.io/api/v1" + jobsv2 "testdata.kb.io/api/v2" + jobsv3 "testdata.kb.io/api/v3" + // +kubebuilder:scaffold:imports +) + +var ( + scheme = runtime.NewScheme() + setupLog = ctrl.Log.WithName("setup") +) + +func init() { + + jobsv1.AddToScheme(scheme) + jobsv2.AddToScheme(scheme) + jobsv3.AddToScheme(scheme) + // +kubebuilder:scaffold:scheme +} + +func main() { + var metricsAddr string + var enableLeaderElection bool + flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") + flag.BoolVar(&enableLeaderElection, "enable-leader-election", false, + "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.") + flag.Parse() + + ctrl.SetLogger(zap.Logger(true)) + + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + Scheme: scheme, + MetricsBindAddress: metricsAddr, + LeaderElection: enableLeaderElection, + }) + if err != nil { + setupLog.Error(err, "unable to start manager") + os.Exit(1) + } + + // +kubebuilder:scaffold:builder + + setupLog.Info("starting manager") + if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + setupLog.Error(err, "problem running manager") + os.Exit(1) + } +}