Skip to content

Commit

Permalink
Fix logic to detect missing GVKs
Browse files Browse the repository at this point in the history
Before revendoring controller-runtime `watch` was running `start` by default which made it possible to use the returned error to discriminated the existing GVKs https://github.com/openshift/cluster-autoscaler-operator/blob/a0b7aead18c86ac9a51f3479cba0c656ed83065f/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go#L122.

Without this commit, the new behaviour introduced by controller-runtime https://github.com/kubernetes-sigs/controller-runtime/blob/b6092bd383f0317f48ee396629ceb46d7dd064b4/pkg/internal/controller/controller.go#L137 is hidding the missing GVKs until the manager is started resulting in a fatal error.
  • Loading branch information
enxebre committed Apr 6, 2020
1 parent 0bc4a9c commit 36e2318
Showing 1 changed file with 46 additions and 24 deletions.
70 changes: 46 additions & 24 deletions pkg/controller/machineautoscaler/machineautoscaler_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func NewReconciler(mgr manager.Manager, config Config) *Reconciler {
}

// AddToManager adds a new Controller to mgr with r as the reconcile.Reconciler
// TODO(alberto): add ginkgo like integration test.
func (r *Reconciler) AddToManager(mgr manager.Manager) error {
// Create a new controller
c, err := controller.New(controllerName, mgr, controller.Options{Reconciler: r})
Expand All @@ -99,43 +100,64 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
return err
}

missingGVKs := []schema.GroupVersionKind{}
missingGVKs, err := getMissingGVKs(mgr.GetRESTMapper(), r.SupportedGVKs())
if err != nil {
return err
}

// Remove missing GVKs from list of supported GVKs.
for _, gvk := range missingGVKs {
r.RemoveSupportedGVK(gvk)
}

// Fail if we didn't find any of the supported target types registered.
if len(r.SupportedGVKs()) < 1 {
return ErrNoSupportedTargets
}

// Watch for changes to each supported target resource type and enqueue
// reconcile requests for their owning MachineAutoscaler resources.
for _, gvk := range r.config.SupportedTargetGVKs {
for _, gvk := range r.SupportedGVKs() {
target := &unstructured.Unstructured{}
target.SetGroupVersionKind(gvk)

err := c.Watch(
// Watch for changes to each supported target resource type and enqueue
// reconcile requests for their owning MachineAutoscaler resources.
if err = c.Watch(
&source.Kind{Type: target},
&handler.EnqueueRequestsFromMapFunc{
ToRequests: handler.ToRequestsFunc(targetOwnerRequest),
})

// If we get an error indicating that no matching type is registered
// with the API, then remove it from the list of supported target GVKs.
// If the type is later registered, a restart of the operator will pick
// it up and properly reconcile any MachineAutoscalers referencing it.
if err != nil && meta.IsNoMatchError(err) {
klog.Warningf("Removing support for unregistered target type: %s", gvk)
missingGVKs = append(missingGVKs, gvk)
} else if err != nil {
}); err != nil {
return err
}
}

// Remove missing GVKs from list of supported GVKs.
for _, gvk := range missingGVKs {
r.RemoveSupportedGVK(gvk)
}
return nil
}

// Fail if we didn't find any of the supported target types registered.
if len(r.config.SupportedTargetGVKs) < 1 {
return ErrNoSupportedTargets
func getMissingGVKs(restMapper meta.RESTMapper, supportedGVKs []schema.GroupVersionKind) ([]schema.GroupVersionKind, error) {
var missingGVKs []schema.GroupVersionKind

for _, gvk := range supportedGVKs {
if _, err := restMapper.KindFor(
schema.GroupVersionResource{
Group: gvk.Group,
Version: gvk.Version,
Resource: gvk.Kind,
}); err != nil {

// If we get an error indicating that no matching type is registered
// with the API, then remove it from the list of supported target GVKs.
// If the type is later registered, a restart of the operator will pick
// it up and properly reconcile any MachineAutoscalers referencing it.
if meta.IsNoMatchError(err) {
klog.Warningf("Removing support for unregistered target type: %s", gvk)
missingGVKs = append(missingGVKs, gvk)
continue
}

return missingGVKs, err
}
}

return nil
return missingGVKs, nil
}

var _ reconcile.Reconciler = &Reconciler{}
Expand Down

0 comments on commit 36e2318

Please sign in to comment.