Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't index and list by arbitrary field with Unstructured objects #1705

Closed
mengqiy opened this issue Oct 25, 2021 · 12 comments
Closed

Can't index and list by arbitrary field with Unstructured objects #1705

mengqiy opened this issue Oct 25, 2021 · 12 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@mengqiy
Copy link
Member

mengqiy commented Oct 25, 2021

I can't index and list by arbitrary field with Unstructured objects in controller-runtime v0.7.0+ (including the latest v0.10.2). It will complain with the following error:
{"reconciler group": "example.co", "reconciler kind": "Foo", "name": "hello", "namespace": "default", "error": "field label not supported: metadata.ownerReferences.example.co.foo"}

I can do that with Unstructured objects in controller-runtime v0.6.*.

Code not working
package main

import (
  "context"
  "flag"
  "fmt"
  "os"

  "k8s.io/apimachinery/pkg/runtime/schema"
  "sigs.k8s.io/controller-runtime/pkg/client"
  "sigs.k8s.io/controller-runtime/pkg/reconcile"

  _ "k8s.io/client-go/plugin/pkg/client/auth"

  "k8s.io/apimachinery/pkg/runtime"
  "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  utilruntime "k8s.io/apimachinery/pkg/util/runtime"
  clientgoscheme "k8s.io/client-go/kubernetes/scheme"
  ctrl "sigs.k8s.io/controller-runtime"
  "sigs.k8s.io/controller-runtime/pkg/log/zap"
)

var (
  sche   = runtime.NewScheme()
  setupLog = ctrl.Log.WithName("setup")
)

const indexName = "metadata.ownerReferences.example.co.foo"

func init() {
  utilruntime.Must(clientgoscheme.AddToScheme(sche))
}

func main() {
  var metricsAddr string
  var enableLeaderElection bool
  var probeAddr string
  flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
  flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
  flag.BoolVar(&enableLeaderElection, "leader-elect", false,
    "Enable leader election for controller manager. "+
      "Enabling this will ensure there is only one active controller manager.")
  opts := zap.Options{
    Development: true,
  }
  opts.BindFlags(flag.CommandLine)
  flag.Parse()

  ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

  mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
    Scheme:                 sche,
    MetricsBindAddress:     metricsAddr,
    Port:                   9443,
    HealthProbeBindAddress: probeAddr,
    LeaderElection:         enableLeaderElection,
    LeaderElectionID:       "dd1da13f.example.co",
  })
  if err != nil {
    setupLog.Error(err, "unable to start manager")
    os.Exit(1)
  }

  uo := &unstructured.Unstructured{}
  uo.SetAPIVersion("example.co/v1")
  uo.SetKind("Foo")

  upo := &unstructured.Unstructured{}
  upo.SetGroupVersionKind(schema.GroupVersionKind{
    Group:   "",
    Version: "v1",
    Kind:    "Pod",
  })

  cl := mgr.GetClient()
  indexer := mgr.GetFieldIndexer()

  ///////////////////////////////////////////////////////////////////////////////
  // Note: We call the IndexField function by passing in a Unstructured object
  ///////////////////////////////////////////////////////////////////////////////
  err = indexer.IndexField(context.Background(), upo, indexName, func(object client.Object) []string { // <==== upo is a Unstructured object.
    ownerRef := metav1.GetControllerOf(object)
    if ownerRef == nil {
      return nil
    }
    if ownerRef.APIVersion != "example.co/v1" || ownerRef.Kind != "Foo" {
      return nil
    }
    return []string{ownerRef.Name}
  })

  err = ctrl.NewControllerManagedBy(mgr).
    For(uo).
    Owns(upo).
    Complete(reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error){
      listOps := []client.ListOption{
        client.MatchingFields{indexName: req.Name},
        client.InNamespace(req.Namespace),
      }
    uout := &unstructured.UnstructuredList{}
    uout.SetGroupVersionKind(schema.GroupVersionKind{
      Group:   "",
      Version: "v1",
      Kind:    "PodList",
    })
  ///////////////////////////////////////////////////////////////////////////////
  // Note: We call cl.List with an UnstructuredList object
  ///////////////////////////////////////////////////////////////////////////////
      if err = cl.List(ctx, uout, listOps...); err != nil {
        return reconcile.Result{}, err
      }

      return reconcile.Result{}, nil
  }))
  if err != nil {
    setupLog.Error(err, "unable to create controller")
    os.Exit(1)
  }

  setupLog.Info("starting manager")
  if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
    setupLog.Error(err, "problem running manager")
    os.Exit(1)
  }
}

I have also verified that v0.10.2 works with typed objects and PartialObjectMetadata objects.

@mengqiy
Copy link
Member Author

mengqiy commented Oct 25, 2021

/kind bug
/kind regression

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. labels Oct 25, 2021
@mengqiy
Copy link
Member Author

mengqiy commented Oct 25, 2021

If this is a known issue or I miss something, please let me know.

@mengqiy
Copy link
Member Author

mengqiy commented Oct 25, 2021

@mengqiy
Copy link
Member Author

mengqiy commented Oct 28, 2021

Also, surprisingly we don't have real tests covering the workflow of using IndexField. It seems currently we only have one example at https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/client/example_test.go#L249-L268 and it is not tested, since the client and indexer are not real.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 26, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 25, 2022
@FillZpp
Copy link
Contributor

FillZpp commented Feb 28, 2022

@mengqiy It is because the delegatingClient defaults to get/list unstructured objects from apiserver, so apiserver returns field label not supported. You can solve it by listing pods with cache cl := mgr.GetCache() in the above code.

BTW, is it the expected behavior that delegatingClient always get/list with direct client for unstructured objects, even if they already have been created informers in cache? @alvaroaleman @vincepri

@alvaroaleman
Copy link
Member

If that was the explanation it would never have worked I think? Because to my knowledge we always behaved like that (unstructured objects go to apiserver, structured to cache).

It is expected and was decided to be like that, at the time that was the only way to avoid using the cache. Today, there is ClientDisableCacheFor which can be used for this purpose. I agree it is not particularly intuitive, but changing it might break ppl in a subtile way.

@FillZpp
Copy link
Contributor

FillZpp commented Mar 1, 2022

Today, there is ClientDisableCacheFor which can be used for this purpose.

That is a little different. ClientDisableCacheFor is mostly for not caching some typed objects and get/list them directly from apiserver. But here the problem is we always get/list unstructured objects from apiserver, even if they have been watched by some controllers and we have informers of them in cache.

I agree it is not particularly intuitive, but changing it might break ppl in a subtile way.

Understand. We can consider this to be the expected behavior for now.

@alvaroaleman
Copy link
Member

For some background, also see #615 and apparently we added an option to hit the cache with unstructured in #1332

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

5 participants