Enhancement: Able to get controller-runtime Client from the Resources

[dmvolod]

In most of the cases, we need to get and utilize controller-runtime Client internally for the intermediate functions
It would be nice to have a functions GetClient() inside the klient/k8s/resources/resources.go to get controller-runtime Client for this case.
Alternatively, we could probably have a helper function in the klient package returning Client as

cr.New(c.cfg, cr.Options{Scheme: scheme.Scheme})

[vladimirvivien]

Hi @dmvolod
Thanks for the suggestion, can you share why you need access to the controller-runtime client itself ? Whatever the reason, maybe it is something klient itself need to support. Let us know.

[dmvolod]

Hi @vladimirvivien
Thanks for response. Our case is: we have a set of the helpers methods (lookup for default StorageClass, some PVC logic, etc.) in our controllers utilising controller-runtime Client and would like to reutilize part of them inside the e2e-framework tests to reduce the codebase. For example we should skip test if default StorageClass not allows to extend a volume size.

[harshanarayana]

@dmvolod Is this not what you are looking ?

e2e-framework/klient/k8s/resources/resources.go

Lines 51 to 70 in e93ce38

	func New(cfg *rest.Config) (*Resources, error) {
	if cfg == nil {
	return nil, errors.New("must provide rest.Config")
	}
	cl, err := cr.New(cfg, cr.Options{Scheme: scheme.Scheme})
	if err != nil {
	return nil, err
	}
	res := &Resources{
	config: cfg,
	scheme: scheme.Scheme,
	client: cl,
	}
	return res, nil
	}
	// GetConfig hepls to get config type *rest.Config

Are you looking for a way to access the resources.client outside via a getter ?

[harshanarayana]

For example we should skip test if default StorageClass not allows to extend a volume size.

Assess("Do Something", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
			r, err := resources.New(c.Client().RESTConfig())
			if err != nil {
				t.Fail()
			}
			var sc v1.StorageClass
			err = r.Get(ctx, "test", "", &sc)
			if err != nil && errors.IsNotFound(err) {
				t.Skip("No tests are there is missing storage class")
			}
			if sc.AllowVolumeExpansion == nil || !*sc.AllowVolumeExpansion {
				t.Skip("Skipping test as the volume expansion is not allowed")
			}
			// Do something
			return ctx
		}).

@dmvolod What you are looking for can be achieved this way too.

[dmvolod]

Thanks, @harshanarayana , I know how to do this, but we have a dozens of functions and don't won't to repeate code of them in the tests instead of just provider one parameter with the cr.Client

[harshanarayana]

I see. So

Are you looking for a way to access the resources.client outside via a getter ?

this is what you are looking for ?

[dmvolod]

Are you looking for a way to access the resources.client outside via a getter ?

Yes, exactly, @harshanarayana , missed your first comment.

[vladimirvivien]

@dmvolod can you provide a code snippet of what you are doing with the controller-client once you get it? I ask because that can be useful functionality worth adding directly in the framework itself.

How are you using the client to detect StorageClass enabled ? Or is it the fact that you have existing code that you don't want to change and exposing the client would make integration easier ?

[Lxrdknows77]

Hi @vladimirvivien

In our kubernetes operator, we use a dedicated struct to provide all parameters to iterate with the k8s cluster. One of these parameters is controller-runtime Client cr client.Client:

type k8sCluster struct {
	Client                client.Client
	Scheme                *runtime.Scheme
	Logger                logr.Logger
	Object                client.Object
	...                  ...
}

And as @dmvolod said before we also have a lot of functions (based on native k8s controller-runtime client functions like Get(), List() etc.).

And also we have a method to validate StorageClass Volume Expansion:

func (k *k8sCluster) IsAllowVolumeExpansion(storageClass *storagev1.StorageClass) bool {
	return !(storageClass.AllowVolumeExpansion == nil || (storageClass.AllowVolumeExpansion != nil && !*storageClass.AllowVolumeExpansion))
}

Thus, to test this (and another) method in our e2e tests we have to re-implement them all in accordance with your klient.Klient client instead if just import our package and use it as it is.

Much better would be have helper function something like @dmvolod mention above:
cr.New(c.cfg, cr.Options{Scheme: scheme.Scheme} to get contriller-runtime Client and then provide it to our k8sCluster struct. And then it will let us to re-use all our methods in e2e tests.

k8sCluster.IsAllowVolumeExpansion(storageClass)

[vladimirvivien]

I think this is a good suggestion and exposing the controller-runtime client does not break the current API.

[harshanarayana]

/assign

[harshanarayana]

@vladimirvivien @Lxrdknows77 @dmvolod I have a draft PR with two ways of fetching the controller runtime client. TPAL when time permits.

[Lxrdknows77]

Thanks for the implementetion, @harshanarayana. Is NewControllerRuntimeClient just an option to directly get a client without using *Resources?

[harshanarayana]

@Lxrdknows77 Yes and also lets you inject your own custom runtime.Scheme for any uses that might arise.

[k8s-triage-robot]

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

[Lxrdknows77]

@harshanarayana Will you fix it or I can recreate PR?

[harshanarayana]

@Lxrdknows77 Sorry, this one totally skipped my list of TODO items. Please feel free to recreate the PR. If you can wait until end of the week, I will have the PR ready

[k8s-triage-robot]

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

[vladimirvivien]

/remove-lifecycle rotten

[vladimirvivien]

@harshanarayana Have you gotten a chance to finalize this so we can merge?

[harshanarayana]

@vladimirvivien I have already update the PR #156 with some of the comments addressed.