Skip to content

[BUG] Wrong kubernetes client is used in plugin manager #3688

Closed

Description

@sonjaer

Describe the bug

We are working on enabling the ray plugin in Propeller running in our cluster A with the remoteClusterConfig connecting to a remote Ray cluster B.

During Propeller startup, we hit a problem:

ray: [PluginInitializationFailed] Error getting informer for %!s(\u003cnil\u003e), caused by: no matches for kind \"RayJob\" in version \"ray.io/v1alpha1\""

Installing the ray crd in cluster A solved this problem. However, we would expect it to be enough for it to just be installed in cluster B.

After fixing that, we also hit another issue during startup where a permission is needed for Propeller service account to list rayjobs in cluster A, where this permission should only be needed in cluster B.

W0516 13:51:19.779009       1 reflector.go:324] pkg/mod/k8s.io/client-go@v0.24.1/tools/cache/reflector.go:167: failed to list *v1alpha1.RayJob: rayjobs.ray.io is forbidden: User "<service-account>" cannot list resource "rayjobs" in API group "ray.io" at the cluster scope
E0516 13:51:19.779088       1 reflector.go:138] pkg/mod/k8s.io/client-go@v0.24.1/tools/cache/reflector.go:167: Failed to watch *v1alpha1.RayJob: failed to list *v1alpha1.RayJob: rayjobs.ray.io is forbidden: User "<service-account>" cannot list resource "rayjobs" in API group "ray.io" at the cluster scope

The problem seem to be that the plugin manager doesn't use the correct k8 client (e.g., the k8 client in the plugin).

Expected behavior

The plugin manager should use the custom kubernetes client from the plugin if it exists.

However at many places it doesn't, e.g.,:

1.https://github.com/flyteorg/flytepropeller/blob/9a4ea000af6bb7b959daa00f26abea7c2e3262e7/pkg/controller/nodes/task/k8s/plugin_manager.go#L654
2.https://github.com/flyteorg/flytepropeller/blob/9a4ea000af6bb7b959daa00f26abea7c2e3262e7/pkg/controller/nodes/task/k8s/plugin_manager.go#L546

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes

Activity

  1. added
    bugSomething isn't working
    untriagedThis issues has not yet been looked at by the Maintainers
    on May 16, 2023
  2. sonjaer commented on May 16, 2023

    @sonjaer
    ContributorAuthor
  3. removed
    untriagedThis issues has not yet been looked at by the Maintainers
    on May 17, 2023
  4. honnix commented on May 22, 2023

    @honnix
    Member

    This would be an interesting change for anyone does have inter-cluster setup because it has impact on how flytepropeller works and behaves, covering at least the following aspects:

    • Before the fix, RBAC for the custom resource is required by the cluster where flytepropller is deployed, even that custom resource never gets created into the cluster; after the fix, this is no longer needed
    • Before the fix, the informer/caching never works as expected for the inter-cluster setup because there is nothing to cache; after the fix, the cache will start being populated and there might be a noticeable change of memory usage pattern
    • There might be some metrics being impacted as well, but not very sure
    • Before the fix, a CRD has to be deployed to both the cluster where flytepropeller is deployed, and the actual cluster where the CRD operator runs; after the fix, not entirely sure whether this is still required (it might still be the case in order for informer to work)

    So, I suggest we give very good highlight in release note to give users more context and possible actions to take.

  5. hamersaw commented on May 22, 2023

    @hamersaw
    Member

    @honnix this sounds great. I think this change makes inter-cluster work how it is supposed to (ie. same as how intra-cluster works) with the only caveat of some minor prometheus metrics. The resource monitor construct seems to work using a map of resource type to monitor, not taking the cluster into consideration. So if there are two plugins that operate over the same resource, where one is inter-cluster and the other intra-cluster, only the first to be registered will emit these metrics. This is quite a corner-case and, if I was a user, would probably get these metrics from the k8s api server more reliably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions