kubernetes / client-go

Go client for Kubernetes.
Apache License 2.0
8.78k stars 2.9k forks source link

Knative Service object misinterpreted as K8s Core Service #1288

Closed tommazzo89 closed 11 months ago

tommazzo89 commented 11 months ago

Hi everyone,

I'm working on a project, where I need to programmatically create Knative services (services.serving.knative.dev) from a Kubernetes controller created with kubebuilder. When creating the Service, I get the following error from API server: "Service \"extract-successes-128mib-83m\" is invalid: spec.ports: Required value". The Knative Service doesn't have a spec.ports field. It seems that the K8s client interprets the Knative Service object as a K8s core Service and inserts that TypeMeta instead of the correct Knative Service TypeMeta. Thus, the API server reports that validation error.

I have already asked the Knative people (see here) and they recommended me to ask in #sig-api-machinery on Slack, but I didn't get any response. So, I'm trying a client-go issue now :) This is a blocking issue for a part of my project, so any help would be appreciated.

Here are the details of what I do:

I use v0.27.4 of k8s.io/api, k8s.io/apimachinery, and k8s.io/client-go and v0.37.2 of knative.dev/serving, I load an existing Knative service, deep clone its spec and ObjectMeta, modify a few things, and then try to create a new one using a ServingV1Interface. This results in the above mentioned error.

On startup I properly add Knserving to my scheme:


import (
    knServing "knative.dev/serving/pkg/apis/serving/v1"
    // ...
)

func init() {
    utilruntime.Must(clientgoscheme.AddToScheme(scheme))
    utilruntime.Must(chunkfuncv1.AddToScheme(scheme))
    utilruntime.Must(knServing.AddToScheme(scheme))
    //+kubebuilder:scaffold:scheme
}

This is how I create a new Service (simplified):

import (
    knServing "knative.dev/serving/pkg/apis/serving/v1"
    // ...
)

svc := &knServing.Service{
        ObjectMeta: *origSvc.ObjectMeta.DeepCopy(),
        Spec:       *origSvc.Spec.DeepCopy(),
    }
// Modify namespace, name, and resources.

// client is a ServingV1Interface from knative.dev/serving/pkg/client/clientset/versioned/typed/serving/v1
newSvc, err := client.Services(svc.Namespace).Create(ctx, svc, meta.CreateOptions{})
// This error mentions that "Service \"extract-successes-128mib-83m\" is invalid: spec.ports: Required value"

Do you know how to fix this?

liggitt commented 11 months ago

Do you have a link to how you are constructing the client instance?

tommazzo89 commented 11 months ago

Hi @liggitt, Thank you for your fast response! Unfortunately, my code is not yet open source (it will be once it's complete).

To construct the client I use the New(rest.Interface) function from knative.dev/serving/pkg/client/clientset/versioned/typed/serving/v1 (see here).

The REST client that I pass to New() is constructed like this (the Manager is constructed by kubebuilder-generated code):

func (fdr *FunctionDescriptionReconciler) SetupWithManager(mgr ctrl.Manager) error {
    k8sConfig := mgr.GetConfig()
    if err := kubeutil.SetKubernetesConfigDefaults(k8sConfig); err != nil {
        return err
    }
    restClient, err := rest.RESTClientFor(k8sConfig)
    if err != nil {
        return err
    }
    fdr.restClient = restClient

    // ...
}

// setKubernetesDefaults sets default values on the provided client config for accessing the
// Kubernetes API or returns an error if any of the defaults are impossible or invalid.
// Original version copied from https://github.com/kubernetes/kubectl/blob/82a943479841e06efdbb8543d28cfcd0c028c8b6/pkg/cmd/util/kubectl_match_version.go#L112-L129
func SetKubernetesConfigDefaults(config *rest.Config) error {
    config.GroupVersion = &schema.GroupVersion{Group: "", Version: "v1"}

    if config.APIPath == "" {
        config.APIPath = "/api"
    }
    if config.NegotiatedSerializer == nil {
        // This codec factory ensures the resources are not converted. Therefore, resources
        // will not be round-tripped through internal versions. Defaulting does not happen
        // on the client.
        config.NegotiatedSerializer = scheme.Codecs.WithoutConversion()
    }
    return rest.SetKubernetesDefaults(config)
}

It just occurred to me that even though I add the Knative Service to the scheme, the REST Client might not know about it. How do I get a scheme object into a new REST Client?

liggitt commented 11 months ago

this is the issue:

    config.GroupVersion = &schema.GroupVersion{Group: "", Version: "v1"}

    if config.APIPath == "" {
        config.APIPath = "/api"
    }

that means the client you are constructing is submitting API requests to /api/v1/${svc.Namespace}, which is the core Service API endpoint, not your custom resource endpoint

liggitt commented 11 months ago

your client should be set up like this:

    config.GroupVersion = &schema.GroupVersion{Group: "serving.knative.dev", Version: "v1"}

    if config.APIPath == "" {
        config.APIPath = "/apis"
    }
tommazzo89 commented 11 months ago

Now it's working! Thanks a lot!

Before I close the issue, I would just like to ask one more thing for my understanding: so, is it necessary to create a new rest.Client for every Group/Version combination? Even if I would want to access a Pod and a Deployment using a rest.Client I would need two, one for the core group and one for the apps group, right?

liggitt commented 11 months ago

Each group/version has a client interface/instance, yes.

The Kubernetes client-go package makes it easy to construct a single http.Client and then reuse it for all the rest.Client instances for the default Kubernetes APIs (for connection reuse, etc) by constructing a clientset:

https://github.com/kubernetes/client-go/blob/1bfd0186bce9c6c6ba0f1079d51116c1a4cddaf3/kubernetes/clientset.go#L455-L460

You can see the shared http.Client gets reused when constructing all the nested rest.Clients

tommazzo89 commented 11 months ago

I see. Perfect, thanks a lot for your help and for the explanation!